Re: [Xen-devel] [PATCH OSSTEST v2 11/15] distros: attempt pvgrub support for PV Wheezy too.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-05-02 at 14:11 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v2 11/15] distros: attempt pvgrub 
> support for PV Wheezy too."):
> > diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> > index 8946ab1..ad478ab 100644
> > --- a/Osstest/Debian.pm
> > +++ b/Osstest/Debian.pm
> > @@ -432,7 +432,7 @@ sub preseed_base ($$;@) {
> >  
> >      $extra_packages ||= '';
> >  
> > -    return <<"END";
> > +    my $preseed= (<<END);
> 
> These parens are unnecessary.

OK

>   (As were the "" before.  Not sure why
> they were there.)

Does the quoting of the termination marker have an impact on the way
things within the heredoc are interpreted?

> 
> > +    $preseed .= <<END if $xopts{EnableBackports};
> > +d-i apt-setup/local0/repository string 
> > http://$c{DebianMirrorHost}/$c{DebianMirrorSubpath} $suite-backports main
> 
> Perhaps this wants \\ (to wrap it) ?

Sure.

> > -    my $extra_packages = "pv-grub-menu" if $xopts{PvMenuLst};
> > +    $xopts{EnableBackports} = 1
> > +   if $xopts{PvMenuLst} and $suite eq "wheezy";
> 
> I normally write this as   $suite =~ m/wheezy/

OK. (But OOI why?)

> 
> > +    my $extra_packages = "pv-grub-menu".
> > +   ($xopts{EnableBackports} ? "/$suite-backports" : "")
> > +       if $xopts{PvMenuLst};
> 
> I think this is a rather strange way of doing things.  It's
> backwards.  What you mean is probably something like this:
> 
>   +    my $extra_packages = "pv-grub-menu".
>   +   ($suite =~ m/wheezy/ ? "/$suite-backports" : "")
>   +       if $xopts{PvMenuLst};
> 
>   +    $xopts{EnableBackports} ||= $extra_packages =~ m#/\S+-backports\b#;
> 
> or even better, do away with the EnableBackports xopt altogether and
> have preseed_base do it itself automatically.

OK.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel