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

bug: Propagate errors from micromamba-shell #241

Merged
merged 2 commits into from
Nov 4, 2024
Merged

Conversation

maresb
Copy link
Contributor

@maresb maresb commented Nov 4, 2024

If the micromamba-shell command fails, the exit command is not propagated, leading to silent failures.

Consider if possible switching to bash for -o pipefail support.

@maresb maresb requested a review from pavelzw as a code owner November 4, 2024 11:39
Copy link
Member

@pavelzw pavelzw left a comment

Choose a reason for hiding this comment

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

Can you update the version in package.json?

@maresb maresb changed the title Propagate errors from micromamba-shell bug: Propagate errors from micromamba-shell Nov 4, 2024
@maresb
Copy link
Contributor Author

maresb commented Nov 4, 2024

Woah, thanks @pavelzw for the instant response!

Do you know offhand if it would be safe to assume that bash is available?

I'm wondering now if exec would be better here; I think that should propagate the error code and also free us from the parent shell.

@pavelzw
Copy link
Member

pavelzw commented Nov 4, 2024

Do you know offhand if it would be safe to assume that bash is available?

i think this is a fair assumption. micromamba-shell is only supported on unix anyway

exec sounds fair, let's do that

@maresb
Copy link
Contributor Author

maresb commented Nov 4, 2024

Ok, I need to do a few quick tests first since I'm wondering what's going on in terms of shell processing and if we would lose that with exec

Before, when the micromamba-shell command failed, the exit command would not be propagated,
leading to silent failures. Now execution is passed to micromamba-shell so that the parent
shell is replaced, and the exit code is that of the micromamba-shell process.
@maresb
Copy link
Contributor Author

maresb commented Nov 4, 2024

Ok @pavelzw, I checked and micromamba seems to be already doing the right thing by acting as a shell with -o pipefail enabled, so I believe exec should do the trick! I rebased with the exec solution, so ready for re-review. Thanks!!!

@pavelzw pavelzw added the enhancement New feature or request label Nov 4, 2024
@pavelzw pavelzw merged commit ab6bf8b into mamba-org:main Nov 4, 2024
63 of 64 checks passed
@maresb maresb deleted the patch-1 branch November 5, 2024 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants