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

PUID & GUID to Dockerfile #15

Closed
wants to merge 2 commits into from
Closed

PUID & GUID to Dockerfile #15

wants to merge 2 commits into from

Conversation

kylepauljohnson
Copy link

@kylepauljohnson kylepauljohnson commented Mar 25, 2021

I think this corrects the issue I raised.

I am unclear if there is any undesired effects of removing the current user openttd, however in local testing all seemed to work as expected.

In the interest of full disclosure I can't fully validate this container config on my Synology as I am building it on an ARM M1 Mac, which fails to run on the Synology. Running locally appears to have the desired effect.

Thanks for your responsiveness on the issue!

@kylepauljohnson kylepauljohnson marked this pull request as ready for review March 25, 2021 00:10
Copy link
Collaborator

@duckfullstop duckfullstop left a comment

Choose a reason for hiding this comment

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

I think we may need to add something to the entrypoint to take ownership of the appropriate directories so that saving and writing files still works, but I'm not able to double check that at the moment. Have you verified that things like autosave still function?

@duckfullstop
Copy link
Collaborator

(And sorry for the delay on responding, it's been a manic week!)

@duckfullstop duckfullstop linked an issue Mar 31, 2021 that may be closed by this pull request
@zogot
Copy link

zogot commented Apr 7, 2021

Also, I dont think 1000:1000 is the correct default UserID and GroupID.

If i start it up fresh, i seem to get 911 userid and 1000 groupid consistently.

I think perhaps the better approach, to help ensure it, is an explicit set of the UID and GID with ENV values that let us define what those are.

So running useradd -u $PUID and ENV PUID=911.

I can look to making the changings tonight if that would help.

@kylepauljohnson
Copy link
Author

@zogot Feel free to run with it. I have been pretty buried as it sounds has @duckfullstop. Thanks for looking at it.

Copy link
Collaborator

@duckfullstop duckfullstop left a comment

Choose a reason for hiding this comment

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

I'm minded to close this PR for a couple reasons:

  1. it's quite old, and the author may not be around anymore
  2. the PUID and PGID changes would only take effect during image build, not during execution - we'd still need that change to the entrypoint

I'm happy to merge if both of the above can be disproved (heck, it might also close #17 in the process), but otherwise closing and then throwing it back to the original issue is probably the right move.

@kylepauljohnson
Copy link
Author

As much as I would like to complete and solve this, I currently have no time. I will go ahead and close this.

@duckfullstop
Copy link
Collaborator

duckfullstop commented Feb 13, 2023

I'll spend some time thinking how to best fix this when I can - probably continue discussion in #14?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UID and GID Specification
3 participants