Closed Bug 1002652 Opened 6 years ago Closed 6 years ago

Please deploy the prevent_webidl_changes hook

Categories

(Developer Services :: General, task)

x86
macOS
task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ehsan, Assigned: fubar)

References

Details

This hook was added in bug 1001106.  We need it on mozilla-central and all of its integration branches, mozilla-aurora and mozilla-beta.

Thanks!
Just so I'm clear, is that "and all of its integration branches, which are m-a and m-b" or is "all of its integration branches" another list of repos? 

Also, changeset cacacc4b45e0, correct?
Assignee: server-ops-devservices → klibby
Ehsan -- it sounds like we would want this on all project branches as well. Can you confirm? Also, can you think of any repos where this hook would cause a problem. Given that this is new policy, I'd rather "over apply" than miss one.

fubar: this will be all of the repos in integration/releases-*, projects/*, and mozilla-central. (neither m-a nor m-b)
Flags: needinfo?(ehsan)
(In reply to Kendall Libby [:fubar] from comment #1)
> Just so I'm clear, is that "and all of its integration branches, which are
> m-a and m-b" or is "all of its integration branches" another list of repos? 

I meant mozilla-inbound, b2g-inbound and fx-team.  I think these are the only ones.

> Also, changeset cacacc4b45e0, correct?

Yep.

(In reply to Hal Wine [:hwine] (use needinfo) from comment #2)
> Ehsan -- it sounds like we would want this on all project branches as well.
> Can you confirm? Also, can you think of any repos where this hook would
> cause a problem. Given that this is new policy, I'd rather "over apply" than
> miss one.

Hmm, project branches usually have their own review policies (and sometimes they don't even require review) so I would say we don't want this to be enabled on any project branches.
Flags: needinfo?(ehsan)
ping?
Flags: needinfo?(klibby)
I'm actually more uncertain about which repos you want this hook on now, between #c0, #c2, and #c3.  Sorry if this is a lot of back and forth, but I don't know how all repos are used where, and by whom, and for what, so I need a clear, explicit list.
Flags: needinfo?(klibby)
Fair enough! Let's go with this:
 mozilla-central
 integration/mozilla-inbound
 integration/b2g-inbound
 integration/fx-team
Ok, added:

pretxnchangegroup.d_webidl = python:mozhghooks.prevent_webidl_changes.hook

Shout if something seems amiss or if you want to add it to other repos.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Thanks, Kendall!  Can you please add mozilla-aurora, mozilla-beta and mozilla-release as well?  I'm trying to figure out an answer as to what we should do for project branchers...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
As far as I can see, nothing in the merge from mozilla-inbound to mozilla-central that I'm attempting to push tonight seems to touch webidl files, but I'm getting the following error when I make that attempt:
WebIDL file dom/webidl/WorkerNavigator.webidl altered in this changeset without DOM peer review

When I run | hg log dom/webidl/WorkerNavigator.webidl | 
The most recent change was in February:
changeset:   181047:c5624c2a2d2f
user:        Gene Lian <clian@mozilla.com>
date:        Mon Feb 24 21:57:15 2014 +0800
summary:     Bug 949325 - C++ wrapper to support DataStore API on the worker (part 2-3, dispatch tasks on the worker to

changeset:   162615:df0f58eb2b6f
user:        Nikhil Marathe <nsm.nikhil@gmail.com>
date:        Tue Nov 19 15:08:50 2013 -0800
summary:     Bug 925437 - Implement navigator.onLine on Workers. r=khuey sr=sicking

changeset:   156577:bdd0b739b34d
user:        Tareq Khandaker <tareqakhandaker@gmail.com>
date:        Wed Nov 20 09:29:01 2013 -0500
summary:     Bug 925847 - WorkerNavigator does not implement all of NavigatorID. r=nsm

changeset:   139669:4e044202c1b4
user:        Ms2ger <ms2ger@gmail.com>
date:        Wed Jul 24 09:38:23 2013 +0200
summary:     Bug 878401 - Move WorkerNavigator to WebIDL; r=khuey






Is this something screwy with Windows file systems or the command prompt maybe? Explorer says that .webidl was modified sometime today, but hg doesn't seem to agree.
changing ni to the hook developer
Flags: needinfo?(klibby) → needinfo?(ehsan)
(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #8)
> Thanks, Kendall!  Can you please add mozilla-aurora, mozilla-beta and
> mozilla-release as well?  I'm trying to figure out an answer as to what we
> should do for project branchers...

Moved additional branches to bug 1004368 to avoid any potential impact to chem spills until the impact on sheriffs & merging is fully understood.
Sounds like we need the new hook to skip changesets with merge in the name
On another merge:
pushing to ssh://hg.mozilla.org/mozilla-central/
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 113 changesets with 818 changes to 424 files
remote:
remote:
remote: ************************** ERROR ****************************
remote:
*** WebIDL file dom/webidl/WorkerNavigator.webidl altered in this changeset without DOM peer review***
remote:
Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..
remote:
remote: *************************************************************
remote:
remote:
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.d_webidl hook failed
Kendall, please may you disabled the hook on all repositories it is updated, since it's blocking us pushing merges, unless we spam the commit messages.
Flags: needinfo?(klibby)
Done.
Flags: needinfo?(klibby)
Super quick - thank you :-)
The hook just caught bdd0b739b34d, which modified Navigator.webidl without a DOM peer review.  Is there really anything related to merge commits here?

Perhaps we should first enable the hook on mozilla-inbound, b2g-inbound and fx-team, wait a day or so, and then enable it on mozilla-central.
Flags: needinfo?(ehsan) → needinfo?(klibby)
Oh hmm, nevermind, didn't pay attention to the dates :(
Flags: needinfo?(klibby)
Please could we also add an "OVERRIDE WEBIDL HOOK" or similar? For two of the merges today I had to pick an arbitrary reviewer to add to the commit message.
(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #17)
> Perhaps we should first enable the hook on mozilla-inbound, b2g-inbound and
> fx-team, wait a day or so, and then enable it on mozilla-central.

Can we let this hook ride the trains w.r.t. m-a, m-b, & m-r?
Depends on: 1004557
Depends on: 1004558
(In reply to Ed Morley [:edmorley UTC+0] from comment #19)
> Please could we also add an "OVERRIDE WEBIDL HOOK" or similar? For two of
> the merges today I had to pick an arbitrary reviewer to add to the commit
> message.

No, we should just deploy it on integration branches first.

(In reply to Hal Wine [:hwine] (use needinfo) from comment #20)
> (In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment
> #17)
> > Perhaps we should first enable the hook on mozilla-inbound, b2g-inbound and
> > fx-team, wait a day or so, and then enable it on mozilla-central.
> 
> Can we let this hook ride the trains w.r.t. m-a, m-b, & m-r?

No need to do that, bug 1004558 fixes the issue with merge commits.
No longer depends on: 1004557
Kendall, can you please deploy the hook as of changeset 723018c7baf1 to mozilla-inbound, b2g-inbound and fx-team please?  Thanks!
Flags: needinfo?(klibby)
All set! Shout or ping me on IRC if you see something amiss.
Flags: needinfo?(klibby)
Thanks!
Kendal, do you mind deploying 9ff49284a16d to mozilla-inbound, b2g-inbound and fx-team please, when you get a chance?  I fixed a problem in the error message generated by the hook.  Thanks!
Flags: needinfo?(klibby)
All set!
Flags: needinfo?(klibby)
I'd say we're ready to deploy this to mozilla-central as well.
Flags: needinfo?(klibby)
enabled on m-c.
Flags: needinfo?(klibby)
Thanks, so I think we're done in this bug.  I'll ping on bug 1004368 next week to allow this hook to bake on trunk for a few days.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Component: Server Operations: Developer Services → General
Product: mozilla.org → Developer Services
You need to log in before you can comment on or make changes to this bug.