Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate builtin realpath #10035

Open
faho opened this issue Sep 29, 2023 · 8 comments
Open

Deprecate builtin realpath #10035

faho opened this issue Sep 29, 2023 · 8 comments

Comments

@faho
Copy link
Member

faho commented Sep 29, 2023

Our realpath builtin is quite odd.

Unlike other builtins, it's not actually used first because we didn't want to implement the entirety of the typically available interface, and so we have a function that gives any existing realpath (or even grealpath) command precedence.

That function also handles most of the option parsing, which is another idea that we should abandon.

But most of all: realpath implements an interface dictated by something else and doesn't fit well with the rest, and has awkward platform-dependent behavior - #5352. It is made almost entirely redundant by path resolve (there's some difference in how they handle non-existent paths), which was introduced in 3.4 (released march 2022). I may be biased, but I like that interface much better - it handles multiple arguments, pipes, NUL-delimited input/output, etc.

I propose we deprecate realpath formally, in the release notes, and then remove it in a few releases time. I would specifically call out a time, like "in 2 years" or something.

I don't believe a warning is a great idea because that would add error output where there is none. (alternatively we could add a deprecation system via e.g. flog categories that you can turn on and off, but this is a bigger change and should be discussed separately)

This is also a special case in that you have to specifically ask for builtin realpath (or otherwise override the function or have a broken installation), so it's pretty easy to find any uses of.

@faho faho added this to the fish next-3.x milestone Sep 29, 2023
@faho
Copy link
Member Author

faho commented Sep 29, 2023

One case that isn't well-covered: fish_add_path uses realpath -s, which won't resolve symlinks but still makes the path absolute. path resolve will resolve all symlinks while path normalize accepts relative paths - foo/bar is kept as-is.

@mqudsi
Copy link
Contributor

mqudsi commented Oct 5, 2023

I don't think this is a good idea because the discoverability of path resolve/normalize isn't great for users coming from other shells. We've made tremendous progress wrt to improving compatibility and making people already comfortable with other shells feel at home with fish, and this feels like a fairly large step back. It's already a PITA targeting older macOS where realpath isn't available (or is available nuked or whatever) and I think people are tremendously appreciative of having basic posix staples available on all modern versions of shells.

One thing that I have thought of for a lot of recent suggestions is that it would be nice to have some sort of onboarding for new users. I think a (colored? grey?) message posted to stderr followed by a repaint of the the prompt when certain commands we wish to provide an FYI for are used for the first time after a major release or a new install (and never shown again, at least until the next major/minor release) is one viable option for educating people about fishier alternatives to less ergonomic or less well-designed posix or posixy features, including realpath and alias.

(There's already precedent, we do this for npm and yarn completions, telling the user a maximum of once per session to install all_the_package_names to get npm/yarn package name completions for npm install ... and afaik users have only found this to be useful and have never complained - but there is a difference between "fyi install this for better completions" and "change your workflow to use this alternative command" and I would not feel comfortable spamming such a suggestion or notice more than once per install/release as opposed to once per interactive session.)

@faho
Copy link
Member Author

faho commented Oct 5, 2023

. We've made tremendous progress wrt to improving compatibility

Note: If we want to improve compatibility with other shells, we should remove realpath.

It's

  1. Not guaranteed to be there
  2. Not something that is always implemented the same everywhere
  3. Commonly has options that ours doesn't have

Plus our version is hidden behind a function, so it's only ever a weird fallback that behaves differently if the system doesn't have any.

making people already comfortable with other shells feel at home with fish

People already comfortable with other shells would know their realpath.

Plus, just the basic idea: Fish isn't compatible. If people want to be comfortable they already have a wide variety of much easier options available.

having basic posix staples available on all modern versions of shells.

realpath is not a posix staple. It's not in posix at all.


Fundamentally: I don't believe that this feature-starved half-reimplementation of a pseudo-standard tool (that on the whole is used "sometimes", not "constantly") is all that helpful in onboarding people from bash. They can learn fifty other, much harder things, they can learn path. Or keep on using the realpath they're used from other shells.

@mqudsi
Copy link
Contributor

mqudsi commented Oct 5, 2023

realpath is not a posix staple. It's not in posix at all.

Sorry, I was more explicit later and said "less ergonomic or less well-designed posix or posixy features,"

People already comfortable with other shells would know their realpath.

and

Plus our version is hidden behind a function, so it's only ever a weird fallback that behaves differently if the system doesn't have any.

kind of cancel each other out as realpath will be the realpath they are used to (in a weird, roundabout way).

@faho
Copy link
Member Author

faho commented Oct 5, 2023

One thing that I have thought of for a lot of recent suggestions is that it would be nice to have some sort of onboarding for new users.

This is, of course, a different suggestion.

I think that's worth looking into. The trick would be making it not annoying, and especially for new users I think simply having too many messages would be overwhelming?

Imagine using "alias" and getting "use abbr instead". Use "realpath" and get "use path resolve instead". Use "if; then; fi" and get "use if; end instead". Use $(( and get "use math instead".

In the worst case all within five minutes.

Anyway, I feel like our time would be better spent on making that, and fish itself, nicer rather than chasing this awkward piecemeal compatibility.

@faho
Copy link
Member Author

faho commented Oct 5, 2023

kind of cancel each other out as realpath will be the realpath they are used to (in a weird, roundabout way).

Well, no. If they're coming from other shells and have no realpath: They don't have a realpath they're used to, and there's no "making them feel at home" for realpath!

If they're not, then there's no realpath they're used to, and, well... the same.

If they're coming from other shells and have a realpath... well, they have a realpath and so aren't using ours to begin with. And if they did require something from realpath other than the bare minimum "takes one path and does a realpath-like thing with it" they'd be pretty disappointed with ours.

What I am saying is that our realpath is neither necessary nor sufficient to provide any sort of comfort. And therefore I would like to drop it in favor of the overall better tool, allowing us to shrink fish's surface area. For those who want to stick with realpath, they already have a realpath and were already using it.

I want fewer awkward tools we recommend people not to use in fish, and realpath is an easy start.


Edit: For some data, I have a clone of all the packages in OMF's "packages-main". I realize that's not a perfect overview, but it's easy to get.

Of those 229 packages, 11 use any kind of realpath. None specifies builtin realpath, so none would explicitly want our implementation.

  • 3 explicitly call command realpath or the realpath command (one is a lua script that shells out after checking for the command)
  • 3 use realpath in a way that won't work with our builtin (one explicitly uses --relative-base=, a GNU-ism)

That means half wouldn't work with our implementation and have never been used with it. (and the rest would be happy with any implementation, so we don't know if they have ever been used with ours, but given the ones that definitely don't work I'm leaning towards not)

@zanchey
Copy link
Member

zanchey commented Dec 31, 2023

We only added realpath as a builtin because it didn't exist on macOS (and maybe Solaris?), but it's in macOS 13 and 14. I think it's fine to drop it, perhaps waiting until after macOS 12 goes out of support. If dropping it before 3.8 is desirable, perhaps change the function to call path resolve?

@anordal
Copy link
Contributor

anordal commented Apr 12, 2024

Good call.

No mention of good old readlink -f? On platforms with GNU coreutils, that has existed longer (realpath was only added to coreutils in 2012). While this mattered more a decade ago than now, there is still the argument of induction, that there is still no reason to switch to realpath and is never going to be (they behave the same on coreutils). Well, unless realpath is more portable to macOS, BSD or something. Is it? Should one even use realpath?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants