Formalising the current 'mozilla-central essential pushes only' recommendation

63 views
Skip to first unread message

Ed Morley

unread,
Apr 7, 2014, 5:37:34 AM4/7/14
to dev.tree-management, Sheriffs, dev.platform
(Follow-ups to dev.tree-management please)

Hi all :-)

The vast majority of mozilla-central landings are now via curated merges
from integration/team repositories. This dramatically increases the
chance that the tip of mozilla-central is in a known-good state, meaning
that:

* Integration/project repos pulling from mozilla-central are less likely
to receive breakage from elsewhere.
* A bad landing + tree closure on one integration/team repo doesn't
impact the ability of other repositories to merge into mozilla-central.
* Developers have the choice of a safer repository to use as their qbase
for local development / try pushes.
* Nightly builds are less prone to being broken & requiring a respin [1].

Non-critical mozilla-central landings are already discouraged and as
such are rare. However, the sheriffs [2] would like to formalise this,
by adjusting the mozilla-central tree rules [3] to state that direct
pushes must be for one of the following reasons:

1) Merging from an integration/team/project repository (there is no
restriction on who may make these merges).
2) Automated blocklist / HSTS preload list updates [4].
3) For the resolution (ie: backout or follow-up fix) of critical
regressions (eg: top-crashers or other major functional regression) that
will result in a Nightly respin or must make the imminent scheduled
Nightly at all costs.
4) Anything else for which common sense (or asking in #developers) says
is an appropriate reason for a direct landing on mozilla-central.

Clearly #4 is very fuzzy - but I'm hopeful self-policing has a good
chance of success and so would like to try that first. Note #4 would not
include the landing of new features directly onto mozilla-central if
they have missed the last integration repository merge on the day of
uplift - the correct course of action would be to request
aurora-approval and uplift instead, due to the amount of merge-day
breakage this has caused in the past.

If there are any other cases that should be explicitly mentioned above
instead of relying on #4, please let me know.

A proportion of the current mozilla-central non-critical commits are
made by people inadvertently pushing to the wrong repository. To prevent
these, once the tree rules are adjusted on the wiki the sheriffs
envisage the next step will be switching mozilla-central to a non-open
tree state (name TBD) using the existing tree closure hook. Backouts,
merges & automated bot updates will not need any additional annotation -
others will simply use a (yet to be chosen) commit message string to
signify awareness & adherence to the new tree policy.

We welcome your feedback :-)

Best wishes,

Ed

[1] We've recently disabled the last good revision functionality on
mozilla-central, since it was both not functioning as expected and also
frequently caused delays in Nightly generation when non-tier 1 jobs are
failed or infra issues caused a single build to fail. The net change is
positive, however it increases the need to ensure mozilla-central is in
a known good state.
[2] https://wiki.mozilla.org/Sheriffing
[3] https://wiki.mozilla.org/Tree_Rules
[4] eg: https://hg.mozilla.org/mozilla-central/rev/c401296f71ae and
https://hg.mozilla.org/mozilla-central/rev/beea7a7f3fc3 - though I think
we may wish to move these to an integration repository in the future,
since they have occasionally caused breakage in the past. However that
will require discussion and automation changes, so I believe we should
maintain the status quo initially.

Gregory Szorc

unread,
Apr 7, 2014, 6:36:47 PM4/7/14
to Ed Morley, dev.tree-management, Sheriffs
On 4/7/14, 2:37 AM, Ed Morley wrote:
> (Follow-ups to dev.tree-management please)
>
> Hi all :-)
>
> The vast majority of mozilla-central landings are now via curated merges
> from integration/team repositories.

I wouldn't say "vast majority."

$ hg log -r 'firstpushtree(central) and firstpushdate("2014")'
--template '{ifeq(p2rev, "-1", "normal", "merge")}\n' | sort | uniq -c

387 merge
239 normal

Of those 239 non-merge commits that landed in central first:

$ hg log -r 'not merge() and firstpushtree(central) and
firstpushdate("2014") and (desc("backout") or desc("backed"))'

-> 49 were backouts

$ hg log -r 'not merge() and firstpushtree(central) and
firstpushdate("2014") and user("ffxbld")'

-> 22 were automated commits for HSTS updates, etc

$ hg log -r 'not merge() and firstpushtree(central) and
firstpushdate("2014") and user("releas...@mozilla.com")'

-> 4 were release tags

$ hg log -r 'not merge() and firstpushtree(central) and
firstpushdate("2014") and user("release+...@mozilla.com")'

-> 4 were b2g release bumps

Of the 626 commits that landed on m-c first, that leaves ~160 (~25%) as
regular commits. Those commits can be seen at
https://pastebin.mozilla.org/4774819. 48 individuals made those commits.
People with multiple commits are as follows:

2 Daniel Holbert <dhol...@cs.stanford.edu>
2 David Keeler <dke...@mozilla.com>
2 Ed Morley <emo...@mozilla.com>
2 Ehsan Akhgari <ehsan....@gmail.com>
2 Jonathan Watt <jw...@jwatt.org>
2 Mark Hammond <mham...@skippinet.com.au>
2 Rail Aliiev <ra...@mozilla.com>
2 Randell Jesup <rje...@jesup.org>
2 Richard Newman <rne...@mozilla.com>
2 Simone Bruno <sbr...@mozilla.com>
2 Wes Kocher <wko...@mozilla.com>
3 Benoit Jacob <bja...@mozilla.com>
3 Mike Hommey <mh+mo...@glandium.org>
3 Ryan VanderMeulen <rya...@gmail.com>
4 Ehsan Akhgari <eh...@mozilla.com>
4 Phil Ringnalda <philri...@gmail.com>
5 Gregory Szorc <g...@mozilla.com>
6 Brian Smith <br...@briansmith.org>
6 Lukas Blakk <lsb...@mozilla.com>
7 Robert Strong <robert....@gmail.com>
15 Honza Bambas <honza...@firemni.cz>
15 L. David Baron <dba...@dbaron.org>
43 Ms2ger <ms2...@gmail.com>

Now we have some numbers, people, and scenarios to attach to this
discussion...

> This dramatically increases the
> chance that the tip of mozilla-central is in a known-good state, meaning
> that:
>
> * Integration/project repos pulling from mozilla-central are less likely
> to receive breakage from elsewhere.
> * A bad landing + tree closure on one integration/team repo doesn't
> impact the ability of other repositories to merge into mozilla-central.
> * Developers have the choice of a safer repository to use as their qbase
> for local development / try pushes.
> * Nightly builds are less prone to being broken & requiring a respin [1].

I'm with you 100% that m-c should strive to be green 100% of the time
and that landing directly on m-c without prior automation coverage
(read: try push) or a strong guarantee the tree won't burn is something
we should work to avoid.

> If there are any other cases that should be explicitly mentioned above
> instead of relying on #4, please let me know.

I frequently land DONTBUILD (NPOTB) docs only changes direct to m-c. I
commonly do this to answer a question someone has posed in IRC or via
email. I can link them to the generated docs (only built from m-c today)
within a few minutes of me landing. I would appreciate an exception to
allow landing docs changes directly to m-c.

> A proportion of the current mozilla-central non-critical commits are
> made by people inadvertently pushing to the wrong repository. To prevent
> these, once the tree rules are adjusted on the wiki the sheriffs
> envisage the next step will be switching mozilla-central to a non-open
> tree state (name TBD) using the existing tree closure hook. Backouts,
> merges & automated bot updates will not need any additional annotation -
> others will simply use a (yet to be chosen) commit message string to
> signify awareness & adherence to the new tree policy.

Most accidental pushes can be avoided by:

1) Not have a "default" entry in the [paths] of your hgrc.
2) Always using -r with `push` to limit pushed changesets to ancestors
of the selected revision. You almost always want this to be '.' (the
working copy changeset).
3) Performing `hg outgoing` before `hg push` to see which changesets
would be pushed.

My mozext extension [1] installs paths to common Firefox repos
automagically so you don't have to keep your hgrc in sync:

$ hg pulltree fx-team

# Find an open tree.
$ hg treestatus
# Rebase changes if needed
$ hg out -r . inbound
$ hg pushtree -r . inbound

[1]
https://hg.mozilla.org/hgcustom/version-control-tools/file/default/hgext/mozext

L. David Baron

unread,
Apr 7, 2014, 7:14:56 PM4/7/14
to Gregory Szorc, Sheriffs, dev.tree-management
I think something is wrong with your data, since 14 of the 15
changesets in your list that are atttributed to me landed in inbound
first, in this push:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=05cf275996a2

The other one was actually a fix for build bustage resulting from a
merge-to-central, so actually belongs in your list:
https://hg.mozilla.org/mozilla-central/rev/4941a2ac0786

(On the primary topic, I'm fine with the change in rules.)

-David

--
ğ„ž L. David Baron http://dbaron.org/ 𝄂
𝄢 Mozilla https://www.mozilla.org/ 𝄂
Before I built a wall I'd ask to know
What I was walling in or walling out,
And to whom I was like to give offense.
- Robert Frost, Mending Wall (1914)
signature.asc

Gregory Szorc

unread,
Apr 7, 2014, 7:30:55 PM4/7/14
to L. David Baron, Sheriffs, dev.tree-management
You are correct. My local pushlog database was somehow missing push data
for your push!

$ hg changesetpushes ada9f861cd50 --all
202434:ada9f861cd50 Bug 989688 patch 6 - Run the reftests in
toolkit/content/tests/reftests/. r=enndeakin

This should have been added in
https://hg.mozilla.org/mozilla-central/rev/0c2c2c895e5d (bug 442419) or
perhaps also in bug 841001 when another test was added to this
directory.
Release Tree Date Username Build Info
central 2014-04-02T06:55:19 cb...@mozilla.com
https://tbpl.mozilla.org/?tree=Mozilla-Central&rev=61b1f28c3c16
cedar 2014-04-02T08:07:57 rya...@gmail.com
https://tbpl.mozilla.org/?tree=Cedar&rev=df340b8a9423
fx-team 2014-04-02T10:25:35 emo...@mozilla.com
https://tbpl.mozilla.org/?tree=Fx-Team&rev=2c41d8ad059f
b2ginbound 2014-04-02T10:56:22 emo...@mozilla.com
https://tbpl.mozilla.org/?tree=B2Ginbound&rev=93796178259f
ash 2014-04-03T08:08:17 gbr...@mozilla.com
https://tbpl.mozilla.org/?tree=Ash&rev=df95040979a2

I reset the database and all appears well now:

$ hg changesetpushes ada9f861cd50 --all
202434:ada9f861cd50 Bug 989688 patch 6 - Run the reftests in
toolkit/content/tests/reftests/. r=enndeakin

This should have been added in
https://hg.mozilla.org/mozilla-central/rev/0c2c2c895e5d (bug 442419) or
perhaps also in bug 841001 when another test was added to this
directory.
Release Tree Date Username Build Info
inbound 2014-04-01T12:04:14 dba...@mozilla.com
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=05cf275996a2
try 2014-04-01T12:14:06 nmat...@mozilla.com
https://tbpl.mozilla.org/?tree=Try&rev=35871c44d4f2
central 2014-04-02T06:55:19 cb...@mozilla.com
https://tbpl.mozilla.org/?tree=Mozilla-Central&rev=61b1f28c3c16
cedar 2014-04-02T08:07:57 rya...@gmail.com
https://tbpl.mozilla.org/?tree=Cedar&rev=df340b8a9423
alder 2014-04-02T09:21:44 Pidg...@gmail.com
https://tbpl.mozilla.org/?tree=Alder&rev=ac2fe233c3a5
pine 2014-04-02T09:27:14 gwa...@mozilla.com
https://tbpl.mozilla.org/?tree=Pine&rev=84cf46ef2a0a
fx-team 2014-04-02T10:25:35 emo...@mozilla.com
https://tbpl.mozilla.org/?tree=Fx-Team&rev=2c41d8ad059f
b2ginbound 2014-04-02T10:56:22 emo...@mozilla.com
https://tbpl.mozilla.org/?tree=B2Ginbound&rev=93796178259f
holly 2014-04-02T11:53:15 wmccl...@mozilla.com
https://tbpl.mozilla.org/?tree=Holly&rev=223da4e02017
birch 2014-04-02T15:38:59 m...@glandium.org
https://tbpl.mozilla.org/?tree=Birch&rev=7b924b517aa8
larch 2014-04-03T07:27:45 mhab...@mozilla.com
https://tbpl.mozilla.org/?tree=Larch&rev=773714cd3749
ash 2014-04-03T08:08:17 gbr...@mozilla.com
https://tbpl.mozilla.org/?tree=Ash&rev=df95040979a2
gum 2014-04-04T07:27:14 honza...@firemni.cz
https://tbpl.mozilla.org/?tree=Gum&rev=e8289dd6c7cc


My numbers are now as follows:

$ hg log -r 'firstpushtree(central) and firstpushdate("2014")'
--template '{ifeq(p2rev, "-1", "normal", "merge")}\n' | sort | uniq -c
388 merge
220 normal

$ hg log -r 'not merge() and firstpushtree(central) and
firstpushdate("2014") and (desc("backout") or desc("backed"))'

-> 48 were backouts

...

2 Daniel Holbert <dhol...@cs.stanford.edu>
2 David Keeler <dke...@mozilla.com>
2 Ed Morley <emo...@mozilla.com>
2 Ehsan Akhgari <ehsan....@gmail.com>
2 Jonathan Watt <jw...@jwatt.org>
2 Mark Hammond <mham...@skippinet.com.au>
2 Rail Aliiev <ra...@mozilla.com>
2 Randell Jesup <rje...@jesup.org>
2 Richard Newman <rne...@mozilla.com>
2 Simone Bruno <sbr...@mozilla.com>
2 Wes Kocher <wko...@mozilla.com>
3 Benoit Jacob <bja...@mozilla.com>
3 Mike Hommey <mh+mo...@glandium.org>
3 Ryan VanderMeulen <rya...@gmail.com>
4 Ehsan Akhgari <eh...@mozilla.com>
4 Phil Ringnalda <philri...@gmail.com>
5 Gregory Szorc <g...@mozilla.com>
6 Brian Smith <br...@briansmith.org>
6 Lukas Blakk <lsb...@mozilla.com>
7 Robert Strong <robert....@gmail.com>
15 Honza Bambas <honza...@firemni.cz>
43 Ms2ger <ms2...@gmail.com>

The results are roughly the same. I guess my database was only slightly
corrupt. Weird.

Ed Morley

unread,
Jun 2, 2014, 10:57:42 AM6/2/14
to dev.tree-management, Sheriffs, dev.platform
On 07/04/2014 10:37, Ed Morley wrote:
> Non-critical mozilla-central landings are already discouraged and as
> such are rare. However, the sheriffs [2] would like to formalise this,
> by adjusting the mozilla-central tree rules [3] to state that direct
> pushes must be for one of the following reasons:

With the absence of any major objections to proceeding with this, I've
updated the tree rules to reflect these changes:
https://wiki.mozilla.org/Tree_Rules#mozilla-central_.28Nightly_channel.29

On 07/04/2014 10:37, Ed Morley wrote:
> A proportion of the current mozilla-central non-critical commits are
> made by people inadvertently pushing to the wrong repository. To prevent
> these, once the tree rules are adjusted on the wiki the sheriffs
> envisage the next step will be switching mozilla-central to a non-open
> tree state (name TBD) using the existing tree closure hook. Backouts,
> merges & automated bot updates will not need any additional annotation -
> others will simply use a (yet to be chosen) commit message string to
> signify awareness & adherence to the new tree policy.

For the short term we'll take advantage of the existing "approval
required" tree hook to remind people about the change, until such a
point as we have a new named state for it. The wiki changes above
describe what to put in the commit message when self-approving (eg
a={merge,chemspill,respin,...}).

NB: For the avoidance of doubt, this is similar to the way the "approval
required" hook was used for the PGO issues last year - and is not
related to the release driver "approval request" workflow in any way.

Cheers,

Ed
Reply all
Reply to author
Forward
0 new messages