1289695 - Artifact builds should check autoland pushheads and/or artifact builds on infra should force-use their "own" build artifacts

Status: RESOLVED DUPLICATE of bug 1382982

(Firefox Build System :: General, defect)

Description

[:Gijs (he/him)]

STR:

1. land a change on autoland that changes nsIURI ( https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=8f1980fa5bee8fe994c59dbb494e8da561c1145c )
2. normal builds happy, artifact builds sad (get backed out!)
3. find this in log (https://public-artifacts.taskcluster.net/GpKpJS5CTMuWeWuawM5jSg/0/public/logs/live_backing.log) :

12:53:05     INFO -  Attempting to find a pushhead containing ff1ef8ec0fd800bf6856c1572c3b1610c45e9b6a on mozilla-central.
12:53:05     INFO -  Attempting to find a pushhead containing ff1ef8ec0fd800bf6856c1572c3b1610c45e9b6a on integration/fx-team.
12:53:05     INFO -  Attempting to find a pushhead containing ff1ef8ec0fd800bf6856c1572c3b1610c45e9b6a on integration/mozilla-inbound.
12:53:05     INFO -  Attempting to find a pushhead containing ff1ef8ec0fd800bf6856c1572c3b1610c45e9b6a on releases/mozilla-aurora.
12:53:06     INFO -  Retrieving the last 50 pushheads starting with id 79889 on integration/mozilla-inbound
12:53:06     INFO -  Retrieving the last 50 pushheads starting with id 20110 on integration/fx-team
12:53:06     INFO -  Retrieving the last 50 pushheads starting with id 30489 on mozilla-central
12:53:08     INFO -  Installing from remote pushhead ff1ef8ec0fd800bf6856c1572c3b1610c45e9b6a on mozilla-central

Uhhh. Yeah. That will make for a sad artifact build, because it compiles its own xpt files based on my modified nsIURI idl file, but then combines it with binary code that uses the old version of that idl file, and then tries to run that code and use nsIURI from JS, which presumably crashes somewhere in xpconnect land (I can't even get a JS exception out of it when using this locally on my linux64 artifact build system after manually pulling the rev back from try and running an artifact build on it, which does the same thing as this try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=87703c04dfb1&filter-searchStr=artifact ).

So any binary change that lands on autoland that changes any interface that gets used by the startup cache precompilation will break the build.

Any binary change that lands on autoland that changes any interface at all would likely crash and burn in tests if you tried to run any with it (which we don't do, yet, but AIUI we're considering using these in certain circumstances).

Similar problems occur on try.

AIUI we don't run artifact builds on infra until the "normal" build has completed, so presumably things in the fx-team/inbound/release/central trees will be happy because we find pushheads. So the trivial workaround is to also fetch pushheads from autoland. Alternatively, and perhaps more interestingly, we could try to enforce that this version of artifact builds on infra only ever use their "own" artifacts, which should be guaranteed to exist anyway.

Comment 1

[Mike Hommey [:glandium]]

Half of the issue is bug 1240134

Comment 2

[:Gijs (he/him)]

(In reply to Mike Hommey [:glandium] from comment #1)
> Half of the issue is bug 1240134

Well, maybe, but then you'd still end up with a frankenbuild where it'd be anybody's guess as to whether the artifact build "worked". If I changed an IDL file and the binary blobs I got had the old version of the IDL, all the JS would be broken for additions/changes to the IDL instead.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

I'll make a patch to check autoland for push heads, we should definitely be doing that. The artifact builds are Tier 2 in Treeherder, so they should never result in an automatic backout.

As long as getting binary artifacts from an "older" build is something we're supporting for people's local builds I think it's a reasonable thing to have happen in automation. I may pick up bug 1240134 this week, the patches there look further along than I thought.

Comment 4

[Chris Manchester (limited bugmail, email directly)]

(In reply to Chris Manchester (:chmanchester) from comment #3)
> I'll make a patch to check autoland for push heads, we should definitely be
> doing that. The artifact builds are Tier 2 in Treeherder, so they should
> never result in an automatic backout.

Well, reading up on this, I guess nobody is supposed to pull from the autoland repo, so this is only an issue for those automation builds. Considering this I don't think it's critical to get artifacts from there, so long as these are Tier 2. The obvious patch doesn't do the trick, because we look for public changesets in the artifact code, and the autoland changesets are draft.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Hmm, just ran into this again.  I sometimes pull from autoland when I need to rebase something on top of that, and it hasn't reached central yet.

Adding autoland to the list in artifacts.py seems to give the expected result.  Should I proceed to make that change?

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5)
> Hmm, just ran into this again.  I sometimes pull from autoland when I need
> to rebase something on top of that, and it hasn't reached central yet.
> 
> Adding autoland to the list in artifacts.py seems to give the expected
> result.  Should I proceed to make that change?

I really don't think we should do this; autolanded builds are, by definition, not yet concerned stable.  Hunting down crap changes that are unrelated to your push is frustrating enough without adding unstable bits to the mix.

I'll leave this for chmanchester to weigh in on.

Comment 7

[Chris Manchester (limited bugmail, email directly)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5)
> Hmm, just ran into this again.  I sometimes pull from autoland when I need
> to rebase something on top of that, and it hasn't reached central yet.
> 
> Adding autoland to the list in artifacts.py seems to give the expected
> result.  Should I proceed to make that change?

If that fixed the issue, then something else is going wrong here. In particular it probably means we have errant public changesets on autoland, because we start the artifact search based on a public changeset and changesets on autoland are supposed to be draft until they merge, as I alluded to in comment 4. In this case what I'd expect to happen is that the artifact code would end up with the most recent binaries from central, which is what the automation artifact builds on this branch do, and usually works fine.

In general the design of autoland is that we're not supposed to pull from it, but this only really works if it's merged very frequently, and I've had to do this to rebase as well.

Given all that we could probably just check for pushheads on autoland to work around to tripping over random public changesets there, but not without consideration, because querying extra trees to check for pushheads potentially adds overhead (on the order of seconds) to every artifact build.

Comment 8

[Gregory Szorc [:gps]]

This could also get fixed by the patches in bug 1382507.

Comment 9

[:Gijs (he/him)]

I think we've changed how we do artifact builds so maybe this is moot now? Or did the AB job just get hidden or something?

Comment 10

[Chris Manchester (limited bugmail, email directly)]

In bug 1382982 we made it so automation artifact builds will query the Autoland repository, which should fix this.