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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux