Fix nvm install error if /sbin/init is absent and errtrace is enabled
#2698
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ljharb
reviewed
Dec 26, 2021
|
I haven't added any tests yet, but I've made the changes we discussed and they don't seem to break anything 🤞🏻 |
ljharb
requested changes
Dec 26, 2021
`nvm install` fails with "Binary download failed, trying source" when - running on Bash; - errtrace (`set -E`) is enabled; - an ERR trap uses `exit` to return a non-zero status; and - /sbin/init doesn't exist. Resolved by moving `ls -dl /sbin/init` to the following `if` statement. In this context, returning non-zero isn't an error and the ERR trap isn't executed.
|
I've added a test that fails under the conditions we've established are problematic unless one or both of the subsequent commits are merged. Let me know if there's anything else you need :) |
ljharb
approved these changes
Dec 27, 2021
This was referenced Dec 27, 2021
|
Thanks for merging this back in December, @ljharb :) I hope it's okay to ask here if a new release is imminent? In CI contexts, at least, workarounds for this bug are still necessary until an nvm release is made and downstream packages pick it up. 🙏🏻 |
|
Unfortunately tests are broken, so merges and releases are blocked on #2802. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Discussed at length in #2678)
ls -dl /sbin/initto the followingifstatement for impliciterror handling (even if enabled,
errexitanderrtracehave noeffect in this context)
set -E) in nvm_get_arch and consolidate existingchecks for
set -eandset -ainto one loopnvm installfails with "Binary download failed, trying source" in ArchLinux package build scripts. This is the combined result of
makepkgrunning these scripts with ERR trap inheritance enabled, and the lack
of error handling when
nvm_get_archcallsls -dl /sbin/init. If/sbin/initdoesn't exist, the ERR trap set bymakepkgexitsnon-zero and the operation fails.