asdf-ruby icon indicating copy to clipboard operation
asdf-ruby copied to clipboard

os_based_configure_options cannot bail early

Open akerl opened this issue 8 years ago • 3 comments

I ran into an issue w/ os_based_configure_options: Because it is called in a subshell (https://github.com/asdf-vm/asdf-ruby/blob/302efde548eb62705a268ceda0fd8898b2d2c6da/bin/install#L57), if it exits, that error gets swallowed up.

This occurs if homebrew isn't available on https://github.com/asdf-vm/asdf-ruby/blob/302efde548eb62705a268ceda0fd8898b2d2c6da/bin/install#L95 .

So in my case, it can't find homebrew, that check exits 1, but that gets absorbed by the subshell and the build attempts to continue. It fails, but as a similar issue, we "exit 1" from the subshell on https://github.com/asdf-vm/asdf-ruby/blob/302efde548eb62705a268ceda0fd8898b2d2c6da/bin/install#L46-L49 , so those exit codes are swallowed.

The end result is that I run an install, the exit code is 0, but no installation has occurred. This appears to be replicated exactly or similar across at least asdf-python and asdf-erlang

akerl avatar May 08 '17 19:05 akerl

On a related note, I'm curious about the utility of ASDF_PKG_MISSING. It's set inside os_based_configure_options, but doesn't bubble up since it's using a subshell, so it'll always be empty everywhere it's used.

akerl avatar May 08 '17 19:05 akerl

For the asdf-erlang plugin we are moving away from strictly requiring homebrew to be installed to just asserting that the dependencies are present. That's another thing that should probably be changed here too.

Stratus3D avatar May 10 '17 18:05 Stratus3D

This code does not exist in this plugin anymore, I think this issue is safe to close out.

For anyone else who lands on this page via google, code setting ASDF_PKG_MISSING in various asdf packages looks like it was copy/pasted from a plugin way back. The variable is not used by the core asdf package and does nothing when set unless there's conditional logic to check for it within the asdf plugin script.

iloveitaly avatar May 10 '21 21:05 iloveitaly