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

RESOLVED FIXED

Status

Firefox OS
GonkIntegration
--
major
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: emorley, Assigned: aki)

Tracking

({sheriffing-P1})

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
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.
(Reporter)

Updated

4 years ago
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
(Reporter)

Comment 1

4 years ago
(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
(Reporter)

Updated

4 years ago
Depends on: 899969
(Reporter)

Comment 2

3 years ago
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
(Reporter)

Comment 3

3 years ago
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.
(Reporter)

Updated

3 years ago
Duplicate of this bug: 910685
(Assignee)

Comment 5

3 years ago
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.
(Reporter)

Comment 6

3 years ago
(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)
(Assignee)

Comment 7

3 years ago
(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.
(Assignee)

Updated

3 years ago
Depends on: 918055
(Assignee)

Updated

3 years ago
Assignee: nobody → aki
(Assignee)

Comment 8

3 years ago
[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.
(Assignee)

Comment 9

3 years ago
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.
(Assignee)

Comment 10

3 years ago
Locked down v2.0: https://github.com/mozilla-b2g/b2g-manifest/commit/cdefade4ea94832a1f98edf0dc3f9c25f2f9bcc5

Ideally b2g_bumper sees nothing to change.
(Assignee)

Comment 11

3 years ago
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.
(Assignee)

Comment 12

3 years ago
Also had to branch https://github.com/mozilla-b2g/hardware_qcom_display
(Assignee)

Comment 13

3 years ago
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.
(Assignee)

Comment 14

3 years ago
https://github.com/mozilla-b2g/kernel_lk and https://github.com/mozilla-b2g/device-flame
(Assignee)

Comment 15

3 years ago
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 =\
(Assignee)

Comment 16

3 years ago
https://github.com/mozilla-b2g/b2g-manifest/commit/990ed724748c4e8a6f95d6079b087166dc1cc385
List of new v1.4 branch repos to follow.
(Assignee)

Comment 17

3 years ago
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.
(Assignee)

Comment 18

3 years ago
https://tbpl.mozilla.org/?tree=Mozilla-B2g30-v1.4&rev=dde439d7f8d0
(Assignee)

Comment 19

3 years ago
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! <
(Assignee)

Comment 20

3 years ago
https://github.com/mozilla-b2g/b2g-manifest/commit/717add62a4cd58cdbd2f832adfd04965043804c0 should reverse the change in comment 17.
(Assignee)

Comment 21

3 years ago
v1.3t: https://github.com/mozilla-b2g/b2g-manifest/commit/837e12025013067171ae3d14e0dc2020b0ce4639
No changes to the b2g_bumper manifests!
(Assignee)

Comment 22

3 years ago
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...
(Assignee)

Comment 23

3 years ago
v1.3: https://github.com/mozilla-b2g/b2g-manifest/commit/b2055338afe9b628faf65bc94cf953a109b51539
No b2g_bumper changes.

v1.3 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
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Reporter)

Comment 24

3 years ago
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 :-)
(Reporter)

Updated

3 years ago
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
(Reporter)

Updated

3 years ago
Duplicate of this bug: 1012618
(Reporter)

Updated

3 years ago
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
(Reporter)

Updated

3 years ago
Blocks: 1028111
(Reporter)

Comment 26

3 years ago
(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.