Skip to content
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

contrib: Dockerfile: actually create gluon user directory #2838

Closed
wants to merge 1 commit into from

Conversation

skorpy2009
Copy link
Member

No description provided.

@neocturne
Copy link
Member

I assume this is already handled by the VOLUME? I don't think it makes much sense to copy the skel files into a directory that is mounted over anyways.

@AiyionPrime AiyionPrime added 0. type: question 2. status: waiting-on-review Awaiting review from the assignee but also interested parties. labels Apr 3, 2023
@AiyionPrime
Copy link
Member

@skorpy2009 is there a problem you intend to solve with this measure?
If not and neoraider is right, we could close this :)

@AiyionPrime AiyionPrime added 2. status: waiting-on-author Waiting on some action from the author and removed 2. status: waiting-on-review Awaiting review from the assignee but also interested parties. labels Apr 5, 2023
@skorpy2009
Copy link
Member Author

@NeoRaider @AiyionPrime currently if you use this container and want to do anything with the user, for example you need git config it breaks because there is no gluon folder

@neocturne
Copy link
Member

The Gluon build directory is supposed to be mounted into the container at /gluon.

Copy link
Member

@AiyionPrime AiyionPrime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For gluons build process this PR does not do any good.
The argument is, for others to reuse the Dockerfile as dependency installer without actually using the rest of the Dockerfile (e.g. the mount) this directory must still not be empty.
As this file lies in contrib, we could support it, if somebody verifies it does not break anything for us.

The commit message should mention as well, why this is a good idea, even though it's not necessary for gluon (to prevent the next person to optimize it out again)

@neocturne
Copy link
Member

It is unclear to me if the behaviors of Docker and Podman differ here (I only have Podman on my system), but Podman creates an empty volume and mounts it to /gluon when starting the container. I would expect the same to happen with Docker, as long as you don't override the mount at /gluon on the command line. I would also expect this not to change when another Dockerfile is based on the gluon/contrib/docker container.

So an explanation in what situation it is even possible to end up with no /gluon when using our Dockerfile would be great.

That being said, I looked at the Dockerfile reference again and found that the contents under the directory specified as a VOLUME will end up as initial content for the new volume by default. So maybe this change is not useless after all, as it will install skel contents into the volume, but that is not what the commit message is saying.

@neocturne
Copy link
Member

Fixed by #2975

@neocturne neocturne closed this Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. type: question 2. status: waiting-on-author Waiting on some action from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants