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

Treat empty $PATH component as equivalent to "." #3914

Closed
faho opened this issue Mar 26, 2017 · 4 comments
Closed

Treat empty $PATH component as equivalent to "." #3914

faho opened this issue Mar 26, 2017 · 4 comments
Assignees
Labels
Milestone

Comments

@faho
Copy link
Member

@faho faho commented Mar 26, 2017

As recently discovered in #3912:

$ printf '%s\n' '#!/bin/sh' 'echo banana' > banana
$ chmod +x banana
$ set PATH "" $PATH
$ banana
# Command not found
$ set PATH "." $PATH
$ banana
banana

As far as I can see, fish just effectively ignores the empty component. The compatible behavior is to treat it like it was ".", i.e. the current directory. This is important because fish inherits these variables from the outside and exports them, so interoperability with POSIX is useful here.

This possibly also applies to $MANPATH, and was recently fixed for $CDPATH (#3901, #2106).

@faho faho added the RFC label Mar 26, 2017
@faho faho added this to the fish-future milestone Mar 26, 2017
@faho faho mentioned this issue Mar 26, 2017
1 of 2 tasks complete
@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Mar 27, 2017

FWIW, I think fish should treat empty path components as equivalent to . for compatibility with POSIX shells. The POSIX shell behavior is FUBAR in my opinion. Anyone wanting to include the CWD can do so by putting . in the path (which is what I do). Nonetheless, this discrepancy is something that only makes it harder for people to switch to fish shell and does not have a tangible benefit that I can see.

@faho
Copy link
Member Author

@faho faho commented Mar 27, 2017

Nonetheless, this discrepancy is something that only makes it harder for people to switch to fish shell and does not have a tangible benefit that I can find.

One benefit is that empty components make it harder to iterate over these lists.

E.g.

for command in $PATH/*
    test -x $command -a ! -d $command; and echo $command
end

would list all executable commands iff there was no empty $PATH component that is treated specially. And if we allow those, it's hard to miss.

(This was obviously found in #3912 - see how I complicated the implementation for empty $PATH components)

Now, the problem is that we pretty much can't get by just ignoring empty $PATH components because we can inherit them from the outside. It might be possible to convert them (to ".") when importing instead.

However, whatever we do, it should be consistent between $PATH, $CDPATH (which currently allows empty components) and $MANPATH.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Mar 28, 2017

My vote is to magically munge all three vars when set/imported to translate empty elements to .. That way scripts don't have to do anything special and constructs like for command in $PATH/a_command work sensibly without having to do anything special. This is trivial to implement by augmenting react_to_variable_change() to special-case those three env vars like we do for curses and locale env vars.

@krader1961 krader1961 removed the RFC label Mar 28, 2017
@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Apr 2, 2017

I feel compelled to expand on my earlier POSIX shell behavior is FUBAR assertion. It's problematic because in POSIX shells it is far too easy, and common, to inadvertently create an empty path element when manipulating one of PATH, CDPATH, MANPATH. Or any similar var for that matter such as LD_LIBRARY_PATH. It was a huge mistake to treat an empty path element as equivalent to an explicit . to represent the CWD.

I would love to eliminate the empty path element is equivalent to . rule. At least within fish if not other shells. However doing so causes problems with no obvious benefits and therefore cannot be justified. Notwithstanding the fact that making such a change is satisfying from a "if we could go back in time and correct that decision" viewpoint.

@krader1961 krader1961 self-assigned this Apr 2, 2017
@krader1961 krader1961 modified the milestones: 2.6.0, fish-future Apr 2, 2017
@krader1961 krader1961 closed this in a4f925d Apr 5, 2017
develop7 added a commit to develop7/fish-shell that referenced this issue Apr 17, 2017
There are at least three env vars describing a sequence of paths to be
searched where an empty path element is implicitly equivalent to ".".
This change converts the implicit "." to explicit whenever the variable
is imported or set. This makes the variable much easier to use in fish
scripts.

Fixes fish-shell#3914
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.