[libvirt PATCH 3/4] ci: Expose CI_USER_LOGIN as a Makefile variable for users to use

[Erik Skultety]

On Fri, Feb 12, 2021 at 01:27:42PM +0100, Andrea Bolognani wrote:
> 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.

Sure I can do that, I just didn't feel like moving around code to achieve the
same thing in a declarative language.

> 
> >  # 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

Can you elaborate what the problem is? The argument is even quoted so I
sincerely don't see a problem and find it much clearer than what you suggest.

Erik