Skip to content

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