Open wtraylor opened 3 years ago
thanks for reporting this @wtraylor! The motivation behind the install.sh
script is to address problems that some users have when installing via pip
. For users not familiar with python or how pip works, the curl | sh
method is convenient, as they have already installed docker (whose installer has better UX than pip
), so using install.sh
is straight-forward in this scenario.
Regarding the problems with uses: sh
inside a container, the specific example can be solved by doing:
steps:
- uses: docker://docker:19.03.10
args: ["--version"]
More fundamentally, one of the main goals of having container-native pipelines is to make them reproducible. By using sh
to invoke commands that are expected to be available in the host, the probability of having a pipeline fail is higher. This is why we note on the documentation that the use of uses: sh
should be minimized. Other additional "best practices" for steps with uses: sh
would be to 1) only reference paths that are within the workspace or bind-mounted folders (via the dir
option); and 2) only invoke POSIX/UNIX commands. These don't guarantee reproducibility but minimize errors. Maybe we should add this to the docs as well.
does this make sense?
does this make sense?
Yes, that makes sense. I see that it is very important to provide an easy installation method through install.sh
. On the other hand, as long as uses: sh
is in Popper’s feature set (whether using it is good practice or not), it should just work, no matter by which method Popper has been installed. I think that is very important.
One way out of the dilemma would be to fail with an error message if Popper running in Docker enconunters uses: sh
and kindly ask the user to install Popper through PIP.
With Popper installed through the
install.sh
script, which runs Popper within Docker, workflow steps withuses: sh
don’t execute on the host shell.Steps to Reproduce
The last command fails with:
That is because Popper cannot execute
docker --version
on the host shell, but only in its own container.Suggestion
My humble suggestion is to remove the
install.sh
feature.A proper installation through PIP has all the features one would expect (including Singularity and host shell). Recommending different installation methods with different feature sets can lead to great confusion, especially for beginners who just want to reproduce a given workflow. This will only be aggravated when (hopefully some day) Popper will be packaged for Linux distributions, because then a
/usr/local/bin/popper
installed throughinstall.sh
could unexpectedly override a/usr/bin/popper
installed through a package manager.