Closed Bug 1413728 Opened 5 years ago Closed 4 years ago

Determine correct behavior for LDFLAGS on Windows DMD builds


(Firefox Build System :: General, enhancement)

Not set


(firefox63 fixed)

Tracking Status
firefox63 --- fixed


(Reporter: chmanchester, Assigned: away)




(1 file)

There's an anomaly in how we build out ldflags for DMD builds on windows, discussed in bug 1411712. On Windows opt DMD builds, we override all OS_LDFLAGS with "-DEBUG". I traced this behavior back to bug 141834, which pre-dates DMD landing by about 10 years, so we should determine whether this is still what we want to be doing.
I don't know why that code works that way, but I think we should just remove it and ensure that MOZ_DMD implies MOZ_DEBUG_SYMBOLS.
Product: Core → Firefox Build System
Compared to a build without this patch, this cuts out: -LARGEADDRESSAWARE -NXCOMPAT -RELEASE -DYNAMICBASE

-NXCOMPAT and -DYNAMICBASE are no-ops because they are on by default. In fact I'd like to remove them altogether in a followup. -LARGEADDRESSAWARE is on by default in 64-bit builds. -RELEASE merely sets the binary's checksum. I very much doubt that DMD is going to notice any of this.

Also, I don't feel particularly inclined to make MOZ_DMD imply MOZ_DEBUG_SYMBOLS. The latter has been on by default for internet-eons. If anyone builds --disable-debug-symbols on Windows, they brought it on themselves.

(My motivation to take this bug is to clean up handling of `-DEBUG` in preparation to use `-DEBUG:GHASH` with lld-link.)
Assignee: nobody → dmajor
Attachment #8998999 - Flags: review?(ted)
Blocks: 1482272
Comment on attachment 8998999 [details] [diff] [review]
Windows DMD builds don't need special LDFLAGS behavior.

Review of attachment 8998999 [details] [diff] [review]:

Sounds good to me! Walking back weird historical oddities like this can be hard to justify, but it's definitely worthwhile to have less complexity to deal with.
Attachment #8998999 - Flags: review?(ted) → review+
Pushed by
Windows DMD builds don't need special LDFLAGS behavior. r=ted
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.