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

Use full paths to core Unix commands in functions #23

Closed
jfrazee opened this issue Oct 28, 2018 · 6 comments
Closed

Use full paths to core Unix commands in functions #23

jfrazee opened this issue Oct 28, 2018 · 6 comments
Labels
wontfix This will not be worked on

Comments

@jfrazee
Copy link

jfrazee commented Oct 28, 2018

The sdk functions currently assume that the standard Unix commands are first on the path and unaliased (e.g., conf.d/sdk.fish#L45), so if you have something like rm aliased to 'rm -i', running sdk will prompt you every time with:

rm: remove regular file '/tmp/tmp.OvvGBzqDYa'? y

This can be fixed by using the full paths to any standard Unix commands (e.g., /bin/rm $pipe instead of rm $pipe).

@reitzig
Copy link
Owner

reitzig commented Oct 28, 2018

Discussions about whether it's smart to alias standard GNU tools (which I assume breaks many a script and program), is that really a robust fix? Different installations might store binaries in different places.

Is /usr/bin/env standard enough to use it instead?

Not sure if any of these replacement assumptions is more minimal than "vanilla GNU tools are on the PATH". :/

@jfrazee
Copy link
Author

jfrazee commented Oct 28, 2018

I sort of expected/I think that's a reasonable response. I'm used to seeing 3 options: (1) what you suggested with env, (2) just assuming the core commands are in /bin (which is nearly always true), or (3) using which to resolve the command.

I think it's fairly likely that env and which are going to be in /usr/bin.

Are you inclined to take a PR using /usr/bin/env then?

@danielporto
Copy link

danielporto commented Jul 22, 2019

just found another problem related to this.
on OSX stat doesnt work like the unix version. Instead, it is required to use gstat which in turn has to be installed using homebrew (brew install coreutils)

One recommended setting would be create an alias to stat with gstat and then solve the compatibility issue.
I couldn't find a way to make fish load these alias before calling sdk.fish thus I had to edit the file directly:
sdk.fish:if not set -q SDKMAN_DIR; or test (stat -c "%U" $SDKMAN_DIR) != (whoami)
to
sdk.fish:if not set -q SDKMAN_DIR; or test (gstat -c "%U" $SDKMAN_DIR) != (whoami)

it seems that this problem is more general and affects OSX users with standard configurations.

@reitzig
Copy link
Owner

reitzig commented Jul 30, 2019

@danielporto, the fact that macOS has a version of stat that's incompatible with GNU's is unfortunate but I don't think it has anything to do with the issue @jfrazee put forth.
--> #29

@jfrazee
Copy link
Author

jfrazee commented Jul 30, 2019

@reitzig @danielporto OS X doesn't have gstat by default either so I think it'd create an even less desirable situation.

@reitzig reitzig added the wontfix This will not be worked on label Jul 30, 2019
@reitzig
Copy link
Owner

reitzig commented Jul 30, 2019

I'm calling it, for now: this won't be fixed. We require GNU-ish tools on the PATH and users to not mess with them.

May reconsider if more issues and/or use cases pop up, and a solution that doesn't create more of a mess than solves.
Using command (cf. #37) seemed the most promising so far, but I'm wary of the side-effects of that.

I note that they implemented a similar workaround for a few commands in fisher (cf. jorgebucaran/fisher#79). The partial solution still seems wrong to me, but I'm ready to defer to more venerable shell artists.

That said, comment jorgebucaran/fisher#79.181039857 seems astute to me: In fish, you can (and probably should) use abbr instead of alias.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants