Re: [PATCH] OSSTEST: introduce a raisin build test

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-05-06 at 12:16 +0100, Stefano Stabellini wrote:
> On Wed, 6 May 2015, Ian Campbell wrote:
> > On Tue, 2015-05-05 at 18:47 +0100, Stefano Stabellini wrote:
> > > > I was suggesting ts-raisin-build could contain just 1. a call to raise
> > > > to list all the relevant trees 2. an iteration over the output,
> > > > copying information to/from runvars, 3. an exception table to allow
> > > > for situations like 'QEMU' ne uc 'qemuu' and 'QEMU_TRADITIONAL' ne uc
> > > > 'qemuu'.
> > > 
> > > raisin does not have a list of trees, if not in the form of a default
> > > config file provided for users' convenience. Theoretically
> > > ts-raisin-build could use the default config and grep _URL
> > > raisin/defconfig, but I think it would be uglier than the current
> > > approach.
> > > 
> > > Keep in mind that with raisin there is no hidden git cloning of external
> > > trees. Not at all. All the trees are specified in the config file and
> > > cloned explicitly. Actually I think that it is nice and natural for
> > > ts-raisin-build to specify a list of URLs, because ts-raisin-build also
> > > wants to specify which components raisin is supposed to build and there
> > > is a 1:1 relationship.
> > 
> > I think a simple mapping from "osstest tree names" to "raisin
> > components", (or vice versa, but the rest of the mail assumes the
> > former) ought to be all which is needed here.
> > 
> > Essentially you want to iterate over the runvars looks for things which
> > match tree_(.*) then (modulo proper Perl syntax):
> > 
> >         my $tree = $1;
> >      my $comp = $osstest2raisin{$tree}
> >         push @components, $comp;
> >         
> >         echo >> config "uc($comp)_URL=$r{tree_${tree}}"
> >         echo >> config "uc($comp)_REVISION=$r{revision_${tree}}"
> > 
> > Finally
> >         my $comps = join(", ", @components);
> >         
> >         echo >> config "COMPONENTS=$comps"
> > 
> > I'm assuming that in raisin the component name is consistently also the
> > first part of the _URL and _REVISION config options, if not then %
> > osstest2raisin would need to be a hash exploding to the various required
> > names.
> 
> Not a problem, names are consistent in raisin.
> But is that much better than

This omits generating $ENABLED_COMPONENTS from the set of things which
osstest asked for.

Doing that is only incrementally more complex with what I suggested
(i.e. fits in nicely), but much more for the version below, (i.e. I
think it would basically look like what I suggested anyway).

>     echo >>config XEN_URL=\\"$r{tree_xen}\\"
>     echo >>config QEMU_URL=\\"$r{tree_qemuu}\\"
>     echo >>config QEMU_TRADITIONAL_URL=\\"$r{tree_qemu}\\"
>     echo >>config SEABIOS_URL=\\"$r{tree_seabios}\\"
>     echo >>config LIBVIRT_URL=\\"$r{tree_libvirt}\\"
>     echo >>config OVMF_URL=\\"$r{tree_ovmf}\\"
> 
>     echo >>config XEN_REVISION=\\"$r{revision_xen}\\"
>     echo >>config QEMU_REVISION=\\"$r{revision_qemuu}\\"
>     echo >>config QEMU_TRADITIONAL_REVISION=\\"$r{revision_qemu}\\"
>     echo >>config SEABIOS_REVISION=\\"$r{revision_seabios}\\"
>     echo >>config LIBVIRT_REVISION=\\"$r{revision_libvirt}\\"
>     echo >>config OVMF_REVISION=\\"$r{revision_ovmf}\\"
> 
> ? This is comparable in lines of code, and much more obvious to the
> reader.
> 
> Also it wouldn't fetch the urls from raisin as Ian suggested. That bit
> is the part that I would prefer to avoid.

I think with the scheme I proposed osstest always picks the precise set
of components and their URL so this wouldn't be necessary.

What Ian suggested only becomes necessary if raisin can clone/build
something which wasn't specified explicitly, which I think you said
wasn't the case? I think that relies on $ENABLED_COMPONENTS accurately
reflecting the input runvars?

What happens if ENABLED_COMPONENTS contains e.g. libvirt but
LIBVIRT_{URL,REVISION} are not specified in the config?

Ian.