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