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

From: Ian Jackson <ian.jackson@eu.citrix.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org
Subject: Re: [PATCH 5/7] osstest: introduce a FreeBSD build script
Date: Tue, 23 May 2017 18:58:59 +0100	[thread overview]
Message-ID: <22820.30819.409970.197915@mariner.uk.xensource.com> (raw)
In-Reply-To: <20170523135148.77673-6-roger.pau@citrix.com>

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.

> +    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 ?

> +    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.

> +# 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.

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

Ian.

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