Skip to content

Use "command -v" instead of "which" to detect commands#1419

Merged
sbc100 merged 2 commits intoemscripten-core:mainfrom
akoeplinger:replace-which
Jul 1, 2024
Merged

Use "command -v" instead of "which" to detect commands#1419
sbc100 merged 2 commits intoemscripten-core:mainfrom
akoeplinger:replace-which

Conversation

@akoeplinger
Copy link
Copy Markdown
Contributor

On a Linux distro that doesn't have the which program installed we're getting the following error:

$ ./emsdk install latest
./emsdk: line 39: exec: python: not found

It's failing to detect the installed python3 and falls back to using python, but this distro doesn't provide a python -> python3 symlink so we fail.

Fix this by using command -v instead which is a POSIX standard.
The same change went into emscripten a couple years ago: emscripten-core/emscripten#15071

On a Linux distro that doesn't have the `which` program installed we're getting the following error:

    ./emsdk install latest
    ./emsdk: line 39: exec: python: not found

It's failing to detect the installed `python3` and falls back to using `python`, but this distro doesn't provide a python -> python3 symlink so we fail.

Fix this by using `command -v` instead which is a POSIX standard.
The same change went into emscripten a couple years ago: emscripten-core/emscripten#15071
Comment thread emsdk
# https://github.com/emscripten-core/emsdk/pull/273)
if [ -z "$EMSDK_PYTHON" ]; then
if PYTHON3="$(which python3 2>/dev/null)"; then
if PYTHON3="$(command -v python3 2>/dev/null)"; then
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all the other changes apart from this line can be reverted.

This is the only place where the code really needs to be portable and run on many different distros. All the other places are basically build scripts where we control the environment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbc100 done

Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, with a limiting of the scope of this change.

@allsey87
Copy link
Copy Markdown
Contributor

allsey87 commented Jul 1, 2024

I think this is fine for now, but eventually it would be good to migrate to rules_python which should be more portable and eliminate the need for most of the .sh and .bat scripts.

@sbc100 sbc100 merged commit 7fbd555 into emscripten-core:main Jul 1, 2024
@akoeplinger akoeplinger deleted the replace-which branch July 2, 2024 13:48
mmorel-35 pushed a commit to mmorel-35/emsdk that referenced this pull request Feb 3, 2026
…ripten-core#1419)

On a Linux distro that doesn't have the `which` program installed we're
getting the following error:

    $ ./emsdk install latest
    ./emsdk: line 39: exec: python: not found

It's failing to detect the installed `python3` and falls back to using
`python`, but this distro doesn't provide a python -> python3 symlink so
we fail.

Fix this by using `command -v` instead which is a POSIX standard.
The same change went into emscripten a couple years ago:
emscripten-core/emscripten#15071
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants