Open Bug 1223530 Opened 4 years ago Updated 8 months ago
_WINCONSOLE to configure
40 bytes, text/x-review-board-request
MOZ_WINCONSOLE has been cargo-culted into a bunch of Makefile.ins, but we really could just set it globally in configure. https://dxr.mozilla.org/mozilla-central/search?q=MOZ_WINCONSOLE+path%3AMakefile.in&redirect=true&case=true
bug 1223530 - Move MOZ_WINCONSOLE to configure. r?gps This patch moves the logic for selecting MOZ_WINCONSOLE out of individual Makefile.in files and into configure. It also changes config.mk to only pass -SUBSYSTEM:CONSOLE if MOZ_WINCONSOLE=1. The MSDN docs state that in the absence of -SUBSYSTEM, the linker will select the proper subsystem based on whether the program contains [w]main or [w]WinMain, so let it do that. One program (windbgdlg) needed a tweak to add a wmain for when MOZ_WINCONSOLE is defined. This patch leaves one instance in security/sandbox/win/wow_helper/Makefile.in, that Makefile has its own separate bug.
Attachment #8687917 - Flags: review?(gps)
Turns out to have been slightly more involved than I expected, but not too bad. This builds fine locally on Windows opt/debug, Try push just to make sure.
https://reviewboard.mozilla.org/r/25261/#review22779 ::: toolkit/crashreporter/client/Makefile.in (Diff revision 1) > -MOZ_WINCONSOLE = 0 Your change makes the crashreporter client use the console on debug builds. This may cause problems depending whether this was really intended in the first place. Same for windbgdlg. ::: toolkit/mozapps/update/tests/Makefile.in (Diff revision 1) > -MOZ_WINCONSOLE = 1 Here, the console was always enabled. In likeliness, this was done on purpose to debug local opt builds. ::: toolkit/xre/test/win/Makefile.in (Diff revision 1) > -MOZ_WINCONSOLE = 1 Same here
(In reply to Mike Hommey [:glandium] from comment #5) > https://reviewboard.mozilla.org/r/25261/#review22779 > > ::: toolkit/crashreporter/client/Makefile.in > (Diff revision 1) > > -MOZ_WINCONSOLE = 0 > > Your change makes the crashreporter client use the console on debug builds. > This may cause problems depending whether this was really intended in the > first place. Yes, I know. I tested this locally and it still works fine. (In reply to Mike Hommey [:glandium] from comment #5) > Here, the console was always enabled. In likeliness, this was done on > purpose to debug local opt builds. This will continue to work fine, since in the absence of MOZ_WINCONSOLE=1, we don't pass any -SUBSYSTEM flag to the linker, so it will choose the proper subsystem based on the type of main symbol present.
Comment on attachment 8687917 [details] MozReview Request: bug 1223530 - Move MOZ_WINCONSOLE to configure. r?gps Reassigning to glandium since he already started review.
Attachment #8687917 - Flags: review?(gps) → review?(mh+mozilla)
Attachment #8687917 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8687917 [details] MozReview Request: bug 1223530 - Move MOZ_WINCONSOLE to configure. r?gps https://reviewboard.mozilla.org/r/25261/#review23147
https://hg.mozilla.org/integration/mozilla-inbound/rev/0104f1c911ca36aaf413df211ebc7320f61ed853 bug 1223530 - Move MOZ_WINCONSOLE to configure. r=glandium
This had to be backed out because it broke Windows XP. As philor puts it, "chalk up another victory for not-by-default on try". https://hg.mozilla.org/integration/mozilla-inbound/rev/27b37512f3f3 This stems from the change in config.mk, because we *do* need to pass -SUBSYSTEM:something because we *also* are specifying a different subsystem version to make things work on Windows XP, which the linker defaults happily don't care about. Which also means we can't rely on the linker picking up the right thing from the main() function used.
Is this still going to be done? If not I'd like to close the dependent bug.
Yes, I still want to fix this. This might actually be easier in light of the fact that we've dropped WinXP support now.
Is this wanted in time for for version 60?
You need to log in before you can comment on or make changes to this bug.