Re: [Xen-devel] [OSSTEST PATCH v2 09/19] ts-debian-fixup: merge origin extra= to our own

2018-03-15 Thread Wei Liu
On Wed, Mar 07, 2018 at 03:06:19PM +, Ian Jackson wrote:
> Wei Liu writes ("[OSSTEST PATCH v2 09/19] ts-debian-fixup: merge origin 
> extra= to our own"):
> > The original extra= was not removed, so there were two extra= in the
> > resulting config file.
> > 
> > It wasn't a problem for xl because the second extra= took precedence.
> > However libvirt tests would only pick up the first extra= --  they
> > worked by chance.
> ...
> > -$cfg .= "\nextra='$extra'\n";
> > +$cfg =~ s/^extra\s*=\s*['"](.*)['"]/extra = '$1 $extra'/mg;
> 
> Isn't this a no-op if there is no extra= already ?

Right. We will need to test if there is already extra=.

> 
> Also you use s///g but AFAICT multiple extra would already be an error
> of some kind and editing them all doesn't seem to make sense...
> 

Yeah, removing the 'g' should be fine.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [OSSTEST PATCH v2 09/19] ts-debian-fixup: merge origin extra= to our own

2018-03-07 Thread Ian Jackson
Wei Liu writes ("[OSSTEST PATCH v2 09/19] ts-debian-fixup: merge origin extra= 
to our own"):
> The original extra= was not removed, so there were two extra= in the
> resulting config file.
> 
> It wasn't a problem for xl because the second extra= took precedence.
> However libvirt tests would only pick up the first extra= --  they
> worked by chance.
...
> -$cfg .= "\nextra='$extra'\n";
> +$cfg =~ s/^extra\s*=\s*['"](.*)['"]/extra = '$1 $extra'/mg;

Isn't this a no-op if there is no extra= already ?

Also you use s///g but AFAICT multiple extra would already be an error
of some kind and editing them all doesn't seem to make sense...

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel