Re: [PATCH 5/7] osstest: introduce a FreeBSD build script

From: Roger Pau Monne <roger.pau@citrix.com>
To: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: xen-devel@lists.xenproject.org
Subject: Re: [PATCH 5/7] osstest: introduce a FreeBSD build script
Date: Wed, 24 May 2017 15:15:20 +0100	[thread overview]
Message-ID: <20170524141520.5pegnb6dhc4cl5zl@MacBook-Pro-de-Roger.local> (raw)
In-Reply-To: <22820.30819.409970.197915@mariner.uk.xensource.com>

On Tue, May 23, 2017 at 06:58:59PM +0100, Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH 5/7] osstest: introduce a FreeBSD build script"):
> > -    my $path= "path_${part}dist";
> > -    logm("checking $k $path");
> > -    get_stashed($path, $r{$k});
> > +    if ("$part" eq "freebsd") {
> > +        foreach (qw(base kernel manifest image)) {
> > +            my $path= "path_${part}-$_";
> > +            logm("checking $k $path");
> > +            get_stashed($path, $r{$k});
> > +        }
> > +    } else {
> > +        my $path= "path_${part}dist";
> > +        logm("checking $k $path");
> > +        get_stashed($path, $r{$k});
> 
> This is quite ugly.  I don't think this knowledge should be in
> ts-build-check.
> 
> ...
> 
> I think in fact that the right answer is probably to have
> ts-build-check be more general somehow.
> 
> I have looked through the history of ts-{,xen-}build-check and I think
> the current approach is a historical accident.  In the beginning it
> was a wrapper around ts-xen-install which used a special check flag;
> then that gradually generalised to what we have now - but it still
> retains the same origins.
> 
> I was going to suggest checking the job status, but might be an
> inconvenience during by-hand testing.
> 
> I considered having sg-run-job specify the parts it's going to use, as
> command line arguments, with plumbing in sg-run-job from the recipe,
> but that's still going to have to be buildjob-runvar-specific.
> 
> I considered controlling this via runvars from make-flight:
>    freebsdbuildjob=391031.build-amd64-freebsd
>    freebsdbuildjobpaths=-base,-kernel,-manifest,-image
> But it's also quite ugly.
> 
> I have a radical suggestion:
> 
> Suppose we have ts-freebsd-build set
>     path_freebsddist=$stash/build/freebsd/
> and have it put the files in there with fixed, known, names
>     path_freebsddist=$stash/build/freebsd/image
>     path_freebsddist=$stash/build/freebsd/manifest
>     path_freebsddist=$stash/build/freebsd/kernel.sets
>     path_freebsddist=$stash/build/freebsd/base.sets
> or something ?
> 
> Is there a reason why that wouldn't work ?
> 
> The stashing process would have to take care to set the runvar only
> after it had created all the files.

That seems fine, and then osstest would rely on the fact that
path_freebsddist must only be set when all the files have been
uploaded, because ts-build-check itself won't check that the files are
there anymore.

> > +    target_cmd_build($ho, 7200, $builddir, <<END);
> > +cd freebsd
> > +export MAKEOBJDIRPREFIX=$builddir/obj
> > +(make $makeflags TARGET=$r{arch} buildworld 2>&1 && touch ../build-ok-stamp) \\
> > +    |tee ../log
> 
> How about using Osstest::BuildSupport::buildcmd_stamped_logged ?

That seems suitable, I've used this rune because it's what
ts-kernel-build was using, now I realize ts-xen-build is indeed using
buildcmd_stamped_logged.

> > +    logm("Creating the install sets");
> > +    # NB: the steps below need to be done as root or the permissions
> > +    # of the files won't be properly set (and the target will fail).
> > +    target_cmd_root($ho, <<END, 900);
> 
> target_cmd_root does not imply set -e; only target_cmd_build does.
> 
> You may want to invent buildcmd_stamped_logged_root or something.

Yes, that sounds right.

> 
> > +# Enable DHCP on all network interfaces
> > +echo 'ifconfig_DEFAULT="DHCP"' >> \$target/etc/rc.conf
> 
> Is this wise ?  We may at some point have hosts which have two network
> interfaces connected (perhaps to the test network, or to each other,
> or something) in which case this is probably wrong.

This just means that on the installer all the network interfaces will
try to get a DHCP address. This is for the installer image itself, the
installed system will only setup DHCP on the primary interface (ie:
the one that matches the IP address at $ho->{Ip}.

> There are a lot of \.  I wonder if you might find
> <<'ENDQ'.<<END.<<'ENDQ' a useful construct.

In fact I can define two perl variables and use them instead. There's
really no reason they have to be shell variables ($target and $output).

Roger.

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