-
Notifications
You must be signed in to change notification settings - Fork 139
Haskell for non root #74
Haskell for non root #74
Conversation
👍 Thanks for the contribution! I think that this is a good fix that should be merged. I do have some nitpicks though: Questions:
@danielbraun89 What can be done to improve the tests? Anything? Is there a way to test that this doesn't regress? I was able to use sudo -iu "$_REMOTE_USER" <<EOF
# https://www.haskell.org/ghcup/
curl --proto '=https' --tlsv1.2 -sSf https://get-ghcup.haskell.org | sh
EOF {
"image": "mcr.microsoft.com/devcontainers/universal:linux",
"features": {
"ghcr.io/jcbhmr/devcontainers-contrib-features/haskell:latest": {} // From pr74 branch
},
} Code changes# The installation script is designed to be run by the non-root user
# The files need to be in the remote user's ~/ home directory
-ROOT_HOME="${HOME}"
-export HOME="${_REMOTE_USER_HOME}"
-
-curl --proto '=https' --tlsv1.2 -sSf https://get-ghcup.haskell.org | $SHELL
-
-export HOME="${ROOT_HOME}"
-chown -R "${_REMOTE_USER}:${_REMOTE_USER}" "${_REMOTE_USER_HOME}"
+# So, how do we switch users? We use 'sudo -iu <username>' to get a
+# login shell of another user! We use $_REMOTE_USER as defined in
+# a spec proposal (but still implemented in Codespaces): https://github.com/devcontainers/spec/blob/main/proposals/features-user-env-variables.md
+# Here's some more examples using it: https://github.com/search?q=org%3Adevcontainers+_REMOTE_USER&type=code
+# We also use /bin/sh as defined in the script hash-bang line instead of $SHELL.
+sudo -iu "$_REMOTE_USER" <<EOF
+ # https://www.haskell.org/ghcup/
+ curl --proto '=https' --tlsv1.2 -sSf https://get-ghcup.haskell.org | sh
+EOF Relevant StackOverflow questions:
|
A note: I added my own PR against DaneWeber's PR branch in DaneWeber#2 with my suggested changes. The open question remains about the version bump. |
thanks @DaneWeber for finding this bug! seems like the testing method we inherited from the standard feature-starter is running with root user, Ill open another issue regarding improving the test to use non-root users as well. this will also catch additional issues as #80 |
@jcbhmr I agree sudo method is quite more elegant... how about ill merge this one as is, and then you PR the sudo implementation directly? Or do you prefer @DaneWeber approve it on his? |
@danielbraun89 It doesn't really matter. Whatever works best for you 🤷♂️. I'll work with whatever. |
Ok so lets go with the double merge in order to minimize the duration this bug stays open |
Happy to see this merged! I'd like to mention that I agree with @jcbhmr's improvement (assuming it's fine for these install scripts) and that I was overly cautious about using |
Fix
HOME
to_REMOTE_USER_HOME
for the execution of the install script.chown
back to the_REMOTE_USER
.I've also auto-formatted the file, but separated that into a different commit in case you don't want those changes.
Preexisting Behavior
The Haskell feature runs the install such that Cabal, Stack, etc. are installed in
/root
and are not useful when running the dev container in VS Code (or presumably other use cases).The GHCup install instructions mention that the commands should be run as non-root:
Reproduction
The following
.devcontainer.json
illustrates the issue (and is what VS Code creates when selecting default values for an Ubuntu devcontainer with the Haskell feature added):After building and opening VS Code in the devcontainer, a new terminal produces the following:
Note the existence of
.cabal
and.stack
in/root
.In the preexisting condition, you can verify that installing as the user succeeds by executing the following:
After everything is downloaded and installed, you'll have to source
~/.bashrc
and then you can see the success: