On Wed, 2021-02-10 at 18:00 +0100, Erik Skultety wrote:
# We need the container process to run with current host IDs # so that it can access the passed in build directory -CI_UID = $(shell id -u) -CI_GID = $(shell id -g) +CI_UID = $(shell id -u $(CI_USER_LOGIN)) +CI_GID = $(shell id -g $(CI_USER_LOGIN))
Please quote uses of $(CI_USER_LOGIN) in the shell. Also, since you're using the variable here, it would be IMHO cleaner if you moved the block that contains its definition before this one.
# We also need the user's login and home directory to prepare the # environment the way some programs expect it -CI_USER_LOGIN = $(shell echo "$$USER") -CI_USER_HOME = $(shell echo "$$HOME") +CI_USER_LOGIN = $(shell whoami) +CI_USER_HOME = $(shell eval echo "~$(CI_USER_LOGIN)")
This use of eval makes me a bit uncomfortable. Can we do CI_USER_HOME = $(shell getent passwd "$(CI_USER_LOGIN)" | cut -d: -f6) instead?
+ # In case we want to debug in the container, having root is actually + # preferable, so reset the CI_PODMAN_ARGS and don't actually perform + # any uid/gid mapping + ifeq ($(CI_UID), 0) + CI_PODMAN_ARGS= + endif
Setting $(CI_PODMAN_ARGS) only to reset it moments later seems worse than just doing ifneq ($(CI_UID, 0) CI_PODMAN_ARGS = \ ... $(NULL) endif The comment is also somewhat misleading: whether or not you're going to debug in the container, whatever that means, is not really relevant - the point is simply that performing these mappings is only necessary when the container process is running as non-root.
@echo " CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion" @echo " CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content" + @echo " CI_USER_LOGIN= - which user should run in the container (default is $$USER)" @echo " CI_ENGINE=auto - container engine to use (podman, docker)" @echo " CI_MESON_ARGS= - extra arguments passed to meson" @echo " CI_NINJA_ARGS= - extra arguments passed to ninja"
We document the default, so this should be CI_USER_LOGIN=$$USER - which user should run in the container And please document this after CI_ENGINE. -- Andrea Bolognani / Red Hat / Virtualization