-
Notifications
You must be signed in to change notification settings - Fork 65
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
Pre-create common config directories #135
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@AObuchow maybe we can also create:
- for NuGet, it's the package manager for .NET:
/home/user/.nuget
- for composer, it's the package manager for PHP :
/home/user/.composer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm split on whether it'd be more readable to put all the mkdir
in one spot (and have one chgrp/chmod covering it all) rather than spreading it throughout the dockerfile, but approving this PR anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amisevsk, AObuchow, l0rd, svor The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
82c393b
to
3c0b27a
Compare
New changes are detected. LGTM label has been removed. |
Fix eclipse-che/che#22029 Pre-creates the following config directories: - ~/.m2 (Maven) - ~/.gradle (Gradle) - ~/.config/pip (Pip) - ~/.sbt/1.0 (SBT) - ~/.cargo (Rust) - ~/certs (SSL) - ~/.composer (PHP) - ~/.nuget (.NET) Signed-off-by: Andrew Obuchowicz <[email protected]>
3c0b27a
to
fbd7473
Compare
@svor @amisevsk @l0rd Thank you all for the reviews. I rebased this PR after merging #115, added the .NET and PHP package manager config directories ( I also followed @amisevsk's suggestion to have a dedicated section for the config directory creation: it's easier to see which directories we're creating when they're all in one spot, rather than spread out throughout the Dockerfile. |
Coming into this discussion late... But what if you focus on leveraging the structure of Raw Example
This way you help initialize a home on first run and ignore where someone has configured things another way. Use the same pattern we already use to setup user env on Linux systems? |
@codekow Could you elaborate further? I'm not entirely clear on how The issue we're trying to solve here is that configuration files (e.g. maven's |
@amisevsk I'm probably looking more at a structural view and maintaining patterns that already exist. The second use case is initializing configs during each runtime. Ex: I need a But the user can override I want the user to have the ability to easily override my solution for user config I didn't consider. My concern with the pattern I see on this thread is a lot of moving parts that can not be disabled if the user doesn't want it. Since words are sometimes hard. Let me know if this makes any sense... if not I'll try to give better examples. 😄 In short - move your logic for user settings initialization into |
Ah I think the piece we're missing is that mkdir -m 755 -p /home/user/.m2/ # as the *root* user If the directory already exists, this command is a no-op, but if it does not, you'll end up with an As for setting up things like
in this context, providing machinery for user setup (to me at least) seems like an unnecessary indirection. Any user setup you do at container run-time will be lost when the container is restarted, so it's actively discouraged to do things like "new user setup" outside the Dockerfile. |
I did a deeper dive into how che sets up the user env and see the primary volumes are:
Everything else is essentially read-only or ephemeral. I assumed |
This PR aims to fix eclipse-che/che#22029, with the scope limited to pre-creating config directories for the following common tools:
/home/user/.m2/
/home/user/.gradle/
/home/user/.config/pip/
/home/user/.cargo/
/home/user/.sbt/1.0/
/home/user/.composer/
/home/user/.nuget
As well as a
/home/user/certs/
directory.Yarn & npm were excluded since they are supposed to be placed in the $HOME directory (
~/.yarnrc
,~.npmrc
)There might be other tools which support configuration files that I've missed in this PR, please feel free to suggest any.
I've pushed
quay.io/aobuchow/universal-developer-image:m2
for testing this PR, I also used the following devfile for testing in Che:Testing
While logged into a cluster with Che installed, apply the following configmap list to your Che user namespace, it will create auto-mount configmaps (using the subPath mount method) for maven, gradle, sbt, pip and cargo:
Though it's a bit overkill, I made these configmaps to ensure the tool configurations could be read by the respective tools.
Maven (settings.xml)
Run the
mvn
command. Sinceoffline
is set to true, the command will fail with the following error message:Pip (pip.conf)
Run
pip install test
. Since we don't have a virtual environment created, and we're usingrequire-virtualenv = True
, you should get the following error:ERROR: Could not find an activated virtualenv (required).
SBT
Run
sbt -sbt-create
. Once the command completes (you'll see a few errors but it should still complete), the sbt prompt will be our custom prompt:[sbt springbootproject] >>
Cargo (config.toml)
Run
cargo init && cargo myalias
.myalias
is a custom alias fortest
, defined in ourconfig.toml
. You should see the following output: