Please deploy the prevent_webidl_changes hook

RESOLVED FIXED

Status

task
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: fubar)

Tracking

Details

Reporter

Description

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

Comment 3

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

Comment 4

5 years ago
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: 5 years ago
Resolution: --- → FIXED
Reporter

Comment 8

5 years ago
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.
Flags: needinfo?(klibby)
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 :-)
Reporter

Comment 17

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

Comment 18

5 years ago
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?
Reporter

Updated

5 years ago
Depends on: 1004557
Reporter

Updated

5 years ago
Depends on: 1004558
Reporter

Comment 21

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

Comment 22

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

Comment 24

5 years ago
Thanks!
Reporter

Comment 25

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

Comment 27

5 years ago
I'd say we're ready to deploy this to mozilla-central as well.
Flags: needinfo?(klibby)
enabled on m-c.
Flags: needinfo?(klibby)
Reporter

Comment 29

5 years ago
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: 5 years ago5 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.