rmyorston / busybox-w32

WIN32 native port of BusyBox.
https://frippery.org/busybox
Other
697 stars 126 forks source link

Strange behaviour of which #443

Open ale5000-git opened 3 months ago

ale5000-git commented 3 months ago
$ busybox ash
$ export PATH='C:/Program Files/Git/usr/bin'
$ command -v which
which
$ which -a start
$ command -v start
C:/Program Files/Git/usr/bin/start

I haven't figured out why which can't find start (it is a shell script without extension).

avih commented 3 months ago

In many of your bug reports it's not obvious what you consider a bug, and why.

In this case, you posted the actual result, but It's not clear what the expected result is. And Why.

So next time you file issues, other than describing the actual result, please also post the expected result, and why.

ale5000-git commented 3 months ago

I haven't figured out why which can't find start

start is a real file (in this case) so if the command -v find it also which should find it.

rmyorston commented 3 months ago

The file isn't seen as being executable.

It's a bug in the code to look for '#!'.

There will be a fix.

rmyorston commented 3 months ago

OK, prerelease PRE-5453 or above has the fix.

ale5000-git commented 3 months ago

Now I have a different doubt. Inside busybox ash command -v also pick not executable files like README.rst instead inside bash it only pick executables.

Which is the correct one?

rmyorston commented 3 months ago

command -v only checks whether a file exists, not that it's executable.

So your busybox ash is behaving correctly and there's something wrong with your bash.

On my Linux sysytem I'm running bash, I have ~/bin on PATH and a non-executable file, links2, there:

$ ls -l ~/bin/links2
-rw-r--r--. 1 rmy rmy 288 Jun 22  2013 /home/rmyf40/bin/links2
$ command -V links2
links2 is /home/rmyf40/bin/links2
$ dash
$ command -V links2
links2 is /home/rmyf40/bin/links2
$ 
avih commented 3 months ago

command -v also pick not executable files like README.rst

In what sense is this file executable? does it have shebang? how else would it get executed?

command -v only checks whether a file exists, not that it's executable.

Not exactly. It check how the shell would interpret a name as a command:

-v Write a string to standard output that indicates the pathname or command that will be used by the shell, in the current shell execution environment (see 2.13 Shell Execution Environment ), to invoke command_name, but do not invoke command_name.

So largely it should take into account whether it can execute of sorts (shebang, executable bit, shell function, etc).

However, I still don't get how README.rst can be considered executable... I think it's not executable, and even if it's in $PATH I'd think command -v should not print it as something which the shell can execute...

EDIT: Oh, I see. You say that command -v README.1st does print its path eventhough it's not executable? if it does, then that sounds wrong to me, but I somehow doubt it would not error...

ale5000-git commented 3 months ago

@avih To be more clear:

busybox ash

$ command -v README.rst; echo $?
C:/.../README.rst
0

bash

$ command -v README.rst; echo $?
1

So busybox only check if the file exist but bash for Windows also check if it is executable.


After a few weeks working on compatibility between busybox / bash for Windows I can say that bash for Windows is a complete mess; I have to add a lot of workarounds in my code. In addition bash included inside Git for Windows has additional bugs compared to the base msys64.

avih commented 3 months ago

busybox ash

$ command -v README.rst; echo $?
C:/.../README.rst
0

Yeah. Got it before too. That looks wrong to me.

EDIT: but surprisingly shells really don't agree on that. The current dir has empty file foo which is not executable:

$ PATH=.:$PATH shcmp -c 'command -v foo'
= /bin/sh, bash, dash_58, dash_591, dash_5102, dash_5111, dash_512, bb_ash, nbsh, fbsh:
./foo
(exit:0)

= bash_posix, zsh, ksh93, ksh93u_m, loksh, loksh_62, oksh, oksh_65, mksh, mksh_posix, pdksh_5214, pdksh_orc, yash, yash_posix:
(exit:1)

= dash_master, bosh, bosh_posix, pbosh, gwsh:
(exit:127)

= fbsh14:
/home/user/dev/tests/svn-stuff/git-svn-sample/svn-sample-project-code--git/./foo
(exit:0)
ale5000-git commented 3 months ago

I also tried this code on bash online and it is found:

$ touch /tmp/README.rst
$ PATH="/tmp:$PATH"
$ command -v README.rst; echo $?
/tmp/README.rst
0
avih commented 3 months ago

I also tried this code on bash

Yeah. See the 1st line in my shell comparison output, bash printf ./foo and exit with code 0.

But in posix mode it fails... (bash_posix).

Butsybox-w32 ash does behave like all the other ash derivatives (dash, free/net bsd sh) and succeds (and like bash too)

But ksh93, zsh, yash and all pdksh derivatives (openbsd sh, mksh, etc) and others fail.

Interestingly, dash master does fail too.

In my mind, command -v was kinda like which, but more powerful because it also takes into account shell functions.

But this comparison makes it quite obvious that it cannot be counted on, because many shells succeed with command -v where which fails...

ale5000-git commented 3 months ago

About the difference it would be interesting to verify what POSIX say about that.

command -v is still useful because usually most "not executable" files aren't in PATH or they have an extension so if you use command -v README you won't find README.rst

Returning back to the main issue, it works perfectly in the latest prerelease, so if there isn't any other problem it can be closed.

avih commented 3 months ago

surprisingly shells really don't agree on that

After closer inspection of the comparison above, all shells except ash-derivatives (and bash in non-posix mode) do check the executable bit.

In dash master it was fixed few months ago, not yet in a release: https://git.kernel.org/pub/scm/utils/dash/dash.git/commit/?id=d489f2e2e9

This follows my initial thought that command -v is similar to which but also covers shell builtins and functions. Specifically, it should not succeed when only not-executables by that name are found in $PATH.

It also follows the POSIX specifications as far as I can tell, about command -v:

Executable utilities, regular built-in utilities, command_names including a character, and any implementation-provided functions that are found using the PATH variable (as described in "2.9.1.4 Command Search and Execution"), shall be written as absolute pathnames.

2.9.14 says:

e. Otherwise, the command shall be searched for using the PATH environment variable as described in XBD 8. Environment Variables"

XBD 8 says about PATH:

the list shall be searched from beginning to end, applying the filename to each prefix and attempting to resolve the resulting pathname (see "4.16 Pathname Resolution"), until an executable file with appropriate execution permissions is found.

So this is correct technically about what buxybox does, but it's a bug:

command -v only checks whether a file exists, not that it's executable.

It does follow upstream busybox ash, and it does follow all other ash derivatives (free/net bsd sh, dash), but it was fixed recently in dash master, and it should be fixed in busybox[-w32] as well.

ale5000-git commented 1 month ago

@rmyorston POSIX has also changed the text for command -v from "Utilities" to "Executable utilities" , you can see all details in my report here => https://bugs.busybox.net/show_bug.cgi?id=16201

avih commented 1 month ago

It does follow upstream busybox ash, and it does follow all other ash derivatives (free/net bsd sh, dash), but it was fixed recently in dash master, and it should be fixed in busybox[-w32] as well.

Fixed in upstream: https://git.busybox.net/busybox/commit/?id=860b3d066f6aaa12dfa0cd2351559e05288cf9b5

rmyorston commented 1 month ago

The upstream fix to bring BusyBox ash into line with the updated POSIX interpretation of command has been merged. Please try the latest prerelease (PRE-5500 or above).

ale5000-git commented 1 month ago

Thanks a lot, it now works perfectly with command -v, command -V and type.

There is only a special case that I find pretty annoying (but I think it also apply to upstream):

BusyBox for Windows:

$ PATH=";" type build.sh
build.sh is build.sh

Bash under Windows:

$ PATH=":" type build.sh
build.sh is /c/Users/my_user/Documents/my_folder/./build.sh

I find both result wrong (in my opinion). When I use these utilities I expect to have them returning a path (excluding builtins and applets), but in this case BusyBox doesn't do it and Bash add an additional useless ./

What do you think?

avih commented 1 month ago

Busybox is wrong and bash is not.

POSIX says that for utilities found using $PATH, command -v should print an absolute pathname. bash prints an absolute pathname (not canonical, but that's not a requirement), while busybox command -v doesn't.

rmyorston commented 1 month ago

Upstream are aware of the issue. Their commit adds this comment in reference to files found in the current directory (where they return "NAME"):

// A bit of discrepancy wrt the path used if file is found here.
// bash 5.2.15 "type" returns "./NAME".
// GNU which v2.21 returns "/CUR/DIR/NAME".
ale5000-git commented 1 month ago

Currently it return this:

$ PATH="" command -v build.sh
build.sh
$ PATH=":" command -v build.sh
build.sh
$ PATH="." command -v build.sh
./build.sh

The third case, although it is annoying, at least it return a path but the first two cases do not.