[PATCH] OSSTEST: introduce a raisin build test

[PATCH] OSSTEST: introduce a raisin build test

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 ap-common       |    5 +++
 mfi-common      |   19 ++++++++++
 sg-run-job      |    5 +++
 ts-raisin-build |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 133 insertions(+)
 create mode 100755 ts-raisin-build

diff --git a/ap-common b/ap-common
index 64749e3..985eeec 100644
--- a/ap-common
+++ b/ap-common
@@ -47,13 +47,18 @@
 # rumpsrc-related runvars needed only for old rumpuser-xen
 # (ie ones which need $bodges=1 in ts-rumpuserxen-build)
 
+: ${TREE_RAISIN:=git://xenbits.xen.org/people/sstabellini/raisin.git}
+: ${DEFAULT_REVISION_RAISIN:=master}
+
 : ${TREE_SEABIOS_UPSTREAM:=git://git.seabios.org/seabios.git}
 : ${PUSH_TREE_SEABIOS:=$XENBITS:/home/xen/git/osstest/seabios.git}
 : ${BASE_TREE_SEABIOS:=git://xenbits.xen.org/osstest/seabios.git}
+: ${TREE_SEABIOS:=$TREE_SEABIOS_UPSTREAM}
 
 : ${TREE_OVMF_UPSTREAM:=https://github.com/tianocore/edk2.git}
 : ${PUSH_TREE_OVMF:=$XENBITS:/home/xen/git/osstest/ovmf.git}
 : ${BASE_TREE_OVMF:=git://xenbits.xen.org/osstest/ovmf.git}
+: ${TREE_OVMF:=$TREE_OVMF_UPSTREAM}
 
 : ${TREE_LINUXFIRMWARE:=git://xenbits.xen.org/osstest/linux-firmware.git}
 : ${PUSH_TREE_LINUXFIRMWARE:=$XENBITS:/home/osstest/ext/linux-firmware.git}
diff --git a/mfi-common b/mfi-common
index 16fc8c5..051c9fc 100644
--- a/mfi-common
+++ b/mfi-common
@@ -215,6 +215,25 @@ create_build_jobs () {
 
     fi
 
+    if [ "x$REVISION_RAISIN" != xdisable ]; then
+
+    ./cs-job-create $flight build-$arch-raisin build-raisin                  \
+                arch=$arch                                                   \
+        tree_xen=$TREE_XEN                                                   \
+                $RUNVARS $BUILD_RUNVARS $BUILD_RAISIN_RUNVARS $arch_runvars \
+                $suite_runvars                                               \
+                host_hostflags=$build_hostflags                              \
+                buildjob=${bfi}build-$arch                                   \
+                tree_raisin=$TREE_RAISIN                                     \
+                tree_qemuu=$TREE_QEMU_UPSTREAM                               \
+                tree_qemu=$TREE_QEMU                                         \
+                tree_seabios=$TREE_SEABIOS                                   \
+                tree_libvirt=$TREE_LIBVIRT                                   \
+                tree_ovmf=$TREE_OVMF                                         \
+                revision_raisin=${REVISION_RAISIN:-${DEFAULT_REVISION_RAISIN}}\
+
+    fi
+
     if branch_wants_rumpkernel_tests; then
 
     case $arch in
diff --git a/sg-run-job b/sg-run-job
index eae159d..449118d 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -346,6 +346,7 @@ proc need-hosts/build {} { return BUILD }
 proc need-hosts/build-kern {} { return BUILD }
 proc need-hosts/build-libvirt {} { return BUILD }
 proc need-hosts/build-rumpuserxen {} { return BUILD }
+proc need-hosts/build-raisin {} { return BUILD }
 
 proc run-job/build {} {
     run-ts . = ts-xen-build
@@ -364,6 +365,10 @@ proc run-job/build-rumpuserxen {} {
     run-ts . = ts-xen-build + host tools
 }
 
+proc run-job/build-raisin {} {
+    run-ts . = ts-raisin-build
+}
+
 proc prepare-build-host {} {
     global jobinfo
     run-ts broken = ts-hosts-allocate + host
diff --git a/ts-raisin-build b/ts-raisin-build
new file mode 100755
index 0000000..f9b8ed6
--- /dev/null
+++ b/ts-raisin-build
@@ -0,0 +1,104 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2013 Citrix Inc.
+# 
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+# 
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+use strict qw(vars);
+use DBI;
+use Osstest;
+use File::Path;
+use POSIX;
+use Osstest::TestSupport;
+use Osstest::BuildSupport;
+
+tsreadconfig();
+selectbuildhost(\@ARGV);
+# remaining arguments are passed as targets to "make"
+builddirsprops();
+
+sub checkout () {
+    prepbuilddirs();
+    build_clone($ho, 'raisin', $builddir, 'raisin');
+
+    target_cmd_build($ho, 100, $builddir, <<END);
+        cd $builddir/raisin
+	>config
+
+    echo >>config ENABLED_COMPONENTS=\\"seabios ovmf xen qemu qemu_traditional libvirt\\"
+    echo >>config MAKE=\\"make $makeflags\\"
+    echo >>config PREFIX=\\"/usr\\"
+    echo >>config DESTDIR=dist
+
+    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=\\"RELEASE-4.5.0\\"
+    echo >>config QEMU_REVISION=\\"qemu-xen-4.5.0\\"
+    echo >>config QEMU_TRADITIONAL_REVISION=\\"xen-4.5.0-rc1\\"
+    echo >>config SEABIOS_REVISION=\\"rel-1.7.5\\"
+    echo >>config LIBVIRT_REVISION=\\"v1.2.14\\"
+    echo >>config OVMF_REVISION=\\"447d264115c476142f884af0be287622cd244423\\"
+END
+#/;
+}
+
+sub build () {
+    target_cmd_root($ho, <<END);
+	    cd $builddir/raisin
+        ./raise -y install-builddep
+END
+#/;
+    target_cmd_build($ho, 9000, $builddir, <<END);
+	    cd $builddir/raisin
+        ./raise -n build
+END
+#/;
+}
+
+sub checkoutput () {
+    target_cmd_build($ho, 100, $builddir, <<END);
+            cd $builddir/raisin/dist
+            ls boot/xen.gz
+            ls usr/sbin/xenstored
+            ls usr/sbin/xenconsoled
+            ls usr/lib/libxenlight.so
+            ls usr/sbin/xl
+            ls usr/lib/xen/boot/hvmloader
+            ls usr/lib/xen/bin/qemu-dm
+            ls usr/lib/xen/bin/qemu-system-i386
+            ls usr/sbin/libvirtd
+END
+#/;
+}
+
+our @probs;
+
+sub trapping ($) {
+    my ($sub) = @_;
+    my $tok= eval { $sub->(); 1; };
+    if (!$tok) { push @probs, $@; print STDERR "failure (trapped): $@\n"; }
+}
+
+checkout();
+trapping(\&build);
+trapping(\&checkoutput);
+
+die "*** something failed:\n\n".(join "\n\n",@probs)."\n** something failed"
+    if @probs;
+
-- 
1.7.10.4

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

[Ian Campbell]

On Fri, 2015-04-24 at 16:46 +0100, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

This looks like a good start, a few comments below.

> diff --git a/ap-common b/ap-common
> index 64749e3..985eeec 100644
> --- a/ap-common
> +++ b/ap-common
> @@ -47,13 +47,18 @@
>  # rumpsrc-related runvars needed only for old rumpuser-xen
>  # (ie ones which need $bodges=1 in ts-rumpuserxen-build)
>  
> +: ${TREE_RAISIN:=git://xenbits.xen.org/people/sstabellini/raisin.git}

I suppose we should move this to a non-people location sooner rather
than later.

> diff --git a/mfi-common b/mfi-common
> index 16fc8c5..051c9fc 100644
> --- a/mfi-common
> +++ b/mfi-common
> @@ -215,6 +215,25 @@ create_build_jobs () {
>  
>      fi
>  
> +    if [ "x$REVISION_RAISIN" != xdisable ]; then
> +
> +    ./cs-job-create $flight build-$arch-raisin build-raisin                  \
> +                arch=$arch                                                   \
> +        tree_xen=$TREE_XEN                                                   \
> +                $RUNVARS $BUILD_RUNVARS $BUILD_RAISIN_RUNVARS $arch_runvars \
> +                $suite_runvars                                               \
> +                host_hostflags=$build_hostflags                              \
> +                buildjob=${bfi}build-$arch                                   \
> +                tree_raisin=$TREE_RAISIN                                     \
> +                tree_qemuu=$TREE_QEMU_UPSTREAM                               \
> +                tree_qemu=$TREE_QEMU                                         \
> +                tree_seabios=$TREE_SEABIOS                                   \
> +                tree_libvirt=$TREE_LIBVIRT                                   \
> +                tree_ovmf=$TREE_OVMF                                         \
> +                revision_raisin=${REVISION_RAISIN:-${DEFAULT_REVISION_RAISIN}}\

I think you also need to pass revision_{xen,qemuu,qemu,libvirt,ovmf} etc
and propagate them when you create the raisin config file instead of
hardcoding a bunch of stuff in the ts- script.

You should also call store_revision() for each git repo which was built
so that it is recorded in the flight metadata. You should do this for
_every_ git repo, not just ones which are explicitly set to a specific
revision (osstest cares for bisection purposes what was built even if it
cannot control that input tree).

Lastly you will (eventually) need to divide the output into one or more
component subtrees (e.g. ts-xen-build splits the hypervisor from the
tools in order to support 32-on-64 configs) and call built_stash_file on
them. Those then produce the outputs which other jobs can consume.

Ian.

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

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH] OSSTEST: introduce a raisin build test"):
> On Fri, 2015-04-24 at 16:46 +0100, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> This looks like a good start, a few comments below.
...
> You should also call store_revision() for each git repo which was built
> so that it is recorded in the flight metadata. You should do this for
> _every_ git repo, not just ones which are explicitly set to a specific
> revision (osstest cares for bisection purposes what was built even if it
> cannot control that input tree).

For bisection, osstest needs to be able to specify the revision of
every repo.  That is, the ts-* script needs to honour appropriate
revision_* variables for all of them, as well as producing
built_revision_*.

It doesn't matter if cr-daily-branch and make-flight don't know how to
set those version runvars.  Bisection flights are constructed by using
existing flights as a template, and the revision runvars are
manipulated appropriately by the bisector.

But it is necessary that:
  * Every tree used[1] has the actual git revision used recorded
    by ts-raisin-build, in built_revision_* runvars.
  * Every tree used has a suitable url[2] specified in tree_*
    or recorded there by by ts-raisin-build.[3]
  * Any setting of revision_* is honoured by ts-raisin-build.

[1] Here a tree is `used' if the build success, or build products,
depends on it, even if the tree is cloned internally by raisin.

As a special exception, if the exact revision of some tree X is
specified by the contents of some other tree Y, it is acceptable
(although undesirable[1a]) for there to be no revision_X, tree_X, and
built_revision_X but only for _Y. [1b]

[1a] If the revision of X referred to in Y is updated in a
non-fast-forwarding way and where the common ancestor of X and X'
(referred to in a versions Y and Y') is likely to be unsuitable or
unuseable, then X should _not_ be mentioned or handled in osstest at
all.

[1b] If X and Y are coupled in the sense that semantic changes to X
are combined with an update to reference a new version of Y, in a
single commit, it may be desirable to update osstest's
adhoc-revtuple-generator to be able to fish the version of X out of a
Y tree.  (Currently it can only do this for the qemu-trad specified in
a xen.git tree.)

[2] The tree_X runvar must provide a URL which if cloned contains not
just the object referred to by revision_X (if specified) but also the
all previous versions (that is, the git objects referred to by
revision_X values in previous flights).  In practice this is satisfied
X is fast forwarding.

[3] It is not essential for tree_X to be set by make-flight and
honoured by ts-raisin-build; but if not it must be set by
ts-raisin-build because the bisector needs to know where to find the
tree so that it can examine its revision graph.


I'm not a huge fan of the way that your current patch hardcodes the
list of subtrees in ts-raisin-build.  I don't see any logical reason
why ts-raisin-build would need to know this, if raisin could be
persuaded to print it.

Thanks,
Ian.

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

[Stefano Stabellini]

On Tue, 5 May 2015, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH] OSSTEST: introduce a raisin build test"):
> > On Fri, 2015-04-24 at 16:46 +0100, Stefano Stabellini wrote:
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > This looks like a good start, a few comments below.
> ...
> > You should also call store_revision() for each git repo which was built
> > so that it is recorded in the flight metadata. You should do this for
> > _every_ git repo, not just ones which are explicitly set to a specific
> > revision (osstest cares for bisection purposes what was built even if it
> > cannot control that input tree).
> 
> For bisection, osstest needs to be able to specify the revision of
> every repo.  That is, the ts-* script needs to honour appropriate
> revision_* variables for all of them, as well as producing
> built_revision_*.
> 
> It doesn't matter if cr-daily-branch and make-flight don't know how to
> set those version runvars.  Bisection flights are constructed by using
> existing flights as a template, and the revision runvars are
> manipulated appropriately by the bisector.
> 
> But it is necessary that:
>   * Every tree used[1] has the actual git revision used recorded
>     by ts-raisin-build, in built_revision_* runvars.
>   * Every tree used has a suitable url[2] specified in tree_*
>     or recorded there by by ts-raisin-build.[3]
>   * Any setting of revision_* is honoured by ts-raisin-build.
> 
> [1] Here a tree is `used' if the build success, or build products,
> depends on it, even if the tree is cloned internally by raisin.
> 
> As a special exception, if the exact revision of some tree X is
> specified by the contents of some other tree Y, it is acceptable
> (although undesirable[1a]) for there to be no revision_X, tree_X, and
> built_revision_X but only for _Y. [1b]
> 
> [1a] If the revision of X referred to in Y is updated in a
> non-fast-forwarding way and where the common ancestor of X and X'
> (referred to in a versions Y and Y') is likely to be unsuitable or
> unuseable, then X should _not_ be mentioned or handled in osstest at
> all.
> 
> [1b] If X and Y are coupled in the sense that semantic changes to X
> are combined with an update to reference a new version of Y, in a
> single commit, it may be desirable to update osstest's
> adhoc-revtuple-generator to be able to fish the version of X out of a
> Y tree.  (Currently it can only do this for the qemu-trad specified in
> a xen.git tree.)
> 
> [2] The tree_X runvar must provide a URL which if cloned contains not
> just the object referred to by revision_X (if specified) but also the
> all previous versions (that is, the git objects referred to by
> revision_X values in previous flights).  In practice this is satisfied
> X is fast forwarding.
> 
> [3] It is not essential for tree_X to be set by make-flight and
> honoured by ts-raisin-build; but if not it must be set by
> ts-raisin-build because the bisector needs to know where to find the
> tree so that it can examine its revision graph.

I agree that revisions should be passed to ts-raisin-build by osstest,
as Ian suggested. AFAICT that should be enough to meet all your
criteria.


> I'm not a huge fan of the way that your current patch hardcodes the
> list of subtrees in ts-raisin-build.  I don't see any logical reason
> why ts-raisin-build would need to know this, if raisin could be
> persuaded to print it.

I am not sure I understand what you mean here. Yes, revisions are
hardcoded and that is a mistake, but trees are not.  This is a snippet
from ts-raisin-build:

    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}\\"

None of the trees are hardcoded, they are all passed to ts-raisin-build
by osstest using tree_* variables, as you suggested above.
What am I missing?

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

[Ian Jackson]

Stefano Stabellini writes ("Re: [PATCH] OSSTEST: introduce a raisin build test"):
> I agree that revisions should be passed to ts-raisin-build by osstest,
> as Ian suggested. AFAICT that should be enough to meet all your
> criteria.

You also need to call store_revision for all the trees involved, as
Ian C says.

> > I'm not a huge fan of the way that your current patch hardcodes the
> > list of subtrees in ts-raisin-build.  I don't see any logical reason
> > why ts-raisin-build would need to know this, if raisin could be
> > persuaded to print it.
> 
> I am not sure I understand what you mean here. Yes, revisions are
> hardcoded and that is a mistake, but trees are not.  This is a snippet
> from ts-raisin-build:
> 
>     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}\\"
> 
> None of the trees are hardcoded, they are all passed to ts-raisin-build
> by osstest using tree_* variables, as you suggested above.
> What am I missing?

The _list of trees_ is hardcoded.  That is, your ts-raisin-build
contains (an expanded form of) the list qw(xen qemuu qemu seabios
libvirt ovmf).

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

Ian.

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

[Stefano Stabellini]

On Tue, 5 May 2015, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [PATCH] OSSTEST: introduce a raisin build test"):
> > I agree that revisions should be passed to ts-raisin-build by osstest,
> > as Ian suggested. AFAICT that should be enough to meet all your
> > criteria.
> 
> You also need to call store_revision for all the trees involved, as
> Ian C says.

I am on it.


> > > I'm not a huge fan of the way that your current patch hardcodes the
> > > list of subtrees in ts-raisin-build.  I don't see any logical reason
> > > why ts-raisin-build would need to know this, if raisin could be
> > > persuaded to print it.
> > 
> > I am not sure I understand what you mean here. Yes, revisions are
> > hardcoded and that is a mistake, but trees are not.  This is a snippet
> > from ts-raisin-build:
> > 
> >     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}\\"
> > 
> > None of the trees are hardcoded, they are all passed to ts-raisin-build
> > by osstest using tree_* variables, as you suggested above.
> > What am I missing?
> 
> The _list of trees_ is hardcoded.  That is, your ts-raisin-build
> contains (an expanded form of) the list qw(xen qemuu qemu seabios
> libvirt ovmf).

I understand what you meant now, thanks.


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

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

[Stefano Stabellini]

On Tue, 5 May 2015, Ian Campbell wrote:
> On Fri, 2015-04-24 at 16:46 +0100, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> This looks like a good start, a few comments below.
> 
> > diff --git a/ap-common b/ap-common
> > index 64749e3..985eeec 100644
> > --- a/ap-common
> > +++ b/ap-common
> > @@ -47,13 +47,18 @@
> >  # rumpsrc-related runvars needed only for old rumpuser-xen
> >  # (ie ones which need $bodges=1 in ts-rumpuserxen-build)
> >  
> > +: ${TREE_RAISIN:=git://xenbits.xen.org/people/sstabellini/raisin.git}
> 
> I suppose we should move this to a non-people location sooner rather
> than later.

Sure


> > diff --git a/mfi-common b/mfi-common
> > index 16fc8c5..051c9fc 100644
> > --- a/mfi-common
> > +++ b/mfi-common
> > @@ -215,6 +215,25 @@ create_build_jobs () {
> >  
> >      fi
> >  
> > +    if [ "x$REVISION_RAISIN" != xdisable ]; then
> > +
> > +    ./cs-job-create $flight build-$arch-raisin build-raisin                  \
> > +                arch=$arch                                                   \
> > +        tree_xen=$TREE_XEN                                                   \
> > +                $RUNVARS $BUILD_RUNVARS $BUILD_RAISIN_RUNVARS $arch_runvars \
> > +                $suite_runvars                                               \
> > +                host_hostflags=$build_hostflags                              \
> > +                buildjob=${bfi}build-$arch                                   \
> > +                tree_raisin=$TREE_RAISIN                                     \
> > +                tree_qemuu=$TREE_QEMU_UPSTREAM                               \
> > +                tree_qemu=$TREE_QEMU                                         \
> > +                tree_seabios=$TREE_SEABIOS                                   \
> > +                tree_libvirt=$TREE_LIBVIRT                                   \
> > +                tree_ovmf=$TREE_OVMF                                         \
> > +                revision_raisin=${REVISION_RAISIN:-${DEFAULT_REVISION_RAISIN}}\
> 
> I think you also need to pass revision_{xen,qemuu,qemu,libvirt,ovmf} etc
> and propagate them when you create the raisin config file instead of
> hardcoding a bunch of stuff in the ts- script.

OK, makes sense


> You should also call store_revision() for each git repo which was built
> so that it is recorded in the flight metadata.

All right.


> You should do this for
> _every_ git repo, not just ones which are explicitly set to a specific
> revision (osstest cares for bisection purposes what was built even if it
> cannot control that input tree).

That's fine as there is no hidden git cloning with raisin. All the trees
are specified explicitly in the config file.


> Lastly you will (eventually) need to divide the output into one or more
> component subtrees (e.g. ts-xen-build splits the hypervisor from the
> tools in order to support 32-on-64 configs) and call built_stash_file on
> them. Those then produce the outputs which other jobs can consume.

Raisin has the capability of installing and configuring stuff on the
host. I guess osstest wouldn't want to reuse that?

Also how is the separation supposed to be done? Given that osstest
requested raisin to build a certain number of components together,
raisin would put them all in the same deb package. From what you wrote I
take that ts-raisin-build should operate differently, but how?

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

[Ian Campbell]

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.

If %osstest2raisin doesn't contain a mapping then that would be an error
I suppose, or you could add "// $tree" to the lookup to default to the
same name.

Ian.

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

[Ian Jackson]

Stefano Stabellini writes ("Re: [PATCH] OSSTEST: introduce a raisin build test"):
> That's fine as there is no hidden git cloning with raisin. All the trees
> are specified explicitly in the config file.

Is this a fundamental design principle ?

The rump kernel build system uses git submodules, which are (very
annoying and) a kind of hidden git cloning, and it also has a
[psuedo-submodule a bit like xen.git wrt qemu et al.

> > Lastly you will (eventually) need to divide the output into one or more
> > component subtrees (e.g. ts-xen-build splits the hypervisor from the
> > tools in order to support 32-on-64 configs) and call built_stash_file on
> > them. Those then produce the outputs which other jobs can consume.
> 
> Raisin has the capability of installing and configuring stuff on the
> host. I guess osstest wouldn't want to reuse that?

Probably not.

> Also how is the separation supposed to be done? Given that osstest
> requested raisin to build a certain number of components together,
> raisin would put them all in the same deb package. From what you wrote I
> take that ts-raisin-build should operate differently, but how?

Your ts-raisin-build could request building components separately, of
course, but I don't think that's sufficient unless your notion of a
`component' separates the Xen tools from the Xen hypervisor.

Here is an example use case, as done by osstest:

 - build Xen on amd64
 - split the hypervisor from the tools,
   producing an amd64 hv and amd64 tools

 - build Xen on i386
 - split the hypervisor (if any) from the tools,
   producing an i386 hv (in applicable Xen versions) and i386 tools

 - install a fresh i386 box
 - put the amd64 hv and the i386 tools on it
 - boot the result, producing a 32-on-64 dom0

Ian.

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

[Stefano Stabellini]

On Wed, 6 May 2015, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [PATCH] OSSTEST: introduce a raisin build test"):
> > That's fine as there is no hidden git cloning with raisin. All the trees
> > are specified explicitly in the config file.
> 
> Is this a fundamental design principle ?
> 
> The rump kernel build system uses git submodules, which are (very
> annoying and) a kind of hidden git cloning, and it also has a
> [psuedo-submodule a bit like xen.git wrt qemu et al.

Oh dear lord.

I am keeping not having hidden git cloning as a fundamental design
principle and goal of the project. I haven't tried to integrated rump
kernels into raisin yet, but as I told you IRL, I have been thinking of
writing the build system in a way that the git cloning is done by
raisin, even for rump kernels.


> > > Lastly you will (eventually) need to divide the output into one or more
> > > component subtrees (e.g. ts-xen-build splits the hypervisor from the
> > > tools in order to support 32-on-64 configs) and call built_stash_file on
> > > them. Those then produce the outputs which other jobs can consume.
> > 
> > Raisin has the capability of installing and configuring stuff on the
> > host. I guess osstest wouldn't want to reuse that?
> 
> Probably not.

I also agree that would not be best for osstest to use it.


> > Also how is the separation supposed to be done? Given that osstest
> > requested raisin to build a certain number of components together,
> > raisin would put them all in the same deb package. From what you wrote I
> > take that ts-raisin-build should operate differently, but how?
> 
> Your ts-raisin-build could request building components separately, of
> course, but I don't think that's sufficient unless your notion of a
> `component' separates the Xen tools from the Xen hypervisor.
> 
> Here is an example use case, as done by osstest:
> 
>  - build Xen on amd64
>  - split the hypervisor from the tools,
>    producing an amd64 hv and amd64 tools
> 
>  - build Xen on i386
>  - split the hypervisor (if any) from the tools,
>    producing an i386 hv (in applicable Xen versions) and i386 tools
> 
>  - install a fresh i386 box
>  - put the amd64 hv and the i386 tools on it
>  - boot the result, producing a 32-on-64 dom0

OK, I understand the split between xen hypervisor and tools. But what
about libvirt, grub2 and any other components that raisin can build?
It looks like we might want to put them all into the tools package.

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

[Stefano Stabellini]

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

    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.

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

[Ian Campbell]

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.

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

[Antti Kantee]

On 06/05/15 11:03, Stefano Stabellini wrote:
> On Wed, 6 May 2015, Ian Jackson wrote:
>> Stefano Stabellini writes ("Re: [PATCH] OSSTEST: introduce a raisin build test"):
>>> That's fine as there is no hidden git cloning with raisin. All the trees
>>> are specified explicitly in the config file.
>>
>> Is this a fundamental design principle ?
>>
>> The rump kernel build system uses git submodules, which are (very
>> annoying and) a kind of hidden git cloning, and it also has a
>> [psuedo-submodule a bit like xen.git wrt qemu et al.
>
> Oh dear lord.
>
> I am keeping not having hidden git cloning as a fundamental design
> principle and goal of the project. I haven't tried to integrated rump
> kernels into raisin yet, but as I told you IRL, I have been thinking of
> writing the build system in a way that the git cloning is done by
> raisin, even for rump kernels.

Well, these days it pulls the submodules in if they're not already 
present.  If you pull them in manually before running the build script, 
no hidden cloning happens.  Given that you want to pull them in manually 
anyway, I don't see a problem.

It's actually more of a historic remnant from times when checking out 
the necessary sources was a non-trivial operation involving separate 
user/kernel repositories and copying/patching sources all around.  I 
think it was fixed over a year ago.

I guess I could remove the "git submodule update" detection/execution 
from the build script entirely.  It's sort of illogical that you don't 
have to execute it after you clone initially but realistically speaking 
have to execute it after every pull -- makes us look like drug dealers.