Closed Bug 1316450 Opened 3 years ago Closed 3 years ago

Enforce that nothing new depends on the XPCOM glue

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox53 fixed)

RESOLVED WORKSFORME
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file)

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?)
Assignee: nobody → mh+mozilla
Summary: Enforce that no new GeckoProgram, GeckoSimplePrograms or GeckoCppUnitTest are added to moz.build → Enforce that nothing new depends on the XPCOM glue
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.
(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)
> 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".
(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 on attachment 8809248 [details]
Bug 1316450 - Enforce that nothing new depends on the XPCOM glue.

https://reviewboard.mozilla.org/r/91834/#review92834
Attachment #8809248 - Flags: review?(gps) → review+
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/621aa115c3df
Enforce that nothing new depends on the XPCOM glue. r=gps
https://hg.mozilla.org/mozilla-central/rev/621aa115c3df
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Depends on: 1317674
You didn't think of including Thunderbird in your list somehow?
Flags: needinfo?(gps)
Pushed by archaeopteryx@coole-files.de:
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
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.
... except comm-central doesn't contain everything, and calendar has a xpcom dependency. Please file a followup.
Flags: needinfo?(gps)
Pushed by archaeopteryx@coole-files.de:
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
Pushed by archaeopteryx@coole-files.de:
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
Backout by archaeopteryx@coole-files.de:
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
(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.
this was backed out in https://hg.mozilla.org/mozilla-central/rev/8f4893e390c3
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → WORKSFORME
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.