Abuild keygen fixes #52
Conversation
abuild-keygen will look for gitconfig to fetch user and email address and set proper PACKAGER variable in abuild.conf
docker creates mount points as root when source does not exists instead create them first with UID 1000 which should be the UID of the builder user inside the container.
| mkdir -p $ABUILD_PACKAGES | ||
| # docker will create mount points as root if not available so create them first | ||
| # use uid 1000 as this is the uid of the builder user in the container | ||
| install -d -o 1000 -g 1000 "$ABUILD_PACKAGES" "$HOME"/.abuild |
There was a problem hiding this comment.
Whoops -- thanks for catching that I don't check "$HOME/".abuild exists.
Not sure about creating them with [UG]ID=1000 though. Maybe better to invoke docker run ... below with --user "$(id -u)":"$(id -g)"? (I'm only on D4M here and it looks like some magic is being done to map through to my host user account in either case, so can't easily test.)
There was a problem hiding this comment.
Why is setting explicitly 1000 a problem? the first user is always assigned 1000?
There was a problem hiding this comment.
hmm, i was thinking only from a point of view from the container, its probably not smart to set 1000 if the current host user has a different uid (which is possible). I guess we need to rethink how to manage those bind mounts or instead use docker volumes.
There was a problem hiding this comment.
Yes-- mapping between the uid spaces in container and on host is always a fiddle.
AIUI, in general there are uids and gids defined in the container, some (typically, most) with corresponding user and group names; and there are uids and gids similarly on the host. None of these need to overlap. When the image is built, I create the builder(1000):builder(1000) user and that will be the user by which commands are executed by default.
At this point, the script is running on the host so should (IMO) be creating files/directories using the default host's uid:gid (ie., no -o or -g switches to install).
When we docker run then ideally we would create files using the uid:gid of the user on the host. But the CLI mechanism for doing that (AIUI) is to use the numeric IDs by passing in --user $(id -u):$(id -g). Unfortunately, that may prevent use of sudo inside the container because there is a good chance that the uid that ends up being used will not exist in /etc/passwd in the container and so sudo will refuse to run.
Perhaps a compromise would be to leave the uid in the container as-is (ie., builder(1000)) but to add something like --group-add $(id -G | sed 's/ / --group-add /g')" to the docker run ... command line -- assuming directories are created with g+w then at least file creation from inside the container will work while still letting us use sudo inside the container...?
(Unfortunately I can't test any of this as D4M does magic to make the mappings work behind my back -- on Mac/Windows it's extra complicated because there are effectively three layers of namespace involved: the actual host (Mac/Windows), the Linux VM, container.)
There was a problem hiding this comment.
(Or we could just give up using bind mounts at all, and use named volumes for everything. But extracting built packages becomes trickier then. Maybe that doesn't matter though?)
| if [ -f "$HOME/.gitconfig" ]; then | ||
| ABUILD_VOLUMES="$ABUILD_VOLUMES -v $HOME/.gitconfig:/home/builder/.gitconfig" | ||
| fi | ||
|
|
There was a problem hiding this comment.
LGTM. (Didn't realise abuild-keygen used git and needed .gitconfig...)
|
I think for packages it makes much more sense to bind mount them. It's the end product which you want to access easily. There are also less security concerns compared to I've been thinking to add an applet (symlink) to dabuild to add some additional functions like edit abuild.conf if it was too move into volumes. dabuild-admin conf would run vi in the container to edit the config. |
|
Merged in 0c11a9e by @clandmeter. Thanks for your contribution! (This pull request has been closed automatically by GitHub PR Closer. If you think that it’s not resolved yet, please add a comment.) |
|
Actually i think we should still add some permissions fixed in the entrypoint to make sure our mount points have correct permissions on first run. ie package dir, distfiles dir and any other dir that gets mounted for the first time. |
No description provided.