Re: [Xen-devel] [PATCH] OSSTEST: introduce a raisin build test

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx>
> > 
> > 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?

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