Closed Bug 1423802 Opened 7 years ago Closed 7 years ago

Link commands using the C compiler are linking C++ libraries

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox59 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(2 files)

Bug 1423802 - Handle stdc++compat and STLPORT_LIBS at the emitter level.
(deleted), text/x-review-board-request
nalexander
: review+
Details
Bug 1423802 - Remove the dummy fallible library.
(deleted), text/x-review-board-request
Details
Specifically, they are linking stdc++compat and STLPORT_LIBS through the Binary() template in build/templates.mozbuild.
Comment on attachment 8935239 [details] Bug 1423802 - Handle stdc++compat and STLPORT_LIBS at the emitter level. https://reviewboard.mozilla.org/r/206122/#review211914 I'm not expert in this area, but I spent some time trying to understand what is happening here. It looks like you're duplicating a lot of the existing functionality of the helper methods like `_link_libraries`, and I can't understand why. That is, in a method that has a general "do these things with these variables", you're adding code that does special parts of the above rather than updating the relevant variables and then doing the general method. Can you explain why that is so? You might prefer to get review from somebody else who might not have the same concern.
Comment on attachment 8935240 [details] Bug 1423802 - Remove the dummy fallible library. https://reviewboard.mozilla.org/r/206124/#review211916 If the previous commit is good, then this will be good too.
Attachment #8935240 - Flags: review+
(In reply to Nick Alexander :nalexander (less responsive until Jan 3, 2018) from comment #3) > Comment on attachment 8935239 [details] > Bug 1423802 - Handle stdc++compat and STLPORT_LIBS at the emitter level. > > https://reviewboard.mozilla.org/r/206122/#review211914 > > I'm not expert in this area, but I spent some time trying to understand what > is happening here. It looks like you're duplicating a lot of the existing > functionality of the helper methods like `_link_libraries`, and I can't > understand why. That is, in a method that has a general "do these things > with these variables", you're adding code that does special parts of the > above rather than updating the relevant variables and then doing the general > method. > > Can you explain why that is so? Because the relevant variables can only be updated after obj.cxx_link is final, which it isn't before every other library listed in the relevant variable has already been handled. I updated the patch to make things clearer in the final code, at the expense of more churn in the patch itself.
Comment on attachment 8935239 [details] Bug 1423802 - Handle stdc++compat and STLPORT_LIBS at the emitter level. https://reviewboard.mozilla.org/r/206122/#review212620 Thanks for reworking this; it's much more clear. And the commit message was first rate in the first version, too!
Attachment #8935239 - Flags: review+
Attachment #8935239 - Flags: review?(core-build-config-reviews)
Attachment #8935240 - Flags: review?(core-build-config-reviews)
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/d7fcb5272323 Handle stdc++compat and STLPORT_LIBS at the emitter level. r=nalexander https://hg.mozilla.org/integration/autoland/rev/8c677030915b Remove the dummy fallible library. r=nalexander
https://hg.mozilla.org/mozilla-central/rev/d7fcb5272323 https://hg.mozilla.org/mozilla-central/rev/8c677030915b
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: