1466362 - webidl hook not blocking push without dom peer review to integration-autoland repo

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, enhancement, P3)

Description

[Andreea Pavel [:apavel]]

When trying to merge autoland to central today, we've received this: https://irccloud.mozilla.com/file/9tlixUbH/image.png

The revision we're merging: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=06461ddb2699b3b9ee7283bb33307d37b107ec08


This is related to bug https://bugzilla.mozilla.org/show_bug.cgi?id=1351193 

Since this had already been landed in autoland (via mozzreviw), there shouldn't have been an issue when trying to merge it into central. 

I've set a higher priority, since this is preventing us from merging autoland to mc.

Comment 1

[Andreea Pavel [:apavel]]

This should fix this issue https://phabricator.services.mozilla.com/D1518 however it needs to be reviewed.

Comment 2

[Tom Prince [:tomprince]]

The hook has been deployed, so the merge can proceed. We should still keep this open to figure out how the offending commit landed on autoland, though.

Comment 3

[Cristina Coroiu [:ccoroiu]]

We were able to merge autoland to mozilla central

Comment 4

[:glob ✱]

this is unrelated to "autoland the service" (soon to be renamed to "transplant" to avoid this type of confusion).

the summary is a change that touched a .wbidl file landed on landed on the integration-autoland repo:
> https://hg.mozilla.org/integration/autoland/rev/94ffa7844ae7
> Bug 1351193 - Part 1: Added new DataTransfer constructor, r=Nika
> dom/webidl/DataTransfer.webidl

at the time it landed "nika" wasn't a known alias for nika (see https://hg.mozilla.org/hgcustom/version-control-tools/rev/16287fb0132a) so the prevent_webidl_changes.py hook should have rejected it.

however it landed on integration-autoland, and was only rejected during the m-c merge.

Comment 5

[:glob ✱]

prevent_webidl_changes applies itself to `firefox_releasing` repos.
`firefox_releasing` requires all of the following:

- a root node of 8ba995b74e18334ab3707f27e9eb8f4e37ba3d29
- phases.publish set to true
- not a user repo

the autoland repo has phases.public set to false, resulting in the following hooks not being applied:
- prevent_ftl_changes.py
- prevent_sync_ipc_changes.py
- prevent_webidl_changes.py
- try_task_config_file.py

i think we should drop the publishing requirement.  gps, does this sound right to you?

Comment 6

[Tom Prince [:tomprince]]

That would also apply to all the project repos (which they may already apply). I know at least the releng repos (birch, maple, jamun) don't generally merge to m-c, and so having the `prevent_*` hooks apply to them causes.

I wonder if it would make sense to explictly exclude project repos as well.

Comment 7

[Gregory Szorc [:gps]]

If we drop the phases requirement, I believe the hooks will become active on Try.

I think the most robust thing to do is define firefox_releasing or supplement it in terms of the repos defined in mozautomation.repository. That Python module defines our canonical list of repositories. It will also enable us to pull in project repos pretty easily.

Comment 8

[:glob ✱]

using the definitions in mozautomation.repository appears to be tricky, as they are working on the external tree name which is hard to deduce from the on-disk repository.

however while exploring options i found the following in hgrc:

> [mozilla]
> enablefirefoxreleases = true

`enablefirefoxreleases` is currently set to true for mozilla-central, integration/autoland, and integration/mozilla-inbound, which looks like what we need here.

it's currently only used as an indicator to look for `firefoxreleases.db` in hgext/firefoxreleases.  this code won't break if enablefirefoxreleases is set but firefoxreleases.db doesn't exist, so if we want to set it on project repos in the future we should be ok.

Comment 9

[:glob ✱]

hrm, `enablefirefoxreleases` isn't exactly a clear name.

gps - how do you feel about renaming `enablefirefoxreleases` to `firefox_releasing`, and use it for both mozhg/util and hgext/firefoxreleases?

Comment 10

[Gregory Szorc [:gps]]

I'm fine with renaming the config option and using it for multiple purposes. I'm sure I'll come back to regret this (triggering multiple behaviors from a single flag almost always backfires). But we can cross that bridge when we come to it.

Comment 11

[Phabricator Automation]

Attachment: mozhg: rename config var enablefirefoxreleases to firefox_releasing (Bug 1466362) r?gps

Comment 12

[Phabricator Automation]

Attachment: mozhg: use firefox_releasing instead of publishing to identify release repos (Bug 1466362) r?gps

Comment 13

[Phabricator Automation]

Comment on attachment 8987722 [details]
mozhg: rename config var enablefirefoxreleases to firefox_releasing (Bug 1466362) r?gps

Gregory Szorc [:gps] has approved the revision.

https://phabricator.services.mozilla.com/D1820

Comment 14

[Phabricator Automation]

Comment on attachment 8987723 [details]
mozhg: use firefox_releasing instead of publishing to identify release repos (Bug 1466362) r?gps

Gregory Szorc [:gps] has approved the revision.

https://phabricator.services.mozilla.com/D1821

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/67e1e3e246f7
mozhg: rename config var enablefirefoxreleases to firefox_releasing r=gps

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/d0f2c8bb2cf7
mozhg: use firefox_releasing instead of publishing to identify release repos r=gps

Comment 17

[Gregory Szorc [:gps]]

I deployed this yesterday and updated .hg/hgrc files for:

mozilla-central
integration/autoland
integration/mozilla-inbound
releases/mozilla-release
releases/mozilla-beta
releases/mozilla-esr60

We may also want to update configs for various repos under projects/.

Also, we need to keep in mind that any time we create a new Firefox repo that we want the hooks to run on, we'll need to update the .hg/hgrc to include firefox_releasing=true. It is easy to forget to do that. Fortunately, with the move to everything is using autoland, hopefully the days of N repos that could be merged into mozilla-central is nearing its end and we'll have a known set of special repos with special requirements rather than N.