1316450 - Enforce that nothing new depends on the XPCOM glue

Status: RESOLVED WORKSFORME

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

We don't want new things depending on the xpcom glue, and it would be better to avoid relying on good will for this not happening. IOW, we should enforce it in code.

The most reliable way would be to check we don't add new dependencies on the xpcom library (I think we only care about xpcomglue_s at the moment?)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1316450 - Enforce that nothing new depends on the XPCOM glue.

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

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 8809248 [details]
Bug 1316450 - Enforce that nothing new depends on the XPCOM glue.

https://reviewboard.mozilla.org/r/91834/#review91812

::: python/mozbuild/mozbuild/frontend/emitter.py:104
(Diff revision 1)
>  )
>  
>  from mozbuild.base import ExecutionSummary
>  
>  
> +ALLOWED_XPCOM_GLUE = {

It's a shame that this duplicates testing/cppunittests.ini -- one more place to modify when removing/converting CppUnitTests. Esp. because we have some in-flight bugs doing that, e.g. TestCertDB and TestSTSParser in bug 1315869.

Comment 3

[Mike Hommey [:glandium]]

(In reply to Nicholas Nethercote [:njn] from comment #2)
> It's a shame that this duplicates testing/cppunittests.ini -- one more place
> to modify when removing/converting CppUnitTests. Esp. because we have some
> in-flight bugs doing that, e.g. TestCertDB and TestSTSParser in bug 1315869.

Seems to me cppunittests.ini should be generated at build time.

(also note that cppunittests.ini doesn't contain all the information added in this patch, so it can't really be used instead)

Comment 4

[Nicholas Nethercote [inactive]]

> Seems to me cppunittests.ini should be generated at build time.

That would be nice
 
> (also note that cppunittests.ini doesn't contain all the information added
> in this patch, so it can't really be used instead)

True. I should have said "overlaps" instead of "duplicates".

Comment 5

[Mike Hommey [:glandium]]

(In reply to Nicholas Nethercote [:njn] from comment #4)
> > Seems to me cppunittests.ini should be generated at build time.
> 
> That would be nice

Filed bug 1316518

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8809248 [details]
Bug 1316450 - Enforce that nothing new depends on the XPCOM glue.

https://reviewboard.mozilla.org/r/91834/#review92834

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8809248 [details]
Bug 1316450 - Enforce that nothing new depends on the XPCOM glue.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91834/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/621aa115c3df
Enforce that nothing new depends on the XPCOM glue. r=gps

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/621aa115c3df

Comment 10

[Jorg K (CEST = GMT+2)]

You didn't think of including Thunderbird in your list somehow?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/8921f4d2c3ee
Enforce that nothing new depends on the XPCOM glue: Remove sdp_unittests (removed in bug 1316886) and TestMinStringAPI (removed in bug 1316732). r=bustage-fix a=bustage-fix on a CLOSED TREE

Comment 12

[Mike Hommey [:glandium]]

There's an explicit exclusion for mailnews/ where presumably the only thing depending on xpcomglue that is not the main program (which should be covered by MOZ_BUILD_APP) is.

Comment 13

[Mike Hommey [:glandium]]

... except comm-central doesn't contain everything, and calendar has a xpcom dependency. Please file a followup.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/f91e37a3afc5
Enforce that nothing new depends on the XPCOM glue: Remove sdp_unittests, not sdp_file_parser. r=bustage a=bustage on a CLOSED TREE

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/fb3473c1770d
Enforce that nothing new depends on the XPCOM glue: Re-add sdp_unittests because its removal hasn't been merged around yet. r=bustage-fix a=bustage-fix on a CLOSED TREE

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebc09f5f36bf
Backed out changeset fb3473c1770d for landing on wrong integration tree. r=backout on a CLOSED TREE

Comment 17

[Jorg K (CEST = GMT+2)]

(In reply to Mike Hommey [:glandium] from comment #13)
> ... except comm-central doesn't contain everything, and calendar has a xpcom
> dependency. Please file a followup.
Done already: Bug 1317674.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8f4893e390c3

Comment 19

[Carsten Book [:Tomcat]]

this was backed out in https://hg.mozilla.org/mozilla-central/rev/8f4893e390c3

Comment 20

[Benjamin Smedberg]

The xpcom glue is no longer, so this served its purpose. I'm not sure of the right bug resolution for that, but I'll try something.