Closed
Bug 472753
Opened 16 years ago
Closed 16 years ago
OS/2 build breaks in nsComponentManager.cpp
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: wuno, Assigned: mozilla)
References
Details
(Keywords: fixed1.9.1)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9.2a1pre) Gecko/20090103 Minefield/3.2a1pre
Build Identifier:
After checkin of bug 462497 OS/2 build breaks
g++ -o nsComponentManager.o -c -DMOZILLA_INTERNAL_API -DOSTYPE=\"OS22.45\" -DOSARCH=OS2 -D_IMPL_NS_COM -IE:/usr/src/hg/mozilla-central/xpcom/components/../reflect/xptinfo/src -IE:/usr/src/hg/mozilla-central/xpcom/components/../base -IE:/usr/src/hg/mozilla-central/xpcom/components/../thread -IE:/usr/src/hg/mozilla-central/xpcom/components/../ds -IE:/usr/src/hg/mozilla-central/xpcom/components/../build -I.. -IE:/usr/src/hg/mozilla-central/xpcom/components -I. -I../../dist/include/string -I../../dist/include -I../../dist/include/xpcom -IE:/mozbuild/dist/include/nspr -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-long-long -pedantic -fno-strict-aliasing -Zomf -pipe -DNDEBUG -DTRIMMED -O2 -DMOZILLA_CLIENT -include ../../mozilla-config.h -Uunix -U__unix -U__unix__ -Wp,-MD,.deps/nsComponentManager.pp E:/usr/src/hg/mozilla-central/xpcom/components/nsComponentManager.cpp
In file included from E:/usr/src/hg/mozilla-central/xpcom/components/nsComponentManager.cpp:87:
E:/mozbuild/dist/include/nspr/private/pprthred.h:354: error: `
EXCEPTIONREGISTRATIONRECORD' was not declared in this scope
E:/mozbuild/dist/include/nspr/private/pprthred.h:354: error: `e' was not
declared in this scope
E:/mozbuild/dist/include/nspr/private/pprthred.h:354: warning: `
PR_OS2_SetFloatExcpHandler' initialized and declared `extern'
E:/mozbuild/dist/include/nspr/private/pprthred.h:354: error: variable or field
`PR_OS2_SetFloatExcpHandler' declared void
E:/mozbuild/dist/include/nspr/private/pprthred.h:354: error: variable `int
PR_OS2_SetFloatExcpHandler' definition is marked dllimport.
E:/mozbuild/dist/include/nspr/private/pprthred.h:355: error: `
EXCEPTIONREGISTRATIONRECORD' was not declared in this scope
E:/mozbuild/dist/include/nspr/private/pprthred.h:355: error: `e' was not
declared in this scope
E:/mozbuild/dist/include/nspr/private/pprthred.h:355: warning: `
PR_OS2_UnsetFloatExcpHandler' initialized and declared `extern'
E:/mozbuild/dist/include/nspr/private/pprthred.h:355: error: variable or field
`PR_OS2_UnsetFloatExcpHandler' declared void
E:/mozbuild/dist/include/nspr/private/pprthred.h:355: error: variable `int
PR_OS2_UnsetFloatExcpHandler' definition is marked dllimport.
E:/usr/src/hg/mozilla-central/xpcom/components/nsComponentManager.cpp: In
member function `void
nsComponentManagerImpl::LoadDeferredModules(nsTArray<DeferredModule>&)':
E:/usr/src/hg/mozilla-central/xpcom/components/nsComponentManager.cpp:3122: warning: comparison
between signed and unsigned integer expressions
E:/usr/src/hg/mozilla-central/xpcom/components/nsComponentManager.cpp: In
member function `virtual nsresult
nsComponentManagerImpl::AutoUnregisterComponent(int, nsIFile*)':
E:/usr/src/hg/mozilla-central/xpcom/components/nsComponentManager.cpp:3164: warning: comparison
between signed and unsigned integer expressions
make.exe[5]: *** [nsComponentManager.o] Error 1
Reproducible: Always
Steps to Reproduce:
1.
2.
3.
Assignee | ||
Comment 1•16 years ago
|
||
That looks like os2.h is included already before the include in pprthred.h. And that #include needs to be prefixed with #define INCL_DOS or something like that.
Comment 2•16 years ago
|
||
--- nsprpub\pr\include\private\pprthred.h.orig
+++ nsprpub\pr\include\privatepprthred.h
@@ -47,6 +47,7 @@
#if defined(XP_OS2)
#define INCL_DOS
#define INCL_DOSERRORS
+#define INCL_DOSEXCEPTIONS
#define INCL_WIN
#include <os2.h>
#endif
?
Reporter | ||
Comment 3•16 years ago
|
||
(In reply to comment #2)
> --- nsprpub\pr\include\private\pprthred.h.orig
> +++ nsprpub\pr\include\privatepprthred.h
> @@ -47,6 +47,7 @@
> #if defined(XP_OS2)
> #define INCL_DOS
> #define INCL_DOSERRORS
> +#define INCL_DOSEXCEPTIONS
> #define INCL_WIN
> #include <os2.h>
> #endif
>
> ?
unfortunately, that doesn't work or is not enough.
I was able to complete the build by if 0ing the inclusion of private\ppthred.h
But why does the inclusion of this header work in toolkit\xre\nsAppRunner.cpp?
64 #ifdef XP_OS2
65 #include "private/pprthred.h"
66 #endif
and would we need it at all on OS/2?
Assignee | ||
Comment 4•16 years ago
|
||
Please preprocess (by changing the g++ command line from "-c -o bla.o" to "-dD -E -o bla.E") the files in question and watch where os2.h gets included first. You can then compare nsAppRunner to nsComponentManager.
Updated•16 years ago
|
Summary: OS/2 build brakes in nsComponentManager.cpp → OS/2 build breaks in nsComponentManager.cpp
Reporter | ||
Comment 5•16 years ago
|
||
(In reply to comment #4)
> Please preprocess (by changing the g++ command line from "-c -o bla.o" to "-dD
> -E -o bla.E") the files in question and watch where os2.h gets included first.
> You can then compare nsAppRunner to nsComponentManager.
I struggled with the commandline length when I copied the lines from the log for nsAppRunner to make the suggested changes (it worked for nsComponentManager). Nevertheless, when I looked at the respective ./deps/*.pp files I saw, that in nsAppRunner.pp os2.h is included directly after pprthred.h, while in nsComponentManager.pp pprthred.h is included at the end, and os2.h was included earlier. I moved the line #include private/pprthred.h a bit around I found that it has to be included before #include nsLocalFile.h
Reporter | ||
Comment 6•16 years ago
|
||
This unbreaks the OS/2 build by moving #include nsLocalFile.h after private/pprthred.h.
Alternatively, one could also move up private/pprthred.h to be included before nsLocalFile.h. Both variants work on OS/2, and neither would brake linux nor Windows (Mac I couldn't test).
Attachment #356314 -
Flags: review?(benjamin)
Assignee | ||
Comment 7•16 years ago
|
||
Walter, does it help to add
#define INCL_DOS
and/or
#define INCL_DOSEXCEPTIONS
to all the other INCL_ defines in xpcom/io/nsLocalFileOS2.h?
Comment 8•16 years ago
|
||
(In reply to comment #6)
> This unbreaks the OS/2 build by moving #include nsLocalFile.h after
> private/pprthred.h.
The order of #include directives shouldn't normally matter. Headers are supposed to have #include for all dependencies, and to use '#ifdef CANONICALIZEDFILENAME' to guard against multiple inclusion.
So the patch looks like a workaround. Is it possible address the cause of the problem?
Reporter | ||
Comment 9•16 years ago
|
||
Comment on attachment 356314 [details] [diff] [review]
move nsLocalFile.h after private/ppthread.h in the list of included headers
(In reply to comment #7)
> Walter, does it help to add
> #define INCL_DOS
> and/or
> #define INCL_DOSEXCEPTIONS
> to all the other INCL_ defines in xpcom/io/nsLocalFileOS2.h?
Both would work (didn't try the combination). Which one is more preferable?
(In reply to comment #8)
> (In reply to comment #6)
> > This unbreaks the OS/2 build by moving #include nsLocalFile.h after
> > private/pprthred.h.
>
> The order of #include directives shouldn't normally matter. Headers are
> supposed to have #include for all dependencies, and to use '#ifdef
> CANONICALIZEDFILENAME' to guard against multiple inclusion.
>
> So the patch looks like a workaround. Is it possible address the cause of the
> problem?
You're right, ask for r canceled in favor for looking at a valid solution for OS/2.
Attachment #356314 -
Attachment is obsolete: true
Attachment #356314 -
Flags: review?(benjamin)
Reporter | ||
Comment 10•16 years ago
|
||
(In reply to comment #9)
> (In reply to comment #7)
> > Walter, does it help to add
> > #define INCL_DOS
> > and/or
> > #define INCL_DOSEXCEPTIONS
> > to all the other INCL_ defines in xpcom/io/nsLocalFileOS2.h?
> Both would work (didn't try the combination). Which one is more preferable?
Peter, Just to test if I understood it correctly what is defined in os2tk45\h\bsedos.h: If we go with #define INCL_DOS in nsLocalFileOS2.h, we can remove except INCL_DOSERRORS all other lines with #define INCL_DOS* from nsLocalFile.h as these are then automatically defined?
Assignee | ||
Comment 11•16 years ago
|
||
Walter, yes, looks like INCL_DOS includes all of them. Please give it a try.
In reply to comment #8)
> The order of #include directives shouldn't normally matter. Headers are
> supposed to have #include for all dependencies, and to use '#ifdef
> CANONICALIZEDFILENAME' to guard against multiple inclusion.
That is correct in theory but not the case on all programming platforms. Some toolkits need #defines before including toolkit headers and then the order does matter, because the same header might be included from different places...
> So the patch looks like a workaround. Is it possible address the cause of the
> problem?
...but yes, in this case we should be able to fix it in OS/2-specific files. Although I am really wondering if private/pprthred.h was meant to be included into stuff outside of NSPR (given the name of the path).
Assignee | ||
Comment 12•16 years ago
|
||
Fix as suggested by Walter in comment 10.
It so far at least got me through compiling XPCOM, if the build completes and runs I'll check it in.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 13•16 years ago
|
||
Took a bit longer because it got too late yesterday, but now I pushed it (http://hg.mozilla.org/mozilla-central/rev/b945b4f67e7e).
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 14•16 years ago
|
||
(In reply to comment #13)
> Took a bit longer because it got too late yesterday, but now I pushed it
> (http://hg.mozilla.org/mozilla-central/rev/b945b4f67e7e).
Peter, could you checkin the patch also to the branch since bug462497 was made it to mozilla-1.9.1 (http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?changeset=7d740ae63d5c)
Assignee | ||
Comment 15•16 years ago
|
||
Sorry, it appears that I completely missed bugmail about this and only found about this when trying to build the next set of nightlies. Pushed to branch now:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/71fb8ce8014f
Keywords: fixed1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•