Closed Bug 1022847 Opened 5 years ago Closed 5 years ago
toolkit/mozapps/update fails to compile with Min
The code under toolkit/mozapps/update does not compile with the mingw-w64 compiler and headers (we encountered this problem doing a 32-bit build for the Tor Browser). Most of the problems seem to be caused by differences between the Microsoft header files and the MinGW ones.
(In reply to Kathleen Brade from comment #0) > Most of the problems seem to be caused by differences > between the Microsoft header files and the MinGW ones. Those ideally should be fixed in mingw-w64. I tried to look at them, so I tried building updater myself. I didn't need _WIN32_WINNT changes nor other include changes. What mingw-w64 version are you using? I will attach the patch I ended with. It's just for error fixes, no warning fixes, just for reference. Also extern "C" changes are probably needed for GCC older than 4.9.
(In reply to Jacek Caban from comment #1) > Those ideally should be fixed in mingw-w64. I tried to look at them, so I > tried building updater myself. I didn't need _WIN32_WINNT changes nor other > include changes. What mingw-w64 version are you using? We were using an older version of mingw-w64. We switched to gcc 4.8.3 and mingw-w64 3.1.0 and some of the problems did go away. However, some of the issues remain. We are cross-compiling on a Linux system and we are mainly working with ESR 24. We will try cross-compiling mozilla-central in order to confirm that your patch fixes all of the issues. We will need to also keep the extern "C" for NS_main() for now because we are not ready to jump to gcc 4.9.
Our mozilla-central build completed successfully using your patch. Is it too late to include this in Firefox 31?
Comment on attachment 8438311 [details] [diff] [review] patch.diff Brian, could I get you to review this? Thanks!
Attachment #8438311 - Flags: review?(netzen)
Attachment #8438311 - Flags: review?(netzen) → review+
(In reply to Kathleen Brade from comment #4) > Our mozilla-central build completed successfully using your patch. Is it > too late to include this in Firefox 31? After this makes it to mozilla-central we can evaluate whether we can take it for Firefox 31.
Assignee: brade → jacek
Pushed to try https://tbpl.mozilla.org/?tree=Try&rev=c78e93956822 If all goes well I'll land this
c:/builds/moz2_slave/try-w64-0000000000000000000000/build/toolkit/mozapps/update/updater/updater.cpp(2988) : error C2589: '(' : illegal token on right side of '::' c:/builds/moz2_slave/try-w64-0000000000000000000000/build/toolkit/mozapps/update/updater/updater.cpp(2988) : error C2059: syntax error : '::' :(
Is that on the max call? #undef max might help at the top?
I guess we should define NOMINMAX for Windows builds. That prevents windef.h from defining min/max macros. I will send a fixed version to try server soon.
I pushed wrong version. Here is the right one: https://tbpl.mozilla.org/?tree=Try&rev=7cfb7bbd3e11
Still busted. I think bbondy may be correct in comment #9 as well http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/client/crashreporter_win.cpp#40
I ran into a similar problem with sandboxing, if I recall correctly there was an implicit compile time include specified that was including windows early. So #undef'ing the max macro worked for me.
The problem from the last push is quite different. Using NOMINMAX, max is not defined. However, this can't be found now. It's declared in algorithm header. The version that I accidentally pushed had it included (although missed NOMINMAX part) and that caused another error: #error : "STL code can only be used with infallible ::operator new()" I may take a look at this later, but maybe someone knows the solution off-hand?
Try setting DISABLE_STL_WRAPPING = True in the moz.build file under the conditions for mingw
This works, thanks: https://tbpl.mozilla.org/?tree=Try&rev=adce3dc47f89 I will land it tomorrow unless you think that needs a new review.
Sorry for the delay. https://hg.mozilla.org/integration/mozilla-inbound/rev/7bbedf6937aa
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment on attachment 8438311 [details] [diff] [review] patch.diff Review of attachment 8438311 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/update/common/updatehelper.cpp @@ -10,4 @@ > #define _WIN32_WINNT 0x602 > #include <objbase.h> > #include <shobjidl.h> > -#pragma comment(lib, "ole32.lib") Removing this line broke --enable-metro builds (bug 1034127). Since this line is inside an #ifdef MOZ_METRO block, it should have no effect on default builds. Can we restore this line?
I'm sorry for intoducing this. I'm away on vacations now so I can't prepare a patch. Moving it to moz.build faile would be even nicer than restoring the file. I will do that on Thursday.
I pushed a partial backout: http://hg.mozilla.org/integration/mozilla-inbound/rev/6d179aa6e46f I decided that moving it to moz.build file may wait for bug 969321 and this way I could push the fix right away.
You need to log in before you can comment on or make changes to this bug.