Change how binary xpcom components manifests are created

RESOLVED FIXED in Firefox 45

Status

()

Core
Build Config
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Assignee)

Description

2 years ago
The FasterMake backend doesn't handle them at all for the moment, and the RecursiveMake backend relies on rules that only exist without --disable-compile-environment.
(Assignee)

Comment 1

2 years ago
Created attachment 8692404 [details] [diff] [review]
Add a NO_COMPONENTS_MANIFEST moz.build variable

The variable is used by Lightning in comm-central and influences
binary-component manifest creation.
Attachment #8692404 - Flags: review?(gps)
(Assignee)

Comment 2

2 years ago
Created attachment 8692405 [details] [diff] [review]
Move NO_COMPONENTS_MANIFEST to moz.build in c-c
Attachment #8692405 - Flags: review?(gps)
(Assignee)

Comment 3

2 years ago
Created attachment 8692408 [details] [diff] [review]
Emit a specialized object for chrome.manifest entries

This new ChromeManifestEntry object type is generic and can hold any kind of
chrome manifest entry, but we currently only emit them for binary components.

References to sub-directory manifests is left to the backend, for now, until
all manifest entries are emitted by the frontend.
Attachment #8692408 - Flags: review?(gps)
(Assignee)

Comment 4

2 years ago
Created attachment 8692409 [details] [diff] [review]
Add basic tests for IS_COMPONENT/NO_COMPONENTS_MANIFEST
Attachment #8692409 - Flags: review?(gps)
(Assignee)

Comment 5

2 years ago
Created attachment 8692411 [details] [diff] [review]
Emit a specialized object for chrome.manifest entries

Initially, I had a separate patch for FasterMake, but at the moment I was going to attach it, I figured it made no sense to separate it considering its size, so I folded it with this one.
Attachment #8692408 - Attachment is obsolete: true
Attachment #8692408 - Flags: review?(gps)
Attachment #8692411 - Flags: review?(gps)
(Assignee)

Comment 6

2 years ago
Comment on attachment 8692409 [details] [diff] [review]
Add basic tests for IS_COMPONENT/NO_COMPONENTS_MANIFEST

Review of attachment 8692409 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ +938,5 @@
> +        self.assertEqual(objs[0].entry.relpath, objs[1].lib_name)
> +        self.assertIsInstance(objs[1], SharedLibrary)
> +        self.assertEqual(objs[1].basename, 'foo')
> +        self.assertIsInstance(objs[2], SharedLibrary)
> +        self.assertEqual(objs[1].basename, 'bar')

typo: objs[2]
(Assignee)

Comment 7

2 years ago
Created attachment 8692948 [details] [diff] [review]
Emit a specialized object for chrome.manifest entries

With a fixup.
Attachment #8692411 - Attachment is obsolete: true
Attachment #8692411 - Flags: review?(gps)
Attachment #8692948 - Flags: review?(gps)

Updated

2 years ago
Attachment #8692404 - Flags: review?(gps) → review+

Updated

2 years ago
Attachment #8692405 - Flags: review?(gps) → review+

Comment 8

2 years ago
Comment on attachment 8692409 [details] [diff] [review]
Add basic tests for IS_COMPONENT/NO_COMPONENTS_MANIFEST

Review of attachment 8692409 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for adding tests.
Attachment #8692409 - Flags: review?(gps) → review+

Comment 9

2 years ago
Comment on attachment 8692948 [details] [diff] [review]
Emit a specialized object for chrome.manifest entries

Review of attachment 8692948 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/backend/recursivemake.py
@@ +1406,5 @@
> +                make_quote(shell_quote('manifest %s' %
> +                                       mozpath.relpath(obj.path,
> +                                                       obj.install_target))),
> +            ]
> +            rule.add_commands(['$(call py_action,buildlist,%s)' %

It seems like we're a few steps away from writing out fragments of the final file directly instead of having to rely on buildlist. Too much scope bloat for this series?
Attachment #8692948 - Flags: review?(gps) → review+
(Assignee)

Comment 10

2 years ago
(In reply to Gregory Szorc [:gps] from comment #9)
> Comment on attachment 8692948 [details] [diff] [review]
> Emit a specialized object for chrome.manifest entries
> 
> Review of attachment 8692948 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: python/mozbuild/mozbuild/backend/recursivemake.py
> @@ +1406,5 @@
> > +                make_quote(shell_quote('manifest %s' %
> > +                                       mozpath.relpath(obj.path,
> > +                                                       obj.install_target))),
> > +            ]
> > +            rule.add_commands(['$(call py_action,buildlist,%s)' %
> 
> It seems like we're a few steps away from writing out fragments of the final
> file directly instead of having to rely on buildlist. Too much scope bloat
> for this series?

We can only partially write those out, as there's part of their data in jar manifests, and as much as I'd like jar manifests to be handled in the frontend (as written in a comment in fastermake.py), it's opening a large can of worms considering the impact on l10n and XPI_NAME-related stuff.

Those files *can* now be written completely from fastermake.py, now, but we don't have anything generic yet besides buildlist to create files with given generated contents. I'm considering adding an install manifest type for that. But yeah, that's out of scope.

Comment 11

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a2355e3d100
https://hg.mozilla.org/integration/mozilla-inbound/rev/8421887c8042
https://hg.mozilla.org/integration/mozilla-inbound/rev/573992ac6b3d
(Assignee)

Comment 12

2 years ago
https://hg.mozilla.org/comm-central/rev/ba38177b5aa3

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2a2355e3d100
https://hg.mozilla.org/mozilla-central/rev/8421887c8042
https://hg.mozilla.org/mozilla-central/rev/573992ac6b3d
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.