search

reitzig / sdkman-for-fish

Adds support for SDKMAN! to fish
MIT License
280 stars 13 forks source link

Use full paths to core Unix commands in functions #23

Closed jfrazee closed 4 years ago

jfrazee commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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

reitzig commented 4 years ago

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.