Re: [libvirt PATCH 09/20] ci: build.sh: Join MESON_ARGS and MESON_OPTS

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

 



On Mon, Aug 21, 2023 at 05:17:06AM -0700, Andrea Bolognani wrote:
> Resurrecting this thread now what you've pushed some of the patches.
> 
> On Mon, Feb 06, 2023 at 02:53:06PM +0100, Erik Skultety wrote:
> > +# $MESON_ARGS correspond to meson's setup args, i.e. configure args. It's
> > +# populated either from a GitLab's job configuration or from command line as
> > +# `$ helper build --meson-configure-args=-Dopt1 -Dopt2` when run in a local
> > +# containerized environment
> 
> You need quotes around the value. As is, the shell will interpret
> '--meson-configure-args=-Dopt1' and '-Dopt2' as separate arguments
> and things will not work the way you expect them to.
> 
> > -meson setup build --werror -Dsystem=true $MESON_OPTS $MESON_ARGS || \
> > +MESON_ARGS="$MESON_ARGS $MESON_OPTS"
> > +
> > +meson setup build --werror -Dsystem=true $MESON_ARGS || \
> 
> This has inverted the priority of the two lists of arguments.
> 
> Before the change, an option (e.g. -Dfoo=enabled) could be added to
> $MESON_ARGS at the job level and it would override the same option
> (e.g. -Dfoo=disabled) defined as part of $MESON_OPTS in the container
> image. Now the option in the container image will always take
> precedence, which is undesirable.
> 

Good points. However, I'm already close to proposing something vastly different
from this, dropping most of these variables as they are to make the local
executions pretty much >95% compatible to what happens in GitLab and overriding
these shell variables simply isn't part of it because it only makes sense right
now, but I think with the changes I have it'll only make sense in interactive
container shell sessions in which case it's left to the developer to set and
pass the right options IMO.

Erik




[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