Re: [Xen-devel] [PATCH v7 1/2] OSSTEST: introduce a raisin build test

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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

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