| Message ID | 1398681696-2773-11-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | New |
| Headers | show |
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. (As were the "" before. Not sure why
they were there.)
> + $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) ?
> - 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/
> + 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.
Ian.
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.
Ian Campbell writes ("Re: [PATCH OSSTEST v2 11/15] distros: attempt pvgrub support for PV Wheezy too."):
> On Fri, 2014-05-02 at 14:11 +0100, Ian Jackson wrote:
> > (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?
Yes, but "" is the same as no quoting. Saying <<'EOF' prevents
variable interpolation.
> > > - 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?)
It makes it easier to add |jessie when the problem remains unfixed.
(Also if someone has some kind of derivative called "wheezy-blarf", it
is more likely to be right than the exact match.)
> > > + 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.
Thanks,
Ian.
On Fri, 2014-05-02 at 15:19 +0100, Ian Jackson wrote: > Ian Campbell writes ("Re: [PATCH OSSTEST v2 11/15] distros: attempt pvgrub support for PV Wheezy too."): > > On Fri, 2014-05-02 at 14:11 +0100, Ian Jackson wrote: > > > (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? > > Yes, but "" is the same as no quoting. Saying <<'EOF' prevents > variable interpolation. Ah, "" is the same as no quoting is what I forget. > > > > - 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?) > > It makes it easier to add |jessie when the problem remains unfixed. > (Also if someone has some kind of derivative called "wheezy-blarf", it > is more likely to be right than the exact match.) Lets hope we never have a legitimate release which is a super/substring of another ;-) (at least bo and other dead short ones are long ago).
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); d-i mirror/suite string $suite d-i debian-installer/locale string en_GB @@ -501,6 +501,14 @@ $xopts{ExtraPreseed} ### END OF DEBIAN PRESEED BASE END + + # deb http://ftp.debian.org/debian/ wheezy-backports main + $preseed .= <<END if $xopts{EnableBackports}; +d-i apt-setup/local0/repository string http://$c{DebianMirrorHost}/$c{DebianMirrorSubpath} $suite-backports main +d-i apt-setup/local0/comment string $suite backports +END + + return $preseed; } sub preseed_create_guest ($$;@) { @@ -508,7 +516,11 @@ sub preseed_create_guest ($$;@) { my $suite= $xopts{Suite} || $c{DebianSuite}; - my $extra_packages = "pv-grub-menu" if $xopts{PvMenuLst}; + $xopts{EnableBackports} = 1 + if $xopts{PvMenuLst} and $suite eq "wheezy"; + my $extra_packages = "pv-grub-menu". + ($xopts{EnableBackports} ? "/$suite-backports" : "") + if $xopts{PvMenuLst}; my $preseed_file= preseed_base($suite, $extra_packages, %xopts); $preseed_file.= (<<END); diff --git a/make-distros-flight b/make-distros-flight index a6e41e0..244d582 100755 --- a/make-distros-flight +++ b/make-distros-flight @@ -69,9 +69,10 @@ test_do_one_netboot () { armhf_*) bootloader="pygrub";; # no pvgrub for arm # Needs a menu.lst, not present in Squeeze+ due to switch to grub2, - # workedaround in Jessie+ with pv-grub-menu package. + # workedaround in Wheezy+ with pv-grub-menu package (backports in Wheezy, + # in Jessie+ main). *_squeeze) bootloader="pygrub";; - *_wheezy) bootloader="pygrub";; + #*_wheezy) bootloader="pygrub";; esac job_create_test test-$xenarch$kern-$dom0arch-$domU-$dist-netboot \
This requires us to install pv-grub-menu from backports, however the version there seems to be buggy right now so it doesn't actually work. Introduce the test anyway, a Debian bug report will follow. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- Osstest/Debian.pm | 16 ++++++++++++++-- make-distros-flight | 5 +++-- 2 files changed, 17 insertions(+), 4 deletions(-)