Closed Bug 910745 Opened 7 years ago Closed 6 years ago

When B2G trunk branches to mozilla-b2gNN manifest revisions should be pinned

Categories

(Firefox OS Graveyard :: GonkIntegration, defect, major)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: emorley, Assigned: aki)

References

Details

(Keywords: sheriffing-P1)

The fact that I even have to file a bug about this facepalm makes me very sad :-(

(In reply to Jonathan Griffin (:jgriffin) from bug 910662 comment #15)
> All of the "caf" repos are hosted at git://codeaurora.org/; I doubt you have
> the ability to back anything out of those.  :m4 is a good contact for those
> repos if you need help with them.

Note: Once we have bug 899969 this problem will be reduced (in that we'll only break one tree) but still exist - ie: 
* broken commit lands on third party repo
* bot updates on manifest on b2g-inbound
* sheriffs sees b2g-inbound is broken by something he/she cannot back out
-> sheriff files bug, closes b2g-inbound and ignores that tree for the day, inbetween having people moan that the tree is closed.

We should therefore do one of:
1) move these external repos to places where the sheriffs have access.
2) give sheriffs access to codeaurora/whatever other third party sad faces we're using.
3) make the bot not auto-update these third party repos (it would only update things under direct mozilla control) and instead a human has to test them and update the manifest on b2g-inbound themselves.

I would prefer #1 or #3.
Summary: B2G builds shouldn't directly pull in third party repos for which sheriffs have no ability to back things out → B2G builds shouldn't directly pull in third party repos from which sheriffs are unable to back things out
(In reply to Ed Morley [:edmorley UTC+1] from comment #0)
> 3) make the bot not auto-update these third party repos

"make the bot not auto-update the manifest.json [1] entries for these third party repos"


[1] bug 899969 comment 5
Depends on: 899969
This caused bug 1011958.

Let's go with #3.
Keywords: sheriffing-P1
Summary: B2G builds shouldn't directly pull in third party repos from which sheriffs are unable to back things out → Third party repositories listed in b2g-manifest should always reference a tag/revision
To clarify:

For each manifest listed at https://github.com/mozilla-b2g/b2g-manifest (eg: https://github.com/mozilla-b2g/b2g-manifest/blob/master/emulator.xml), we should make sure that any repository whose upstream is not on a Github Mozilla owned account has an explicit "revision=<rev_or_tag>". This will prevent us from indiscriminately pulling master from repos not under our control - repos which the sheriffs do not have commit access to perform a backout.

This will mean that devs making changes to third party repositories will need to both land their change in that repository and also update the manifest - however I don't believe this is too onerous a requirement to ensure that B2G builds conform with:
https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#8.29_Must_avoid_patterns_known_to_cause_non_deterministic_failures

Note: I'm explicitly not saying we should make these changes for repos owned by the Mozilla Github account, since sheriffs have access to those - though if others wish to do so for those too, I will not object.
Duplicate of this bug: 910685
I think you're missing

4) keep the manifest pointing to a branch; have the b2g_bumper bot keep updating the in-tree version.  builds/tests will use the in-tree version.  when bustage happens, we see that it's due to the in-tree manifest; if we can't do anything about the upstream repo, we can, at that point, lock down the revision while following up with the upstream repo owners.

Aiui that's how b2g_bumper is supposed to work, and the reason we implemented it for the sheriffs.

I don't think (2) is viable.

With our current solution (4), we keep up with the upstream repos.  I think this is highly desirable on master, and potentially when the next version is on Aurora.  I could agree with locking down to a revision when we move to a b2gXX_vX_X repo, but we can't do that by locking down (3) or forking (1) the mirror repo unless we also want to lock down/fork master.  I don't think we do.

I think this discussion should involve the developers as well.
(In reply to Aki Sasaki [:aki] from comment #5)
> I think you're missing
> 
> 4) keep the manifest pointing to a branch; have the b2g_bumper bot keep
> updating the in-tree version.  builds/tests will use the in-tree version. 
> when bustage happens, we see that it's due to the in-tree manifest; if we
> can't do anything about the upstream repo, we can, at that point, lock down
> the revision while following up with the upstream repo owners.
> 
> Aiui that's how b2g_bumper is supposed to work, and the reason we
> implemented it for the sheriffs.


I did realise about #4, since it is what caused the tree closure in bug 910685 and was the reason for filing this bug back in August and reviving it again now. I agree that there is a tradeoff, but I'm not convinced that we've made it in the correct direction at present.

> I don't think (2) is viable.

Comment 0 is slightly out of date, comment 3 has a more correct version of what is being proposed.

> With our current solution (4), we keep up with the upstream repos.  I think
> this is highly desirable on master

I guess we can see how frequently commits are made upstream to see how much hassle it would be to update the manifest each time? In addition we could make a report that on a regular basis emails people about upstream changes between the locked rev and master.

> I could agree with locking down to a revision when we move to a
> b2gXX_vX_X repo, 

That's bug 1012618 and IMO is a serious flaw at present, that we should fix asap.

> but we can't do that by locking down (3) or forking (1) the
> mirror repo unless we also want to lock down/fork master.  I don't think we
> do.

We just pin to a revision in the b2g-manifest files, when creating the new branch there each time, surely? (https://github.com/mozilla-b2g/b2g-manifest/branches)
(In reply to Ed Morley [:edmorley UTC+0] from comment #6)
> (In reply to Aki Sasaki [:aki] from comment #5)
> > I think you're missing
> > 
> > 4) keep the manifest pointing to a branch; have the b2g_bumper bot keep
> > updating the in-tree version.  builds/tests will use the in-tree version. 
> > when bustage happens, we see that it's due to the in-tree manifest; if we
> > can't do anything about the upstream repo, we can, at that point, lock down
> > the revision while following up with the upstream repo owners.
> > 
> > Aiui that's how b2g_bumper is supposed to work, and the reason we
> > implemented it for the sheriffs.
> 
> 
> I did realise about #4, since it is what caused the tree closure in bug
> 910685 and was the reason for filing this bug back in August and reviving it
> again now. I agree that there is a tradeoff, but I'm not convinced that
> we've made it in the correct direction at present.

I think we absolutely have made it in the correct direction on master/mozilla-central.
I agree we could lock it down more on branches closer to release, with the caveat that pushing new changes will be extra manual effort, and master/vX.X branches will diverge over time.

> > I don't think (2) is viable.
> 
> Comment 0 is slightly out of date, comment 3 has a more correct version of
> what is being proposed.

I still disagree that we should do this on the master branch.

> > With our current solution (4), we keep up with the upstream repos.  I think
> > this is highly desirable on master
> 
> I guess we can see how frequently commits are made upstream to see how much
> hassle it would be to update the manifest each time? In addition we could
> make a report that on a regular basis emails people about upstream changes
> between the locked rev and master.

Will sheriffs do these manifest updates?
If not, who are you volunteering for this extra manual work?

If the upstream commits are rare, isn't solution (4) a lot less onerous as well?

> > I could agree with locking down to a revision when we move to a
> > b2gXX_vX_X repo, 
> 
> That's bug 1012618 and IMO is a serious flaw at present, that we should fix
> asap.
> 
> > but we can't do that by locking down (3) or forking (1) the
> > mirror repo unless we also want to lock down/fork master.  I don't think we
> > do.
> 
> We just pin to a revision in the b2g-manifest files, when creating the new
> branch there each time, surely?
> (https://github.com/mozilla-b2g/b2g-manifest/branches)

Ok, so I was disagreeing with comment 0, which involves locking down or forking the mirror.
Pinning the revision for branches closer to release, while keeping master on an upstream branch, is an acceptable solution to me, with the caveats:

a) we need to decide when.  Do we lock this branch down at branch creation (when we move to Aurora), or is that too soon?  Locking it down by the time we create the b2gXX branch sounds good + reasonable; that is 6 weeks after we create the git branches.  From releng's perspective, it's easiest to lock down at branch creation time (moving to Aurora) but I do worry about the manual uplift effort.

b) we should keep master on a branch, not a revision.  Locking down the b2gXX branches (and potentially Aurora) to a revision will cause these upstream repo revisions to diverge over time, making gecko uplifts a little more risky.

Sounds like we're closer than I thought when I wrote comment 5, though.
Depends on: 918055
Assignee: nobody → aki
[11:00]	<aki>	mwu: is tara.xml still in use?
[11:02]	<mwu>	aki: no
[11:03]	<mwu>	probably can even be removed
[11:07]	<aki>	awesome
[11:07]	<aki>	ty

(mh)deathduck:/src/mdauto/build/b2g-manifest [19:28:55] (v1.3*)
1017$ git ls-remote git://codeaurora.org/platform/external/icu4c
AU_LINUX_GECKO_B2G_ICS_1.3.01.03.00.019.113
b275e63734d5f7598596ae5deb7a9d1ba2e6655e       
refs/tags/AU_LINUX_GECKO_B2G_ICS_1.3.01.03.00.019.113

Awesome.

Ok, I'm going to lock icu4c to
AU_LINUX_GECKO_B2G_ICS_1.3.01.03.00.019.113 and nuke tara.xml, which no
one has spoken up for.
https://github.com/mozilla-b2g/b2g-manifest/commit/23c5743313031d46ef595fb5073a6c972ee8c36c , asking for Panos to verify nexus-s.xml so I can transplant to v2.0, v1.4, v1.3, v1.3t.
https://github.com/mozilla-b2g/platform_system_nfcd was missing a v2.0 branch for some reason... possibly because of branching via the old b2g-branch script.

Created that branch, hopefully b2g_bumper will now not error and yet not see a change to land in aurora.
And https://github.com/mozilla-b2g/device-mako . We shouldn't hit this in Sept 1 because we'll branch and lock revisions with the same script.  This one-time bug is going to take significantly longer than I want it to because of the discrepancy between the two scripts, though.
Woot, b2g_bumper bumped the manifests and the only change was gaia!! https://hg.mozilla.org/releases/mozilla-aurora/rev/9c915d7c224d

Moving on to v1.4, which I'd like to lock down before any potential v1.4d branch =\
v1.4 branches created for

https://github.com/mozilla-b2g/platform_system_nfcd
https://github.com/mozilla-b2g/hardware_qcom_display
https://github.com/mozilla-b2g/device-mako

We see some changes from b2g_bumper in platform_frameworks_av and platform_system_core:
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/dde439d7f8d0

It looks like our new setting is more accurate branch-wise.  Whether this causes regressions remains to be seen.
Uh oh: https://tbpl.mozilla.org/php/getParsedLog.php?id=41992640&tree=Mozilla-B2g30-v1.4
14:27:14    ERROR -  build/core/base_rules.mk:134: *** frameworks/native/libs/cpustats: MODULE.TARGET.STATIC_LIBRARIES.libcpustats already defined by frameworks/av/media/libcpustats.  Stop.
14:27:14     INFO -  real	0m4.562s
14:27:14     INFO -  user	0m3.362s
14:27:14     INFO -  sys	0m1.112s
14:27:14     INFO -  
14:27:14     INFO -  > Build failed! <
Sigh, git commit, not git commit -a.
https://github.com/mozilla-b2g/b2g-manifest/commit/2d72291e63c46a385813062d33d45fc191a30592 landed for v1.3t ... still no changes to the b2g_bumper manifests.  Now for v1.3...
Thank you for working on this - however if I understand correctly, this has actually fixed bug 1012618, rather than this bug, which was for trunk. I'll morph the summary of this bug and file a new one for trunk.

Thanks for your work here though :-)
Summary: Third party repositories listed in b2g-manifest should always reference a tag/revision → Third party repositories listed in b2g-manifest should always reference a tag/revision once B2G branches for release
Duplicate of this bug: 1012618
Summary: Third party repositories listed in b2g-manifest should always reference a tag/revision once B2G branches for release → When B2G trunk branches to mozilla-b2gNN manifest revisions should be pinned
Blocks: 1028111
(In reply to Ed Morley [:edmorley UTC+0] from comment #24)
> file a new one for trunk.

Bug 1028111
You need to log in before you can comment on or make changes to this bug.