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
