1170718 - Remove hg hook that complains about not reving UUIDs

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

UUIDs don't really serve any purpose anymore. We shouldn't complain about not changing them.

Comment 1

[Gregory Szorc [:gps]]

This seems like a big change and it's news to me! I'd like a second opinion.

Benjamin: do you support this change?

Comment 2

[Benjamin Smedberg]

Aren't UUIDs still used by the build system to do correct incremental rebuilds?

I feel like if we aren't going to care about this we ought to get rid of IDL UUIDs altogether and generate random UUIDs at build time.

Comment 3

[Gregory Szorc [:gps]]

The build system gets its dependencies from the IDL parser (.deps attribute). And, from a brief reading of xpcom/idl-parser/xpidl.py. I /think/ UUIDs have nothing to do with dependency management.

Assuming the build system doesn't care about UUIDs, does that mean you approve removing the hook? Or do you want to block it on changing how UUIDs work?

Comment 4

[Benjamin Smedberg]

We should not make changes to the hg hooks until we've changed the rule about revving IIDs. Since that hasn't happened, the hook should stay in place.

Comment 5

[Gregory Szorc [:gps]]

Closing this until a new IID policy is instituted.

Comment 6

[(no longer active)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> We should not make changes to the hg hooks until we've changed the rule
> about revving IIDs. Since that hasn't happened, the hook should stay in
> place.

Revving IIDs was only required for binary extensions as far as I know, which we don't support any more.  Is there another reason to keep doing that?

Comment 7

[Nathan Froyd [:froydnj]]

(In reply to :Ehsan Akhgari from comment #6)
> (In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> > We should not make changes to the hg hooks until we've changed the rule
> > about revving IIDs. Since that hasn't happened, the hook should stay in
> > place.
> 
> Revving IIDs was only required for binary extensions as far as I know, which
> we don't support any more.  Is there another reason to keep doing that?

Bug 977464?  (Thank you gps for laying out the problem in that bug report!)

Comment 8

[(no longer active)]

(In reply to Nathan Froyd [:froydnj] from comment #7)
> (In reply to :Ehsan Akhgari from comment #6)
> > (In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> > > We should not make changes to the hg hooks until we've changed the rule
> > > about revving IIDs. Since that hasn't happened, the hook should stay in
> > > place.
> > 
> > Revving IIDs was only required for binary extensions as far as I know, which
> > we don't support any more.  Is there another reason to keep doing that?
> 
> Bug 977464?  (Thank you gps for laying out the problem in that bug report!)

I submitted a patch to remove this requirement for incremental builds.

Comment 9

[(no longer active)]

Greg, what's the correct way to disable a hook for central/inbound and the like but keeping it enabled on Aurora/Beta/Release etc?

Comment 10

[Gregory Szorc [:gps]]

You ask me or fubar to remove the hook and we SSH into the server and do it.

Comment 11

[(no longer active)]

Thanks!  Will that mean that the hook will ride the trains to disappear from all branches?

If not, it may be better to modify the hook to check out the version number in browser/config/version.txt and auto-disable itself on 47 onward.

What do you think?

Comment 12

[Gregory Szorc [:gps]]

Hooks don't ride the trains since each repo is independent (sadly).

We could look at version.txt. But it's probably easier to just disable the hook every 6 weeks since the hook will disappear in a few months.

Comment 13

[(no longer active)]

(In reply to Gregory Szorc [:gps] from comment #12)
> We could look at version.txt. But it's probably easier to just disable the
> hook every 6 weeks since the hook will disappear in a few months.

Even given things like ESR and b2g branches?

If that's fine with you, I'm happy to just have the hook be disabled manually, but I have no faith in my memory to be able to remind you guys at the right times...  :(

Comment 14

[(no longer active)]

Greg, can you please remove this hook from central, inbound, fx-team and b2g-inbound (and other similar repos which I may be forgetting)?

(Or if you'd like me to do the version.txt alternative, please let me know.)

Comment 15

[(no longer active)]

Ping?

Comment 16

[Gregory Szorc [:gps]]

Removed from central, inbound, fx-team, b2g-inbound, and comm-central (which I also thinks meets the requirements since it is a tip-like repo).

Comment 17

[Nathan Froyd [:froydnj]]

(In reply to Gregory Szorc [:gps] from comment #16)
> Removed from central, inbound, fx-team, b2g-inbound, and comm-central (which
> I also thinks meets the requirements since it is a tip-like repo).

comm-central might still want this, since Thunderbird still permits binary extensions...though I suppose if Gecko isn't updating its UUIDs, Thunderbird's binary extension story gets tricky anyway...

Comment 18

[Gregory Szorc [:gps]]

Do you want this hook on c-c?

Comment 19

[Masatoshi Kimura [:emk]]

C-c binary components use components in m-c, they will have hard time with the unrev'ed components.
Reintroducing only to c-c will not solve them problem.

Comment 20

[Joshua Cranmer [:jcranmer]]

(In reply to Gregory Szorc [:gps] from comment #18)
> Do you want this hook on c-c?

If mozilla-central isn't revving its UUIDs, then there's little point in forcing comm-central to rev its UUIDs, so no.

Comment 21

[Gregory Szorc [:gps]]

I just removed the UUID hook from mozilla-beta since it looks like we stopped requiring UUID bumps as part of the 47 release.

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

So that's everywhere we care about at this point, right? Can we resolve this bug?

Comment 23

[Gregory Szorc [:gps]]

hghooks/mozhghooks/prevent_idl_change_without_uuid_bump.py and hghooks/mozhghooks/prevent_uuid_changes.py (and associated tests) still exist. I assume we'll want to nuke these hooks and traces of them from repo configs.

I'll do that right now...

Comment 24

[Gregory Szorc [:gps]]

Attachment: hghooks: remove legacy IDL/UUID change verification hooks (bug 1170718);

These deleted hooks (and references to them) were used to prevent
unwanted binary compatibility breakage with XPIDL interfaces (which
would impact extensions).

We no longer support binary extensions on any repository that we still
release from. So these hooks no longer serve a purpose and can be
deleted.

Review commit: https://reviewboard.mozilla.org/r/183204/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183204/

Comment 25

[Nathan Froyd [:froydnj]]

Comment on attachment 8911782 [details]
hghooks: remove legacy IDL/UUID change verification hooks (bug 1170718);

https://reviewboard.mozilla.org/r/183204/#review188360

Thanks for cleaning this up.

::: hgserver/tests/test-hooks-smoketest.t
(Diff revision 1)
>  Install hooks that mimic production
>  
>    $ hgmo exec hgssh /set-hgrc-option mozilla-central hooks pretxnchangegroup.b_singlehead python:mozhghooks.single_head_per_branch.hook
>    $ hgmo exec hgssh /set-hgrc-option mozilla-central hooks pretxnchangegroup.c_commitmessage python:mozhghooks.commit-message.hook
>    $ hgmo exec hgssh /set-hgrc-option mozilla-central hooks pretxnchangegroup.d_webidl python:mozhghooks.prevent_webidl_changes.hook
> -  $ hgmo exec hgssh /set-hgrc-option mozilla-central hooks pretxnchangegroup.e_idl_no_uuid python:mozhghooks.prevent_idl_change_without_uuid_bump.hook

I guess we never enabled the `prevent_uuid_changes` hook for smoketesting... =/

Comment 26

[Gregory Szorc [:gps]]

Comment on attachment 8911782 [details]
hghooks: remove legacy IDL/UUID change verification hooks (bug 1170718);

https://reviewboard.mozilla.org/r/183204/#review188360

> I guess we never enabled the `prevent_uuid_changes` hook for smoketesting... =/

This test fail is relatively new compared to the hook. The test coverage wasn't terrible important, as the hook's .t tests were sufficient for most cases. So no frowny face is deserved :)

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/6a20a18a2d58
hghooks: remove legacy IDL/UUID change verification hooks ; r=froydnj