* [PATCH v7 0/2] OSSTEST: introduce a raisin build test
@ 2015-06-22 15:15 Stefano Stabellini
2015-06-22 15:16 ` [PATCH v7 1/2] " Stefano Stabellini
2015-06-22 15:16 ` [PATCH v7 2/2] OSSTest: push successful raisin builds Stefano Stabellini
0 siblings, 2 replies; 10+ messages in thread
From: Stefano Stabellini @ 2015-06-22 15:15 UTC (permalink / raw)
To: xen-devel; +Cc: wei.liu2, ian.jackson, ian.campbell, stefano.stabellini
Hi all,
the first patch introduces a Raisin build job, meant to test Raisin
itself. The second patch push-gates Raisin on successful builds.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Changes in v7:
- update raisin git url after http://marc.info/?l=xen-devel&m=143472251602803
- add a patch to push-gate raisin
Changes in v6:
- move divide to Osstest/BuildSupport.pm and rename it to
divide_xen_build; use divide_xen_build in ts-raisin-build and
ts-xen-build.
Changes in v5:
- remove setting DEFAULT_REVISION_RAISIN to master
- disable raisin when branch is not xen*
- do not overwrite raisin default variables unless both revision_* and
tree_* are set of a given component
Changes in v4:
- introduce enable_raisin in mfi-common: only build raisin when building
xen-unstable
- start off from the default raisin config, then append osstest config
options to it
- do not write variable to the raisin config if the conrresponding
runvar is not set
- remove TREE_OVMF and TREE_SEABIOS
Changes in v3:
- use $raisindir throughout ts-raisin-build
- do not specify ENABLED_COMPONENTS so that empty REVISION variables can
be used to disable building a raisin component
Changes in v2:
- set revision_* variables in mfi-common;
- in ts-raisin-build set the *_REVISION config options based on the
revision_* variables;
- in ts-raisin-build, call store_revision appropriately;
- divide the output in an hypervisor and a tools tarball.
Stefano Stabellini (2):
OSSTEST: introduce a raisin build test
OSSTest: push successful raisin builds
Osstest/BuildSupport.pm | 48 +++++++++++++++++
ap-common | 5 ++
ap-fetch-version | 3 ++
ap-fetch-version-old | 5 ++
ap-print-url | 3 ++
ap-push | 5 ++
cr-daily-branch | 8 +++
cri-common | 1 +
mfi-common | 41 +++++++++++++++
sg-run-job | 5 ++
ts-raisin-build | 133 +++++++++++++++++++++++++++++++++++++++++++++++
ts-xen-build | 44 +---------------
12 files changed, 259 insertions(+), 42 deletions(-)
create mode 100755 ts-raisin-build
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v7 1/2] OSSTEST: introduce a raisin build test 2015-06-22 15:15 [PATCH v7 0/2] OSSTEST: introduce a raisin build test Stefano Stabellini @ 2015-06-22 15:16 ` Stefano Stabellini 2015-07-09 15:33 ` Ian Campbell 2015-06-22 15:16 ` [PATCH v7 2/2] OSSTest: push successful raisin builds Stefano Stabellini 1 sibling, 1 reply; 10+ messages in thread From: Stefano Stabellini @ 2015-06-22 15:16 UTC (permalink / raw) To: xen-devel; +Cc: wei.liu2, ian.jackson, ian.campbell, stefano.stabellini Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- Changes in v7: - update raisin git url after http://marc.info/?l=xen-devel&m=143472251602803 Changes in v6: - move divide to Osstest/BuildSupport.pm and rename it to divide_xen_build; use divide_xen_build in ts-raisin-build and ts-xen-build. Changes in v5: - remove setting DEFAULT_REVISION_RAISIN to master - disable raisin when branch is not xen* - do not overwrite raisin default variables unless both revision_* and tree_* are set of a given component Changes in v4: - introduce enable_raisin in mfi-common: only build raisin when building xen-unstable - start off from the default raisin config, then append osstest config options to it - do not write variable to the raisin config if the conrresponding runvar is not set - remove TREE_OVMF and TREE_SEABIOS Changes in v3: - use $raisindir throughout ts-raisin-build - do not specify ENABLED_COMPONENTS so that empty REVISION variables can be used to disable building a raisin component Changes in v2: - set revision_* variables in mfi-common; - in ts-raisin-build set the *_REVISION config options based on the revision_* variables; - in ts-raisin-build, call store_revision appropriately; - divide the output in an hypervisor and a tools tarball. --- Osstest/BuildSupport.pm | 48 +++++++++++++++++ ap-common | 2 + mfi-common | 41 +++++++++++++++ sg-run-job | 5 ++ ts-raisin-build | 133 +++++++++++++++++++++++++++++++++++++++++++++++ ts-xen-build | 44 +--------------- 6 files changed, 231 insertions(+), 42 deletions(-) create mode 100755 ts-raisin-build diff --git a/Osstest/BuildSupport.pm b/Osstest/BuildSupport.pm index 933f6e1..76a3feb 100644 --- a/Osstest/BuildSupport.pm +++ b/Osstest/BuildSupport.pm @@ -43,6 +43,10 @@ BEGIN { xendist $xendist + divide_xen_build + trapping + die_if_fail + submodulefixup submodule_have submodule_find ); @@ -84,6 +88,50 @@ sub xendist () { ($ho, 'xendist', '', $r{"buildjob"}); } +sub divide_xen_build ($$) { + my ($subdir, $postfix) = @_; + # Only move hv to xeninstall, so that we can have + # xenpolicy in tools tarball. + # + # The files inside boot/ after `make dist' are + # xen-$XEN_VERSION: Xen binary + # xen.gz/xen: symlink to xen-$XEN_VERSION + # xen-$MAJOR: symlink to xen-$XEN_VERSION + # xen-$MAJOR.$MINOR: symlink to xen-$XEN_VERSION + # xen-sym-$XEN_VERSION: Xen symbol + # xenpolicy-$XEN_VERSION: flask policy binary if xsm is enabled + # + # So the following snippet will leave xenpolicy* in + # install/boot and get packaged to tools tarball. + target_cmd_build($ho, 100, $builddir, <<END); + cd $subdir + mkdir xen$postfix + for f in *$postfix; do + mkdir -p \$f/lib + done + if test -d $postfix/boot; then + if test -f $postfix/boot/xen.gz || test -f $postfix/boot/xen; then + mkdir xen$postfix/boot + mvfiles=`find $postfix/boot -name 'xen[a-z]*' -prune -o -name 'xen*' -print` + mv \$mvfiles xen$postfix/boot/. + fi + fi +END +} + +our @probs; + +sub trapping ($) { + my ($sub) = @_; + my $tok= eval { $sub->(); 1; }; + if (!$tok) { push @probs, $@; print STDERR "failure (trapped): $@\n"; } +} + +sub die_if_fail () { + die "*** something failed:\n\n".(join "\n\n",@probs)."\n** something failed" + if @probs; +} + #----- submodules ----- sub submodulefixup ($$$$) { diff --git a/ap-common b/ap-common index 64749e3..6e1c472 100644 --- a/ap-common +++ b/ap-common @@ -47,6 +47,8 @@ # 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/raisin.git} + : ${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} diff --git a/mfi-common b/mfi-common index 16fc8c5..d75823d 100644 --- a/mfi-common +++ b/mfi-common @@ -148,6 +148,22 @@ create_build_jobs () { *) enable_ovmf=true; esac + case "$xenbranch" in + xen-3.*-testing) enable_raisin=false;; + xen-4.0-testing) enable_raisin=false;; + xen-4.1-testing) enable_raisin=false;; + xen-4.2-testing) enable_raisin=false;; + xen-4.3-testing) enable_raisin=false;; + xen-4.4-testing) enable_raisin=false;; + xen-4.5-testing) enable_raisin=false;; + *) enable_raisin=true; + esac + case "$branch" in + xen*) ;; + *) enable_raisin=false;; + esac + + eval " arch_runvars=\"\$ARCH_RUNVARS_$arch\" " @@ -215,6 +231,31 @@ create_build_jobs () { fi + if [ "x$REVISION_RAISIN" != xdisable ] && [ "$enable_raisin" = "true" ]; 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_xen=$REVISION_XEN \ + revision_qemu=$REVISION_QEMU \ + revision_qemuu=$REVISION_QEMU_UPSTREAM \ + revision_seabios=$REVISION_SEABIOS \ + revision_ovmf=$REVISION_OVMF \ + revision_libvirt=$REVISION_LIBVIRT \ + 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..2a81f97 --- /dev/null +++ b/ts-raisin-build @@ -0,0 +1,133 @@ +#!/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(); + +my $raisindir="$builddir/raisin"; + +sub checkout () { + prepbuilddirs(); + build_clone($ho, 'raisin', $builddir, 'raisin'); + + target_cmd_build($ho, 100, $builddir, <<END. + cd $raisindir + cp defconfig config + >>config + + echo >>config MAKE=\\"make $makeflags\\" + echo >>config PREFIX=\\"/usr\\" + echo >>config DESTDIR=dist + echo >>config ENABLED_COMPONENTS=\\"seabios ovmf xen qemu qemu_traditional libvirt\\" +END + (nonempty($r{tree_xen}) && nonempty($r{revision_xen}) ? <<END : ''). + echo >>config XEN_URL=\\"$r{tree_xen}\\" + echo >>config XEN_REVISION=\\"$r{revision_xen}\\" +END + (nonempty($r{tree_qemuu}) && nonempty($r{revision_qemuu}) ? <<END : ''). + echo >>config QEMU_URL=\\"$r{tree_qemuu}\\" + echo >>config QEMU_REVISION=\\"$r{revision_qemuu}\\" +END + (nonempty($r{tree_qemu}) && nonempty($r{revision_qemu}) ? <<END : ''). + echo >>config QEMU_TRADITIONAL_URL=\\"$r{tree_qemu}\\" + echo >>config QEMU_TRADITIONAL_REVISION=\\"$r{revision_qemu}\\" +END + (nonempty($r{tree_seabios}) && nonempty($r{revision_seabios}) ? <<END : ''). + echo >>config SEABIOS_URL=\\"$r{tree_seabios}\\" + echo >>config SEABIOS_REVISION=\\"$r{revision_seabios}\\" +END + (nonempty($r{tree_libvirt}) && nonempty($r{revision_libvirt}) ? <<END : ''). + echo >>config LIBVIRT_URL=\\"$r{tree_libvirt}\\" + echo >>config LIBVIRT_REVISION=\\"$r{revision_libvirt}\\" +END + (nonempty($r{tree_ovmf}) && nonempty($r{revision_ovmf}) ? <<END : '') + echo >>config OVMF_URL=\\"$r{tree_ovmf}\\" + echo >>config OVMF_REVISION=\\"$r{revision_ovmf}\\" +END + ); +} + +sub build () { + target_cmd_root($ho, <<END); + cd $raisindir + ./raise -y install-builddep +END +#/; + target_cmd_build($ho, 9000, $builddir, <<END); + cd $raisindir + ./raise -n build +END +#/; +} + +sub checkoutput () { + target_cmd_build($ho, 100, $builddir, <<END); + cd $raisindir/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 +#/; +} + +sub collectversions () { + store_revision($ho, 'xen', "$raisindir/xen-dir", 1); + store_revision($ho, 'qemu', "$raisindir/qemu-traditional-dir", 1); + store_revision($ho, 'qemuu', "$raisindir/qemu-dir", 1); + store_revision($ho, 'seabios', "$raisindir/seabios-dir", 1); + store_revision($ho, 'ovmf', "$raisindir/ovmf-dir", 1); + store_revision($ho, 'libvirt', "$raisindir/libvirt-dir", 1); +} + +sub stash () { + foreach my $part ('', 'xen') { + built_stash($ho, $builddir, + "raisin/${part}dist", + "${part}dist"); + } + built_stash_file($ho, $builddir, "xen-syms", "raisin/xen-dir/xen/xen-syms", 1); + built_stash_file($ho, $builddir, "raisin-config", "raisin/config", 1); + built_stash_file($ho, $builddir, "seabios-config", + "raisin/seabios-dir/.config", 1); + built_compress_stashed("xen-syms"); +} + + +checkout(); +trapping(\&build); +trapping(\&checkoutput); +trapping(\&collectversions); +divide_xen_build('raisin', 'dist'); +stash(); +die_if_fail(); + diff --git a/ts-xen-build b/ts-xen-build index e757f68..10fc95b 100755 --- a/ts-xen-build +++ b/ts-xen-build @@ -145,36 +145,6 @@ sub collectversions () { store_revision($ho, 'ovmf', "$tools/firmware/ovmf-dir", 1); } -sub divide () { - # Only move hv to xeninstall, so that we can have - # xenpolicy in tools tarball. - # - # The files inside boot/ after `make dist' are - # xen-$XEN_VERSION: Xen binary - # xen.gz/xen: symlink to xen-$XEN_VERSION - # xen-$MAJOR: symlink to xen-$XEN_VERSION - # xen-$MAJOR.$MINOR: symlink to xen-$XEN_VERSION - # xen-sym-$XEN_VERSION: Xen symbol - # xenpolicy-$XEN_VERSION: flask policy binary if xsm is enabled - # - # So the following snippet will leave xenpolicy* in - # install/boot and get packaged to tools tarball. - target_cmd_build($ho, 100, $builddir, <<END); - cd xen/dist - mkdir xeninstall - for f in *install; do - mkdir -p \$f/lib - done - if test -d install/boot; then - if test -f install/boot/xen.gz || test -f install/boot/xen; then - mkdir xeninstall/boot - mvfiles=`find install/boot -name 'xen[a-z]*' -prune -o -name 'xen*' -print` - mv \$mvfiles xeninstall/boot/. - fi - fi -END -} - sub stash () { foreach my $part ('', 'xen') { built_stash($ho, $builddir, @@ -212,22 +182,12 @@ END $chk->finish(); } -our @probs; - -sub trapping ($) { - my ($sub) = @_; - my $tok= eval { $sub->(); 1; }; - if (!$tok) { push @probs, $@; print STDERR "failure (trapped): $@\n"; } -} - checkout(); trapping(\&build); trapping(\&collectversions); +die_if_fail(); -die "*** something failed:\n\n".(join "\n\n",@probs)."\n** something failed" - if @probs; - -divide(); +divide_xen_build('xen/dist', 'install'); stash(); checkversions(); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/2] OSSTEST: introduce a raisin build test 2015-06-22 15:16 ` [PATCH v7 1/2] " Stefano Stabellini @ 2015-07-09 15:33 ` Ian Campbell 2015-07-20 12:56 ` Stefano Stabellini 0 siblings, 1 reply; 10+ messages in thread From: Ian Campbell @ 2015-07-09 15:33 UTC (permalink / raw) To: Stefano Stabellini; +Cc: wei.liu2, ian.jackson, xen-devel On Mon, 2015-06-22 at 16:16 +0100, Stefano Stabellini wrote: > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> In general I'm very suspicious of a 200 line patch with _no_ commit message at all. What about the details of the stuff around overriding the versions iff tree and revision are set etc and why that matters and how the two projects therefore interact wrt selecting what to build? I remember discussing this at length either IRL or in older versions of the thread and we both know it wasn't trivial to arrive at how this should be done, but I _already_ can't remember how we reached this point or why and that is information which really needs to be recorded so it can be referred to later when we wonder why it does this. The scope is also missing, i.e. this is currently just a straight raisin build test, and the results are not consumed by anything. This is important because of all the stuff about splitting up the dist dir and incremental builds which we discussed means that as it stands this test step is not producing build artefacts suitable as an input for actual tests. I expect there's also stuff in the intra-version changelog which actually belongs in the main changelog, such as why you are refactoring certain things into BuildSupport.pm, which deserves some sort of brief mention IMHO. [...] > Changes in v3: > - use $raisindir throughout ts-raisin-build > - do not specify ENABLED_COMPONENTS so that empty REVISION variables can > be used to disable building a raisin component I see we do again in the code below, which I suspect was deliberate and based on some discussion, but it's not mentioned in the Changes in v4+ and since the changelog doesn't explain anything I can't even begin to guess why or how we arrived at this point. [...] > +sub build () { > + target_cmd_root($ho, <<END); > + cd $raisindir > + ./raise -y install-builddep If two of these happen to run in parallel (build machines can be shared) then one or the other risks timing out on the underlying dpkg lock waiting for the other, since the wait might be arbitrarily long depending on what is going on. It also risks other builds happening in an environment which differs from one build to the next (if this happened to run in the middle) or even things changing while a build is happening. This is why all build dependencies are installed from ts-xen-build-prep, that step is run once for each build host as it is installed. This does unfortunately mean that I think we can't take advantage of raisin's tracking of necessary build dependencies, but at least it will be checking things for us. Ian. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/2] OSSTEST: introduce a raisin build test 2015-07-09 15:33 ` Ian Campbell @ 2015-07-20 12:56 ` Stefano Stabellini 2015-07-20 13:20 ` Ian Campbell 0 siblings, 1 reply; 10+ messages in thread From: Stefano Stabellini @ 2015-07-20 12:56 UTC (permalink / raw) To: Ian Campbell; +Cc: wei.liu2, xen-devel, ian.jackson, Stefano Stabellini On Thu, 9 Jul 2015, Ian Campbell wrote: > On Mon, 2015-06-22 at 16:16 +0100, Stefano Stabellini wrote: > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > In general I'm very suspicious of a 200 line patch with _no_ commit > message at all. > > What about the details of the stuff around overriding the versions iff > tree and revision are set etc and why that matters and how the two > projects therefore interact wrt selecting what to build? I remember > discussing this at length either IRL or in older versions of the thread > and we both know it wasn't trivial to arrive at how this should be done, > but I _already_ can't remember how we reached this point or why and that > is information which really needs to be recorded so it can be referred > to later when we wonder why it does this. > > The scope is also missing, i.e. this is currently just a straight raisin > build test, and the results are not consumed by anything. This is > important because of all the stuff about splitting up the dist dir and > incremental builds which we discussed means that as it stands this test > step is not producing build artefacts suitable as an input for actual > tests. > > I expect there's also stuff in the intra-version changelog which > actually belongs in the main changelog, such as why you are refactoring > certain things into BuildSupport.pm, which deserves some sort of brief > mention IMHO. > > [...] I didn't do a good job with the commit message. I'll try to track down the old discussion and write some more details. > > Changes in v3: > > - use $raisindir throughout ts-raisin-build > > - do not specify ENABLED_COMPONENTS so that empty REVISION variables can > > be used to disable building a raisin component > > I see we do again in the code below, which I suspect was deliberate and > based on some discussion, but it's not mentioned in the Changes in v4+ > and since the changelog doesn't explain anything I can't even begin to > guess why or how we arrived at this point. > > [...] > > +sub build () { > > + target_cmd_root($ho, <<END); > > + cd $raisindir > > + ./raise -y install-builddep > > If two of these happen to run in parallel (build machines can be shared) > then one or the other risks timing out on the underlying dpkg lock > waiting for the other, since the wait might be arbitrarily long > depending on what is going on. > > It also risks other builds happening in an environment which differs > from one build to the next (if this happened to run in the middle) or > even things changing while a build is happening. > > This is why all build dependencies are installed from ts-xen-build-prep, > that step is run once for each build host as it is installed. This does > unfortunately mean that I think we can't take advantage of raisin's > tracking of necessary build dependencies, but at least it will be > checking things for us. It is fine for OSSTest not use install-builddep. I think that the best way for OSSTest to use Raisin would be to call raise in ts-xen-build-prep to list the dependencies and install them from there. But here we are testing Raisin itself so I think that in this context it is actually important to run install-builddep. I don't think it would cause any troubles to other builds happening in parallel because the build dependencies of those builds would be installed by ts-xen-build-prep beforehand, that I was told is run before any build jobs? If I am wrong, then we have a problem. Otherwise is there a way to guarantee that only one raisin build happen simultaneously? Given that the series is already at v7, I would prefer to remove the call to install-builddep, even though I think it is important, rather than make substantial modifications. However, if I do remove the call to install-builddep from ts-raisin-build, I would have to install any missing build dependencies somewhere else. Where would that be? In ts-xen-build-prep? Is that script even called for non-xen build jobs? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/2] OSSTEST: introduce a raisin build test 2015-07-20 12:56 ` Stefano Stabellini @ 2015-07-20 13:20 ` Ian Campbell 2015-07-20 13:55 ` Ian Jackson 0 siblings, 1 reply; 10+ messages in thread From: Ian Campbell @ 2015-07-20 13:20 UTC (permalink / raw) To: Stefano Stabellini, ian.jackson; +Cc: wei.liu2, xen-devel Ian -- various questions for you here. On Mon, 2015-07-20 at 13:56 +0100, Stefano Stabellini wrote: > On Thu, 9 Jul 2015, Ian Campbell wrote: > > On Mon, 2015-06-22 at 16:16 +0100, Stefano Stabellini wrote: > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > [...] > > I didn't do a good job with the commit message. I'll try to track down > the old discussion and write some more details. Thanks! > > > Changes in v3: > > > - use $raisindir throughout ts-raisin-build > > > - do not specify ENABLED_COMPONENTS so that empty REVISION variables can > > > be used to disable building a raisin component > > > > I see we do again in the code below, which I suspect was deliberate and > > based on some discussion, but it's not mentioned in the Changes in v4+ > > and since the changelog doesn't explain anything I can't even begin to > > guess why or how we arrived at this point. > > > > [...] > > > +sub build () { > > > + target_cmd_root($ho, <<END); > > > + cd $raisindir > > > + ./raise -y install-builddep > > > > If two of these happen to run in parallel (build machines can be shared) > > then one or the other risks timing out on the underlying dpkg lock > > waiting for the other, since the wait might be arbitrarily long > > depending on what is going on. > > > > It also risks other builds happening in an environment which differs > > from one build to the next (if this happened to run in the middle) or > > even things changing while a build is happening. > > > > This is why all build dependencies are installed from ts-xen-build-prep, > > that step is run once for each build host as it is installed. This does > > unfortunately mean that I think we can't take advantage of raisin's > > tracking of necessary build dependencies, but at least it will be > > checking things for us. > > It is fine for OSSTest not use install-builddep. I think that the best > way for OSSTest to use Raisin would be to call raise in > ts-xen-build-prep to list the dependencies and install them from there. Unfortunately ts-xen-build-prep will not have access to the repo, since it won't be cloned. Also when reusing a host (which happens for builds) I think this step is skipped (Ian J: can you confirm or deny that?) > But here we are testing Raisin itself so I think that in this context it > is actually important to run install-builddep. I don't think it would > cause any troubles to other builds happening in parallel because the > build dependencies of those builds would be installed by > ts-xen-build-prep beforehand, that I was told is run before any build > jobs? If I am wrong, then we have a problem. I think there are two problems, one is that install-builddep will take the apt/dpkg lock, which will be shared by any other invocations -- making the timeout hard to decide upon (since it will pipeline things). Second if install-builddep did install something extra due to some change in raisin.git, for example, then some jobs would see new build deps arrive half way through, or subsequent tests of older branches might see something installed which wouldn't have been before. Is there a check-builddep? That's the thing I would suggest we run here, so we get an explicit failure. > Otherwise is there a way to guarantee that only one raisin build happen > simultaneously? Ian? I suspect we don't want this, > > Given that the series is already at v7, I would prefer to remove the > call to install-builddep, even though I think it is important, rather > than make substantial modifications. However, if I do remove the call to > install-builddep from ts-raisin-build, I would have to install any > missing build dependencies somewhere else. Where would that be? In > ts-xen-build-prep? Is that script even called for non-xen build jobs? In ts-xen-build-prep, I think. Ian. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 1/2] OSSTEST: introduce a raisin build test 2015-07-20 13:20 ` Ian Campbell @ 2015-07-20 13:55 ` Ian Jackson 0 siblings, 0 replies; 10+ messages in thread From: Ian Jackson @ 2015-07-20 13:55 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel, wei.liu2, Stefano Stabellini Ian Campbell writes ("Re: [PATCH v7 1/2] OSSTEST: introduce a raisin build test"): > Unfortunately ts-xen-build-prep will not have access to the repo, since > it won't be cloned. Also when reusing a host (which happens for builds) > I think this step is skipped (Ian J: can you confirm or deny that?) Indeed. It is ts-xen-build-prep which marks a shared build host as `ready', when it is done. Near the top of ts-xen-build-prep (and ts-host-install) you can see exit 0 if $ho->{SharedReady}; > I think there are two problems, one is that install-builddep will take > the apt/dpkg lock, which will be shared by any other invocations -- > making the timeout hard to decide upon (since it will pipeline things). Worse, unless we took special measures (like target_install_packages does) we would be relying on the underlying apt/dpkg lock, which apt and dpkg try to lock in a non-waiting mode: that is, if the lock is already held they simply fail. > Second if install-builddep did install something extra due to some > change in raisin.git, for example, then some jobs would see new build > deps arrive half way through, or subsequent tests of older branches > might see something installed which wouldn't have been before. Indeed, that would be bad. > Is there a check-builddep? That's the thing I would suggest we run here, > so we get an explicit failure. That would be best. > > Otherwise is there a way to guarantee that only one raisin build happen > > simultaneously? > > Ian? It would be possible to completely avoid sharing the build host. That would be quite wasteful - particularly as the raisin build is likely to be able to otherwise share not just the effort of installing the host, but also a ccache. > > Given that the series is already at v7, I would prefer to remove the > > call to install-builddep, even though I think it is important, rather > > than make substantial modifications. However, if I do remove the call to > > install-builddep from ts-raisin-build, I would have to install any > > missing build dependencies somewhere else. Where would that be? In > > ts-xen-build-prep? Is that script even called for non-xen build jobs? > > In ts-xen-build-prep, I think. ts-xen-build-prep is slightly misnamed. It is run for all build jobs. (Or, relied on as having been run, for a shared host.) Ian. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v7 2/2] OSSTest: push successful raisin builds 2015-06-22 15:15 [PATCH v7 0/2] OSSTEST: introduce a raisin build test Stefano Stabellini 2015-06-22 15:16 ` [PATCH v7 1/2] " Stefano Stabellini @ 2015-06-22 15:16 ` Stefano Stabellini 2015-07-09 15:39 ` Ian Campbell 1 sibling, 1 reply; 10+ messages in thread From: Stefano Stabellini @ 2015-06-22 15:16 UTC (permalink / raw) To: xen-devel; +Cc: wei.liu2, ian.jackson, ian.campbell, stefano.stabellini Determine the most recent raisin revision that needs to be tested, by comparing with the already tested xen-tested-master branch. Push to raisin.git:xen-tested-master when the build is successful. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- ap-common | 3 +++ ap-fetch-version | 3 +++ ap-fetch-version-old | 5 +++++ ap-print-url | 3 +++ ap-push | 5 +++++ cr-daily-branch | 8 ++++++++ cri-common | 1 + 7 files changed, 28 insertions(+) diff --git a/ap-common b/ap-common index 6e1c472..34613bf 100644 --- a/ap-common +++ b/ap-common @@ -48,6 +48,8 @@ # (ie ones which need $bodges=1 in ts-rumpuserxen-build) : ${TREE_RAISIN:=git://xenbits.xen.org/raisin.git} +: ${PUSH_TREE_RAISIN:=$XENBITS:/home/xen/git/raisin.git} +: ${BASE_TREE_RAISIN:=$TREE_RAISIN} : ${TREE_SEABIOS_UPSTREAM:=git://git.seabios.org/seabios.git} : ${PUSH_TREE_SEABIOS:=$XENBITS:/home/xen/git/osstest/seabios.git} @@ -84,6 +86,7 @@ fi : ${LOCALREV_RUMPUSERXEN:=daily-cron.$branch} : ${LOCALREV_SEABIOS:=daily-cron.$branch} : ${LOCALREV_OVMF:=daily-cron.$branch} +: ${LOCALREV_RAISIN:=daily-cron.$branch} : ${TREEBASE_LINUX_XCP:=http://hg.uk.xensource.com/carbon/trunk/linux-2.6.27} diff --git a/ap-fetch-version b/ap-fetch-version index 33aaf00..fffbbcf 100755 --- a/ap-fetch-version +++ b/ap-fetch-version @@ -80,6 +80,9 @@ libvirt) rumpuserxen) repo_tree_rev_fetch_git rumpuserxen \ $TREE_RUMPUSERXEN master $LOCALREV_RUMPUSERXEN +raisin) + repo_tree_rev_fetch_git raisin \ + $TREE_RAISIN master $LOCALREV_RAISIN ;; seabios) repo_tree_rev_fetch_git seabios \ diff --git a/ap-fetch-version-old b/ap-fetch-version-old index 0b4739b..9a2544d 100755 --- a/ap-fetch-version-old +++ b/ap-fetch-version-old @@ -29,6 +29,7 @@ select_xenbranch : ${BASE_LOCALREV_LINUX:=daily-cron.$branch.old} : ${BASE_LOCALREV_LIBVIRT:=daily-cron.$branch.old} : ${BASE_LOCALREV_RUMPUSERXEN:=daily-cron.$branch.old} +: ${BASE_LOCALREV_RAISIN:=daily-cron.$branch.old} : ${BASE_LOCALREV_SEABIOS:=daily-cron.$branch.old} : ${BASE_LOCALREV_OVMF:=daily-cron.$branch.old} @@ -89,6 +90,10 @@ rumpuserxen) repo_tree_rev_fetch_git rumpuserxen \ $BASE_TREE_RUMPUSERXEN xen-tested-master $BASE_LOCALREV_RUMPUSERXEN ;; +raisin) + repo_tree_rev_fetch_git raisin \ + $BASE_TREE_RAISIN xen-tested-master $BASE_LOCALREV_RAISIN + ;; seabios) repo_tree_rev_fetch_git seabios \ $BASE_TREE_SEABIOS xen-tested-master $BASE_LOCALREV_SEABIOS diff --git a/ap-print-url b/ap-print-url index 3db2375..e999b65 100755 --- a/ap-print-url +++ b/ap-print-url @@ -55,6 +55,9 @@ libvirt) rumpuserxen) echo $TREE_RUMPUSERXEN ;; +raisin) + echo $TREE_RAISIN + ;; seabios) echo $TREE_SEABIOS_UPSTREAM ;; diff --git a/ap-push b/ap-push index c141464..c869a8b 100755 --- a/ap-push +++ b/ap-push @@ -37,6 +37,7 @@ TREE_LIBVIRT=$PUSH_TREE_LIBVIRT TREE_RUMPUSERXEN=$PUSH_TREE_RUMPUSERXEN TREE_SEABIOS=$PUSH_TREE_SEABIOS TREE_OVMF=$PUSH_TREE_OVMF +TREE_RAISIN=$PUSH_TREE_RAISIN if info_linux_tree "$branch"; then cd $repos/linux @@ -89,6 +90,10 @@ rumpuserxen) cd $repos/rumpuserxen git push $TREE_RUMPUSERXEN $revision:xen-tested-master ;; +raisin) + cd $repos/raisin + git push $TREE_RAISIN $revision:xen-tested-master + ;; seabios) cd $repos/seabios git push $TREE_SEABIOS $revision:refs/heads/xen-tested-master diff --git a/cr-daily-branch b/cr-daily-branch index c7d1071..0b7bb78 100755 --- a/cr-daily-branch +++ b/cr-daily-branch @@ -164,6 +164,10 @@ if [ "x$REVISION_RUMPUSERXEN" = x ]; then determine_version REVISION_RUMPUSERXEN rumpuserxen RUMPUSERXEN export REVISION_RUMPUSERXEN fi +if [ "x$REVISION_RAISIN" = x ]; then + determine_version REVISION_RAISIN raisin RAISIN + export REVISION_RAISIN +fi if [ "x$REVISION_LINUXFIRMWARE" = x ]; then determine_version REVISION_LINUXFIRMWARE linuxfirmware LINUXFIRMWARE export REVISION_LINUXFIRMWARE @@ -209,6 +213,10 @@ rumpuserxen) export REVISION_LINUX_OLD=disable export REVISION_LIBVIRT=disable ;; +raisin) + realtree=raisin + NEW_REVISION=$REVISION_RAISIN + ;; seabios) realtree=seabios NEW_REVISION=$REVISION_SEABIOS diff --git a/cri-common b/cri-common index ad44546..c091622 100644 --- a/cri-common +++ b/cri-common @@ -70,6 +70,7 @@ select_xenbranch () { linuxfirmware) tree=linuxfirmware; xenbranch=xen-unstable ;; libvirt) tree=libvirt; xenbranch=xen-unstable ;; rumpuserxen) tree=rumpuserxen; xenbranch=xen-unstable ;; + raisin) tree=raisin; xenbranch=xen-unstable ;; seabios) tree=seabios; xenbranch=xen-unstable ;; ovmf) tree=ovmf; xenbranch=xen-unstable ;; osstest) tree=osstest; xenbranch=xen-unstable ;; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v7 2/2] OSSTest: push successful raisin builds 2015-06-22 15:16 ` [PATCH v7 2/2] OSSTest: push successful raisin builds Stefano Stabellini @ 2015-07-09 15:39 ` Ian Campbell 2015-07-20 13:41 ` Stefano Stabellini 0 siblings, 1 reply; 10+ messages in thread From: Ian Campbell @ 2015-07-09 15:39 UTC (permalink / raw) To: Stefano Stabellini; +Cc: wei.liu2, ian.jackson, xen-devel On Mon, 2015-06-22 at 16:16 +0100, Stefano Stabellini wrote: > Determine the most recent raisin revision that needs to be tested, by comparing > with the already tested xen-tested-master branch. Push to > raisin.git:xen-tested-master when the build is successful. The mechanics here look right to me, but do you perhaps want to adopt the staging->master push gate scheme used in xen.git etc? This would make master (the default thing which users use) be something which has undergone at least some amount of testing. We typically use xen-tested-master for 3rd party things where the output of the test gate is mainly just for osstest's bookkeeping and not really aimed at actually users. Oh, one thing you missed is adding raisin to the default $BRANCHES at the top of cr-for-branches, without that there will be no raisin flights run and therefore no push. You might also want to considering all non-raisin build and test jobs from the raisin flight, since they are a waste of time until the raisin build results are actually used for something. You can filter tests in make-flight:job_create_test_filter_callback and for jobs you will need my "mfi-common: Allow make-*flight to filter the set of build jobs to include" patch which adds job_create_build_filter_callback. Ian. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 2/2] OSSTest: push successful raisin builds 2015-07-09 15:39 ` Ian Campbell @ 2015-07-20 13:41 ` Stefano Stabellini 2015-07-20 13:46 ` Ian Campbell 0 siblings, 1 reply; 10+ messages in thread From: Stefano Stabellini @ 2015-07-20 13:41 UTC (permalink / raw) To: Ian Campbell; +Cc: wei.liu2, xen-devel, ian.jackson, Stefano Stabellini On Thu, 9 Jul 2015, Ian Campbell wrote: > On Mon, 2015-06-22 at 16:16 +0100, Stefano Stabellini wrote: > > Determine the most recent raisin revision that needs to be tested, by comparing > > with the already tested xen-tested-master branch. Push to > > raisin.git:xen-tested-master when the build is successful. > > The mechanics here look right to me, but do you perhaps want to adopt > the staging->master push gate scheme used in xen.git etc? This would > make master (the default thing which users use) be something which has > undergone at least some amount of testing. We typically use > xen-tested-master for 3rd party things where the output of the test gate > is mainly just for osstest's bookkeeping and not really aimed at > actually users. OK > Oh, one thing you missed is adding raisin to the default $BRANCHES at > the top of cr-for-branches, without that there will be no raisin flights > run and therefore no push. OK > You might also want to considering all non-raisin build and test jobs > from the raisin flight, since they are a waste of time until the raisin > build results are actually used for something. You can filter tests in > make-flight:job_create_test_filter_callback and for jobs you will need > my "mfi-common: Allow make-*flight to filter the set of build jobs to > include" patch which adds job_create_build_filter_callback. I filtered all jobs in make-flight (there are no jobs with raisin yet). We can add a filter on the builds later? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v7 2/2] OSSTest: push successful raisin builds 2015-07-20 13:41 ` Stefano Stabellini @ 2015-07-20 13:46 ` Ian Campbell 0 siblings, 0 replies; 10+ messages in thread From: Ian Campbell @ 2015-07-20 13:46 UTC (permalink / raw) To: Stefano Stabellini; +Cc: wei.liu2, ian.jackson, xen-devel On Mon, 2015-07-20 at 14:41 +0100, Stefano Stabellini wrote: > On Thu, 9 Jul 2015, Ian Campbell wrote: > > On Mon, 2015-06-22 at 16:16 +0100, Stefano Stabellini wrote: > > > Determine the most recent raisin revision that needs to be tested, by comparing > > > with the already tested xen-tested-master branch. Push to > > > raisin.git:xen-tested-master when the build is successful. > > > > The mechanics here look right to me, but do you perhaps want to adopt > > the staging->master push gate scheme used in xen.git etc? This would > > make master (the default thing which users use) be something which has > > undergone at least some amount of testing. We typically use > > xen-tested-master for 3rd party things where the output of the test gate > > is mainly just for osstest's bookkeeping and not really aimed at > > actually users. > > OK > > > > Oh, one thing you missed is adding raisin to the default $BRANCHES at > > the top of cr-for-branches, without that there will be no raisin flights > > run and therefore no push. > > OK > > > > You might also want to considering all non-raisin build and test jobs > > from the raisin flight, since they are a waste of time until the raisin > > build results are actually used for something. You can filter tests in > > make-flight:job_create_test_filter_callback and for jobs you will need > > my "mfi-common: Allow make-*flight to filter the set of build jobs to > > include" patch which adds job_create_build_filter_callback. > > I filtered all jobs in make-flight (there are no jobs with raisin yet). > We can add a filter on the builds later? Sure. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-07-20 13:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-06-22 15:15 [PATCH v7 0/2] OSSTEST: introduce a raisin build test Stefano Stabellini 2015-06-22 15:16 ` [PATCH v7 1/2] " Stefano Stabellini 2015-07-09 15:33 ` Ian Campbell 2015-07-20 12:56 ` Stefano Stabellini 2015-07-20 13:20 ` Ian Campbell 2015-07-20 13:55 ` Ian Jackson 2015-06-22 15:16 ` [PATCH v7 2/2] OSSTest: push successful raisin builds Stefano Stabellini 2015-07-09 15:39 ` Ian Campbell 2015-07-20 13:41 ` Stefano Stabellini 2015-07-20 13:46 ` Ian Campbell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.