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

Improve install.sh #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

YouJiacheng
Copy link

@YouJiacheng YouJiacheng commented Nov 21, 2023

#8 (comment)

parent=$(ps -o comm $PPID |tail -1) will break docker build with error: process ID out of range if /bin/sh is actually bash or bash is explicitly used.

Reason: RUN ["/bin/bash", "-c", "/bin/bash <(curl -L micro.mamba.pm/install.sh)"] will cause the shell environment has $$=1 and $PPID=0.

As discussed in https://stackoverflow.com/questions/77524391/ppid-behavior-of-bash-c-and-dockerfile-run , https://stackoverflow.com/questions/67827986/why-is-bash-handling-child-processes-different-compared-to-sh, https://stackoverflow.com/questions/76310409/does-bash-promise-to-optimize-c-into-plain-exec-in-simple-cases, and https://unix.stackexchange.com/a/466523/537347, bash will introduce optimization that exec the last command and make $$=1 (considering the PID namespace mechanism of container).

RUN ["/bin/bash", "-c", "(/bin/bash <(curl -L micro.mamba.pm/install.sh))"] (i.e. create a subshell) or RUN ["/bin/bash", "-c", "/bin/bash <(curl -L micro.mamba.pm/install.sh); exit"] can work.

It seems that we should fallback to $$ if $PPID is not exists of is not a compatible shell.

As discussed in #8 (comment), for non-POSIX compliant shell for example fish and xonsh, install.sh might be executed by bash or sh. Thus we cannot use $$ by default.

However, using $PPID instead of $$ does introduce counter-intuitive behavior: run zsh <(curl -L micro.mamba.pm/install.sh) in a bash will init bash.
Instruction in the documentation (and README of this repo) "${SHELL}" <(curl -L https://micro.mamba.pm/install.sh) does NOT always use sh, suggesting that "${SHELL}" will be initialized (but actually NOT). And there isn't any remark/note around this instruction.

Nonetheless, allow something like INIT_WHICH_SHELL (in {bash,cmd.exe,dash,fish,posix,powershell,tcsh,xonsh,zsh}) to be set before the script is run to facilitate non-interactive installation can be really helpful.

cc @AntoinePrv

Copy link
Member

@AntoinePrv AntoinePrv left a comment

Choose a reason for hiding this comment

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

Thanks!

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.

2 participants