Closed Bug 1227892 Opened 4 years ago Closed 4 years ago

Change how binary xpcom components manifests are created

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: glandium, Assigned: glandium)

Details

Attachments

(4 files, 2 obsolete files)

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.
The variable is used by Lightning in comm-central and influences
binary-component manifest creation.
Attachment #8692404 - Flags: review?(gps)
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)
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)
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]
With a fixup.
Attachment #8692411 - Attachment is obsolete: true
Attachment #8692411 - Flags: review?(gps)
Attachment #8692948 - Flags: review?(gps)
Attachment #8692404 - Flags: review?(gps) → review+
Attachment #8692405 - Flags: review?(gps) → review+
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 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+
(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.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.