Closed Bug 886526 Opened 6 years ago Closed 6 years ago

kill IS_COMPONENT and MODULE_NAME for things that go into libxul

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla25

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

Details

Attachments

(1 file, 7 obsolete files)

No description provided.
Attachment #766894 - Flags: review?(benjamin)
I ran accross these files which seem to be used for nothing.
Attachment #766895 - Flags: review?(benjamin)
IS_COMPONENT is only used to add some EXTRA_DSO_LDOPTS stuff after the previous patches so there's no reason to specify it if we're only going to build a static library.
MODULE_NAME is used for nothing other than checking that its set afaict
Attachment #767243 - Flags: review?(benjamin)
this goes with the previous patch and removes the stuff that isn't needed anymore and would break the build with just the previous patch.
Attachment #767245 - Flags: review?(benjamin)
Attachment #766894 - Flags: review?(benjamin) → review+
Attachment #766895 - Flags: review?(benjamin) → review+
Attachment #767243 - Flags: review?(benjamin) → review+
Attachment #767245 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/170101f09b3c
https://hg.mozilla.org/mozilla-central/rev/1488f54606a2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I didn't catch this earlier because the old symblinks wer not removed by dep builds, but removing IS_COMPONENT means those static libraries will be coppied to objdir/staticlib/ not objdir/staticlib/components/ because of target_libs.mk:35.  I'm not sure if that effects anything else, but I suspect not and local builds that rm staticlib/ first work after this patch.
Attachment #770956 - Flags: review?(benjamin)
Comment on attachment 770956 [details] [diff] [review]
bug 886526 - adjust toolkit/library/Makefile.in to expect all exported static libs in objdir/staticlib/ and never objdir/staticlib/components/

I think this will work. There's a lot of leftover history from when we supported dynamic and libxul and static builds; now that libxul is required a lot of that distinction is probably irrelevant. I hope!
Attachment #770956 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/4feb9e5a7b68
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Depends on: 898504
This was backed out for causing bug 898504.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
this patch appears to be enough on its on to increase linker max  vsize from 3.45g on inbound to 3.7g on try.  I suspect that what's happening is we end up passing object files to the linker multiple times and it maps them more than once but that remainsto be proved.
(In reply to comment #13)
> Created attachment 783926 [details] [diff] [review]
>   --> https://bugzilla.mozilla.org/attachment.cgi?id=783926&action=edit
> always export libs to just staticlib/
> 
> this patch appears to be enough on its on to increase linker max  vsize from
> 3.45g on inbound to 3.7g on try.  I suspect that what's happening is we end up
> passing object files to the linker multiple times and it maps them more than
> once but that remainsto be proved.

Given what I know from the root cause of this problem in MSVC, your theory is totally plausible.  Thanks for tracking this down (and thanks to Matt for being on top of dev-tree-mgmt!)
(In reply to Trevor Saunders (:tbsaunde) from comment #13)
> Created attachment 783926 [details] [diff] [review]
> always export libs to just staticlib/
> 
> this patch appears to be enough on its on to increase linker max  vsize from
> 3.45g on inbound to 3.7g on try.  I suspect that what's happening is we end
> up passing object files to the linker multiple times and it maps them more
> than once but that remainsto be proved.

You can add --verbose to EXPAND_LIBS_EXEC in config/config.mk. That will give you the exact command line invoked and the contents of the list file used on that command line.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #14)
> (In reply to comment #13)
> > Created attachment 783926 [details] [diff] [review]
> >   --> https://bugzilla.mozilla.org/attachment.cgi?id=783926&action=edit
> > always export libs to just staticlib/
> > 
> > this patch appears to be enough on its on to increase linker max  vsize from
> > 3.45g on inbound to 3.7g on try.  I suspect that what's happening is we end up
> > passing object files to the linker multiple times and it maps them more than
> > once but that remainsto be proved.
> 
> Given what I know from the root cause of this problem in MSVC, your theory
> is totally plausible.  Thanks for tracking this down (and thanks to Matt for

sadly as plausable as it is it doesn't seem to be true, I compared two logs where --verbose were passed, and  the only difference I can see is in the order of object files in the file list we pass the linker.So apparently the order of files is effects the amount of memory the linker uses which I guess isn't that crazy since it could effect how / when you resolve symbols I guess.  This raises the interesting question of if we can order the files in a better way than we currently do.
I think bsmedberg is out, and ted is workweeking so tossing this at glandium
Attachment #787899 - Flags: review?(mh+mozilla)
Attachment #766894 - Attachment is obsolete: true
Attachment #766895 - Attachment is obsolete: true
Attachment #767243 - Attachment is obsolete: true
Attachment #767245 - Attachment is obsolete: true
Attachment #770956 - Attachment is obsolete: true
Attachment #783926 - Attachment is obsolete: true
Attachment #787899 - Attachment is obsolete: true
Attachment #787899 - Flags: review?(mh+mozilla)
Attachment #787919 - Flags: review?(mh+mozilla)
Comment on attachment 787919 [details] [diff] [review]
bug 886526 - remove IS_COMPONENT and MODULE_NAME makefile vars for things in libxul r=bsmedberg

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

::: xpcom/glue/tests/gtest/Makefile.in
@@ +15,2 @@
>  
> +LOCAL_INCLUDES = \ -I$(srcdir)/../.. \

Remove the backslashes.
Attachment #787919 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/cb5af54006d6
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/2dafc2b959cf
disallow MODULE_NAME and IS_COMPONENT for makefiles in libxul r=bsmedberg
https://hg.mozilla.org/comm-central/rev/557b63bc4535
backout because it probably made us use a lot more memory to link on windows
https://hg.mozilla.org/comm-central/rev/ff4064f6df33
remove IS_COMPONENT and MODULE_NAME makefile vars for things in libxul r=bsmedberg r=glandium
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.