feat(poetry): use sudo to install curl if running as user #119
Conversation
If the UID is not 0 (root), uses sudo to install curl. Also checking if curl is already installed before trying to install it.
CoRfr
requested review from
jonathan-johnston and
shaundialpad
and removed request for
a team
poetry/orb.yaml
Outdated
| @@ -26,16 +26,24 @@ commands: | |||
| type: string | |||
| steps: | |||
| - run: | |||
| name: install curl | |||
| name: Install curl | |||
There was a problem hiding this comment.
nit: leave this lowercase please, all our step names are.
| apt-get update -qy | ||
| apt-get install -qy curl | ||
| if cat /etc/issue | grep "Alpine" >/dev/null 2>&1; then | ||
| if [ "$ID" = 0 ]; then export SUDO=""; else export SUDO="sudo"; fi |
There was a problem hiding this comment.
I was under the impression Alpine doesn't tend to ship with the sudo binary, do you have a sample flow for how you tested this which I could play with?
I haven't ever really had opportunity to do this, but I have vague recollections about them using su-exec instead?
There was a problem hiding this comment.
Looks like the default alpine doesn't ship with either, so in any case it needs to be installed beforehand.
There was a problem hiding this comment.
Hmm, is running alpine with ID!=0 just not possible, then, if you need to use non-default packages? Maybe we need to make that an error case? Or avoid setting this SUDO value in that case? Just want to make sure we aren't breaking any cases which previously worked, here.
poetry/orb.yaml
Outdated
| exit 1 | ||
| if [[ $EUID == 0 ]]; then export SUDO=""; else export SUDO="sudo"; fi | ||
| fi | ||
| echo "export SUDO=$SUDO" >> ${BASH_ENV} |
There was a problem hiding this comment.
I don't think we need this -- the SUDO variable isn't used in any future steps here, so we can keep it contained into only this step rather than potentially affecting future tasks.
poetry/orb.yaml
Outdated
| if [[ $EUID == 0 ]]; then export SUDO=""; else export SUDO="sudo"; fi | ||
| fi | ||
| echo "export SUDO=$SUDO" >> ${BASH_ENV} | ||
| if [ ! "$(command -v curl)" ]; then |
There was a problem hiding this comment.
Let's move this check to be the first line of this step: there's no need to do any work in this step if curl is already installed!
Summary
If the UID is not 0 (root), uses sudo to install curl. Also checking if curl is already installed before trying to install it.
Standard Checklist