Closed Bug 939074 Opened 6 years ago Closed 6 years ago

Infer LIBXUL_LIBRARY from FINAL_LIBRARY

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla28

People

(Reporter: glandium, Assigned: glandium)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files, 2 obsolete files)

No description provided.
Depends on: 939622
Attached patch Remove most LIBXUL_LIBRARY (obsolete) — Splinter Review
This needs to be folded with the other patch, but that would make review harder. Note that after this there *are* a few LIBXUL_LIBRARY left, that's for gtest, and we can't do otherwise just yet.
Attachment #8333615 - Flags: review?(gps)
Comment on attachment 8333614 [details] [diff] [review]
Infer LIBXUL_LIBRARY from FINAL_LIBRARY

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

::: python/mozbuild/mozbuild/frontend/emitter.py
@@ +113,5 @@
>              libs.values()[0].link_static_lib(reldir, libname)
> +            self._libs[libname][reldir].refcount += 1
> +            # The refcount can't go above 1 right now. It might in the future,
> +            # but that will have to be specifically handled. At which point the
> +            # refcount might have to be a list of referencees, for better error

'references'

@@ +115,5 @@
> +            # The refcount can't go above 1 right now. It might in the future,
> +            # but that will have to be specifically handled. At which point the
> +            # refcount might have to be a list of referencees, for better error
> +            # reporting.
> +            assert self._libs[libname][reldir].refcount <= 1:

Drop the colon here
(In reply to :Ms2ger from comment #3)
> Comment on attachment 8333614 [details] [diff] [review]
> Infer LIBXUL_LIBRARY from FINAL_LIBRARY
> 
> Review of attachment 8333614 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: python/mozbuild/mozbuild/frontend/emitter.py
> @@ +113,5 @@
> >              libs.values()[0].link_static_lib(reldir, libname)
> > +            self._libs[libname][reldir].refcount += 1
> > +            # The refcount can't go above 1 right now. It might in the future,
> > +            # but that will have to be specifically handled. At which point the
> > +            # refcount might have to be a list of referencees, for better error
> 
> 'references'

That was not a typo. It's probably not a word, though.

> @@ +115,5 @@
> > +            # The refcount can't go above 1 right now. It might in the future,
> > +            # but that will have to be specifically handled. At which point the
> > +            # refcount might have to be a list of referencees, for better error
> > +            # reporting.
> > +            assert self._libs[libname][reldir].refcount <= 1:
> 
> Drop the colon here

Gah, I fixed it but in the wrong patch.
Fixed typo.
Attachment #8333746 - Flags: review?(gps)
Attachment #8333614 - Attachment is obsolete: true
Attachment #8333614 - Flags: review?(gps)
There was one remaining in mobile/android/components/build.
Attachment #8333788 - Flags: review?(gps)
Attachment #8333615 - Attachment is obsolete: true
Attachment #8333615 - Flags: review?(gps)
Comment on attachment 8333746 [details] [diff] [review]
Infer LIBXUL_LIBRARY from FINAL_LIBRARY

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

Some tests would be nice. I also wouldn't mind some docs in build/docs when this stuff finally gets to a somewhat stable state.

::: python/mozbuild/mozbuild/frontend/emitter.py
@@ +128,5 @@
> +            for path, libdef in libs.items():
> +                # For all root libraries (i.e. libraries that don't have a
> +                # FINAL_LIBRARY), record, for each static library it links
> +                # (recursively), that its FINAL_LIBRARY is that root library.
> +                if libdef.refcount == 0:

if not libdef.refcount:

(I don't like seeing things compared to falsy values, ever)
Attachment #8333746 - Flags: review?(gps) → review+
Comment on attachment 8333788 [details] [diff] [review]
Remove most LIBXUL_LIBRARY

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

rs=me.
Attachment #8333788 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/6d271ac31d9a
https://hg.mozilla.org/mozilla-central/rev/71cbd5b14aec
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Blocks: 943197
Whiteboard: [qa-]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.