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

Crystal::System::Process shell invocation broken on Linux with FreeBSD sh #13919

Closed
valpackett opened this issue Oct 31, 2023 · 5 comments · Fixed by #13942
Closed

Crystal::System::Process shell invocation broken on Linux with FreeBSD sh #13919

valpackett opened this issue Oct 31, 2023 · 5 comments · Fixed by #13942
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:unix topic:stdlib:system

Comments

@valpackett
Copy link
Contributor

valpackett commented Oct 31, 2023

Chimera Linux uses FreeBSD sh, so this condition:

{% if flag?(:freebsd) || flag?(:dragonfly) %}
shell_args << ""
{% end %}

doesn't match and as a result, the substitution is broken which breaks e.g. the compilation of macros (ld: error: undefined symbol: main due to no object files actually getting provided) :/

Is the conditional actually necessary? I'm not sure when the empty arg could break things…

@nekopsykose
Copy link
Contributor

a quick test in yash, bash, zsh, dash, mksh, busybox ash reveals that all of them are consistent in printing 1 2 3 for shell -c "echo \$@" -- "1" "2" "3" and -c "echo \$@" -- "" "1" "2" "3". the empty "" is only required for the bsd shells apparently, but it also does not break any other. this should be safe to just remove the if condition and make it universal

@nekopsykose
Copy link
Contributor

nekopsykose commented Oct 31, 2023

hm, no, it seems the -- is being passed as $0. so the reason it happens to work 'consistently' is because the shell is actually using -- as $0, and then $@ is 1-onward. but on the bsd shell, the -- delimiter is actually parsed, and then the "" is required to make $0 something else (and so using it universally will break $1 on other shells with a nul)

@nekopsykose
Copy link
Contributor

ah ok, so just replacing -- with sh (any string) should work. /bin/sh -c 'program "$@"' sh 1 2 3 seems to work with every shell

@q66
Copy link

q66 commented Oct 31, 2023

basically just replace -- with sh in the original args and then drop the special case for freebsd/dragonfly entirely

@straight-shoota
Copy link
Member

Oh that's interesting. So I suppose another option to write the same thing would be ["/bin/sh", "-c" "$0 ${@}", command, *args]?
We can't do that though because it would break use cases where command includes ${@}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:unix topic:stdlib:system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants