Re: [libvirt PATCH 3/4] ci: Expose CI_USER_LOGIN as a Makefile variable for users to use
On Fri, 2021-02-12 at 13:41 +0100, Erik Skultety wrote:
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 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.
It's just a code smell. In general, I prefer straightforward
constructs to "fancy" ones, especially in languages where quoting and
the like matter so much.
But I agree with you that it's safe in this specific scenario, so if
you'd rather keep it this way I won't NACK the patch because of that.
--
Andrea Bolognani / Red Hat / Virtualization