Skip to content

install-emscripten: hint emsdk_env.fish for fish shell - #427

Open
Andrej730 wants to merge 2 commits into
pyodide:mainfrom
Andrej730:hint-fish-source
Open

install-emscripten: hint emsdk_env.fish for fish shell#427
Andrej730 wants to merge 2 commits into
pyodide:mainfrom
Andrej730:hint-fish-source

Conversation

@Andrej730

@Andrej730 Andrej730 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Small change to print

Use `source /.../0.29.4/emsdk/emsdk_env.fish` to set up the environment.

instead of

Use `source /.../0.29.4/emsdk/emsdk_env.sh` to set up the environment.

Kept stumbling upon trying to copy the string to source the environment and running it without modifying extension.

Relying on SHELL env variable is not ideal (e.g. if someone logins with fish and then enters bash, SHELL is still fish), but it should do for the most cases.

Tests also don't hardcode path to script anymore, which also resolves the mismatch if those tests will be ran on Windows.

Unsure if those tests will actually are unblocked on Windows, let's test in CI.

@Andrej730

Copy link
Copy Markdown
Contributor Author
pyodide_build/tests/test_cli_install_emscripten.py::test_install_emscripten_no_xbuildenv PASSED [  9%]
pyodide_build/tests/test_cli_install_emscripten.py::test_install_emscripten_default_version PASSED [ 18%]
pyodide_build/tests/test_cli_install_emscripten.py::test_install_emscripten_specific_version PASSED [ 27%]
pyodide_build/tests/test_cli_install_emscripten.py::test_install_emscripten_with_existing_emsdk PASSED [ 36%]
pyodide_build/tests/test_cli_install_emscripten.py::test_install_emscripten_git_failure PASSED [ 45%]
pyodide_build/tests/test_cli_install_emscripten.py::test_install_emscripten_emsdk_install_failure PASSED [ 54%]
pyodide_build/tests/test_cli_install_emscripten.py::test_install_emscripten_force_flag PASSED [ 63%]
pyodide_build/tests/test_cli_install_emscripten.py::test_install_emscripten_output_format PASSED [ 72%]
pyodide_build/tests/test_common.py::test_default_xbuildenv_path_env_var_non_writable PASSED [ 81%]
pyodide_build/tests/test_venv.py::test_windows_host_pip_wrapper_runs_with_spaces_in_paths PASSED [ 90%]
pyodide_build/tests/test_venv.py::test_windows_pyodide_cli_script_runs_with_spaces_in_paths PASSED [100%]

I guess those tests are unblocked now on Windows.

@agriyakhetarpal

Copy link
Copy Markdown
Member

Relying on SHELL env variable is not ideal (e.g. if someone logins with fish and then enters bash, SHELL is still fish), but it should do for the most cases.

Do you think introducing shellingham or a similar dependency would be a good way to resolve this in a way that works for the vast majority of shells? If yes, I would be open to it. (If I remember correctly, it was already being picked up as a transitive dependency in our tree at some point)

@Andrej730

Andrej730 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Do you think introducing shellingham or a similar dependency would be a good way to resolve this in a way that works for the vast majority of shells? If yes, I would be open to it. (If I remember correctly, it was already being picked up as a transitive dependency in our tree at some point)

It could resolve more cases, but it seems this is such a minor issue and it's trivially fixable in case someone actually needs the fix for some shell - so maybe it's worth keeping it very simple?

@agriyakhetarpal

Copy link
Copy Markdown
Member

I think so, but I believe expanding our list of supported shells in this manner sets a precedent that can lead to the inclusion of more shells in the future, which justifies the need for shellingham at some point. To that effect, doing so now is better than later.

@Andrej730

Copy link
Copy Markdown
Contributor Author

If more advanced handling is needed, could it be addressed separately? I’m not sure I’d want to look into different shells detection details for this change.

@ryanking13

Copy link
Copy Markdown
Member

Maybe we can introduce shellingham when there is another request to support other shells? I don't really think there are any csh users who want to build packages, and PowerShell users can already use batch files (unless they are running powershell inside linux)

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