-
Notifications
You must be signed in to change notification settings - Fork 348
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
opam 1.2.2 destroys the $PATH variable when used with the fish shell #2165
Comments
Related to issues #1797, #1877, #1932, #2063, #2068, #2070. The code is at https://github.com/ocaml/opam/blob/master/src/state/opamState.ml#L2037 if you want to have a look. The same code is called with the expanded PATH (for |
Yeah, sometimes fish can be pretty annoying... Too bad its so pretty and tempting :) The quickest fix I can think of is to avoid quotes if the string is a variable expansion for the array we are working with.
I think this should work both expanded and unexpanded variables. In addition to that, we need to include this extra line before the command that sets the MANPATH:
This is because we have to make sure that MANPATH always ends up starting with a semicolon. |
Thanks! Care to fill a pull-request ? |
Is there a way to test the pull-request changes without overwriting my ~/.opam with |
Yes! Just export |
After I installed opam 1.2.2, sourcing the .opam/opam/init.fish script ruins the PATH environment variable and makes my shell unusable.
The problem is that Opam is putting quotes around the $PATH:
This sets the PATH to be a 2-element array, with the second element being a space-separated list of the old paths. What we actually want is an array of length N+1 and to get that we should leave out the quotes:
There is a similar issue with the MANPATH variable but its a bit more complicated because the semantics of MANPATH are different from the semantics for PATH. In order for
man
to find the default system man pages, either MANPATH needs to be unset or MANPATH needs to be a colon-separated list with an empty element in it (that it, it must start with:
, end with:
or have a::
somewhere in the middle). If we use the same technique we use for PATH it won't work in the case when MANPATH is previously unset. This is an unfortunate quirk in fish.Anyway, changing the MANPATH assignment to this version seems to work for me:
The first line is to make sure system manpages still work when MANPATH is initially unset and in the second line we got rid of the quotes just like we did for PATH.
A second possibility would be to set MANPATH to an one-element array containing a colon-separated string in any format we want. This would be more future-proof in case a future version of fish decides to stop special-casing MANPATH but is a bit more tricky to implement (in a way that doesn't break if there are spaces or newlines in the file names).
The text was updated successfully, but these errors were encountered: