Closed
Bug 1352368
Opened 7 years ago
Closed 7 years ago
[Build bustage] Remove old Windows defines from WinMessages.h
Categories
(Core :: Printing: Output, enhancement)
Core
Printing: Output
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: fatseng, Assigned: fatseng)
References
Details
User Story
The issue here is that our WinMessages.h header will cause build bustage if it is included in the expanded include tree before Windows headers like Winuser.h or windows.h. Specifically, to support old Windows SDKs it checks if certain windows defines are defined and if not it defines them. If a more recent Windows header is then included after WinMessages.h we get a compile time error because the Windows header defines the defines unconditionally. Having include order matter is bad since build errors can bite people unexpectedly and in ways that can waste non-trivial amounts of their time. Since we now require VS 2015 to build, I believe all these defines in WinMessages.h are no longer necessary. We should just remove them and eliminate this issue.
Attachments
(1 file)
No description provided.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → fatseng
Assignee | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
Comment on attachment 8853362 [details] Bug 1352368 - Remove some old Windows defines from WinMessages.h There are some redefintion warnings as following. Remove old Windows defines from WinMessages.h to fix them. 1:50.12 Unified_cpp_widget_windows3.cpp 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'WM_GETOBJECT': macro redefinition 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(2032): warning C4005: 'WM_GETOBJECT': macro redefinition 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(72): note: see previous definition of 'WM_GETOBJECT' 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VK_TO_VSC': macro redefinition 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6663): warning C4005: 'MAPVK_VK_TO_VSC': macro redefinition 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(100): note: see previous definition of 'MAPVK_VK_TO_VSC' 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VSC_TO_VK': macro redefinition 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6664): warning C4005: 'MAPVK_VSC_TO_VK': macro redefinition 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(101): note: see previous definition of 'MAPVK_VSC_TO_VK' 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VK_TO_CHAR': macro redefinition 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6665): warning C4005: 'MAPVK_VK_TO_CHAR': macro redefinition 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(102): note: see previous definition of 'MAPVK_VK_TO_CHAR' 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VSC_TO_VK_EX': macro redefinition 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6666): warning C4005: 'MAPVK_VSC_TO_VK_EX': macro redefinition 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(103): note: see previous definition of 'MAPVK_VSC_TO_VK_EX' 1:50.12 Warning: C4005 in C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h: 'MAPVK_VK_TO_VSC_EX': macro redefinition 1:50.12 C:\Program Files (x86)\Windows Kits\10\include\10.0.14393.0\um\winuser.h(6669): warning C4005: 'MAPVK_VK_TO_VSC_EX': macro redefinition 1:50.12 c:\mozilla-source\gecko-0215\obj-i686-pc-mingw32\dist\include\mozilla/widget/WinMessages.h(104): note: see previous definition of 'MAPVK_VK_TO_VSC_EX'
Attachment #8853362 -
Flags: review?(jwatt)
Updated•7 years ago
|
User Story: (updated)
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8853362 [details] Bug 1352368 - Remove some old Windows defines from WinMessages.h https://reviewboard.mozilla.org/r/125480/#review128020 ::: commit-message-7cb42:1 (Diff revision 1) > +Bug 1352368 - Remove old Windows defines from WinMessages.h I think this should be "Remove some old Windows defines from WinMessage.h" since it looks like we're leaving other ones. ::: commit-message-7cb42:2 (Diff revision 1) > +Bug 1352368 - Remove old Windows defines from WinMessages.h > +We require Visual Studio 2015 to build, so we no longer need these defines in WinMessages.h Make this "These defines cause include ordering issues that can result in build errors if WinMessages.h is included before certain Windows headers. To solve this issue this commit simply removes those defines since nowadays we require Visua Studio 2015 to build and no longer need them." And blank line between the summary line and the extended comment, plesae.
Attachment #8853362 -
Flags: review?(jwatt) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8853362 [details] Bug 1352368 - Remove some old Windows defines from WinMessages.h https://reviewboard.mozilla.org/r/125480/#review128138
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8c9fcd6aef01 Remove some old Windows defines from WinMessages.h r=jwatt
Keywords: checkin-needed
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8c9fcd6aef01
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee | ||
Comment 9•7 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff616b2f8508dfa69e152025a5e6c0cefa66f67b&selectedJob=88600186
You need to log in
before you can comment on or make changes to this bug.
Description
•