Closed
Bug 799121
Opened 13 years ago
Closed 13 years ago
Build problems with nrappkit and 8.0 sdk
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla19
People
(Reporter: jimm, Assigned: jimm)
References
Details
(Whiteboard: [metro-mvp][LOE:1][completed-elm])
Attachments
(3 files, 4 obsolete files)
|
11.85 KB,
patch
|
bbondy
:
review+
ehsan.akhgari
:
feedback+
|
Details | Diff | Splinter Review |
|
12.34 KB,
patch
|
Details | Diff | Splinter Review | |
|
657 bytes,
patch
|
Details | Diff | Splinter Review |
The cause here might be one of two things - new defines in the 8.0 sdk, or perhaps this has something to do with the upped win32 platform defines we set for elm builds which expose more from the sdk.
Need to do an mc build to see if this can reproduce there. For the time being I'm disabling webrtc on Elm.
jim@CAGE /F/Mozilla/firefox/ELM-REL/media/mtransport/third_party/nrappkit
$ pymake
make.py[0]: Entering directory 'f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit'
make.py[1]: Entering directory 'f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit'
make.py[2]: Entering directory 'f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit\nrappkit_nrappkit'
make.py[2]: Leaving directory 'f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit\nrappkit_nrappkit'
make.py[1]: Leaving directory 'f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit'
make.py[1]: Entering directory 'f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit'
make.py[2]: Entering directory 'f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit\nrappkit_nrappkit'
r_time.c
f:\Mozilla\firefox\elm\config\rules.mk:946:0$ cl InvokeClWithDependencyGeneration cl -Fosrc/util/libekr/r_time.obj -c -D_WIN32_WINNT=0x0602 -DWINVER=0x0602 -DWIN32 -D_WINDOWS -DNOMINMAX -DPSAPI_VERSION=1 -D_CRT_RAND_S -DCERT
_CHAIN_PARA_HAS_EXTRA_FIELDS -DWIN32_LEAN_AND_MEAN -D_ATL_NO_OPENGL -D_HAS_EXCEPTIONS=0 -D_SECURE_ATL -DCHROMIUM_BUILD -DTOOLKIT_VIEWS=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_CONFIGURATION_P
OLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_GPU=1 -DENABLE_EGLIMAGE=1 -DUSE_SKIA=1 -D__STD_C -D_CRT_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_DEPRECATE -DENABLE_TASK_MANAGER=1 -DENABLE_WEB_INTENTS=1 -DENABLE_EXTENS
IONS=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PROTECTOR_SERVICE=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_BACKGROUND=1 -DENABLE_AUTOMATION=1 -DENABLE_PRINTING=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DSANITY_CHEC
KS -DR_PLATFORM_INT_TYPES='"mozilla/StandardInteger.h"' -DR_DEFINED_INT2=int16_t -DR_DEFINED_UINT2=uint16_t -DR_DEFINED_INT4=int32_t -DR_DEFINED_UINT4=uint32_t -DR_DEFINED_INT8=int64_t -DR_DEFINED_UINT8=uint64_t -DWIN -D__UNU
SED__="" -DHAVE_STRDUP=1 -DNO_REG_RPC -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DUNICODE -D_UNICODE -I. -I../../../../../../elm/media/mtransport/third_party/n
rappkit//../../../webrtc/trunk/third_party/wtl/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/event -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/log -I../../../../../../elm/me
dia/mtransport/third_party/nrappkit//src/port/generic/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/registry -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/share -I../../../../
../../elm/media/mtransport/third_party/nrappkit//src/stats -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/util -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/util/libekr -I../../../../.
./dist/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/port/win32/include -I"C:\Program Files (x86)\Microsoft DirectX SDK (June 2010)/include" -TC -nologo -W3 -Gy -Fdgenerated.pdb -we4553 -DNDEBU
G -DTRIMMED -Zi -UDEBUG -DNDEBUG -O1 -Oy -MD -FI ../../../../../dist/include/mozilla-config.h -DMOZILLA_CLIENT f:/Mozilla/firefox/ELM-REL/media/mtransport/third_party/nrappkit/nrappkit_nrappkit/../../../../../..
/elm/media/mtransport/third_party/nrappkit/src/util/libekr/r_time.c
r_time.c
f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit\nrappkit_nrappkit\../../../../../dist/include/mozilla-config.h(109) : warning C4005: 'WINVER' : macro redefinition
command-line arguments : see previous definition of 'WINVER'
f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit\nrappkit_nrappkit\../../../../../dist/include/mozilla-config.h(116) : warning C4005: '_WIN32_WINNT' : macro redefinition
command-line arguments : see previous definition of '_WIN32_WINNT'
f:/Mozilla/firefox/ELM-REL/media/mtransport/third_party/nrappkit/nrappkit_nrappkit/../../../../../../elm/media/mtransport/third_party/nrappkit/src/util/libekr/r_time.c(172) : warning C4244: '=' : conversion from 'UINT8' to 'l
ong', possible loss of data
byteorder.c
f:\Mozilla\firefox\elm\config\rules.mk:946:0$ cl InvokeClWithDependencyGeneration cl -Fosrc/util/byteorder.obj -c -D_WIN32_WINNT=0x0602 -DWINVER=0x0602 -DWIN32 -D_WINDOWS -DNOMINMAX -DPSAPI_VERSION=1 -D_CRT_RAND_S -DCERT_CHA
IN_PARA_HAS_EXTRA_FIELDS -DWIN32_LEAN_AND_MEAN -D_ATL_NO_OPENGL -D_HAS_EXCEPTIONS=0 -D_SECURE_ATL -DCHROMIUM_BUILD -DTOOLKIT_VIEWS=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_CONFIGURATION_POLIC
Y -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_GPU=1 -DENABLE_EGLIMAGE=1 -DUSE_SKIA=1 -D__STD_C -D_CRT_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_DEPRECATE -DENABLE_TASK_MANAGER=1 -DENABLE_WEB_INTENTS=1 -DENABLE_EXTENSIONS
=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PROTECTOR_SERVICE=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_BACKGROUND=1 -DENABLE_AUTOMATION=1 -DENABLE_PRINTING=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DSANITY_CHECKS -
DR_PLATFORM_INT_TYPES='"mozilla/StandardInteger.h"' -DR_DEFINED_INT2=int16_t -DR_DEFINED_UINT2=uint16_t -DR_DEFINED_INT4=int32_t -DR_DEFINED_UINT4=uint32_t -DR_DEFINED_INT8=int64_t -DR_DEFINED_UINT8=uint64_t -DWIN -D__UNUSED_
_="" -DHAVE_STRDUP=1 -DNO_REG_RPC -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DUNICODE -D_UNICODE -I. -I../../../../../../elm/media/mtransport/third_party/nrapp
kit//../../../webrtc/trunk/third_party/wtl/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/event -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/log -I../../../../../../elm/media/
mtransport/third_party/nrappkit//src/port/generic/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/registry -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/share -I../../../../../.
./elm/media/mtransport/third_party/nrappkit//src/stats -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/util -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/util/libekr -I../../../../../di
st/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/port/win32/include -I"C:\Program Files (x86)\Microsoft DirectX SDK (June 2010)/include" -TC -nologo -W3 -Gy -Fdgenerated.pdb -we4553 -DNDEBUG -D
TRIMMED -Zi -UDEBUG -DNDEBUG -O1 -Oy -MD -FI ../../../../../dist/include/mozilla-config.h -DMOZILLA_CLIENT f:/Mozilla/firefox/ELM-REL/media/mtransport/third_party/nrappkit/nrappkit_nrappkit/../../../../../../elm
/media/mtransport/third_party/nrappkit/src/util/byteorder.c
byteorder.c
f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit\nrappkit_nrappkit\../../../../../dist/include/mozilla-config.h(109) : warning C4005: 'WINVER' : macro redefinition
command-line arguments : see previous definition of 'WINVER'
f:\Mozilla\firefox\ELM-REL\media\mtransport\third_party\nrappkit\nrappkit_nrappkit\../../../../../dist/include/mozilla-config.h(116) : warning C4005: '_WIN32_WINNT' : macro redefinition
command-line arguments : see previous definition of '_WIN32_WINNT'
C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\INCLUDE\errno.h(102) : warning C4005: 'EHOSTUNREACH' : macro redefinition
f:\Mozilla\firefox\elm\media\mtransport\third_party\nrappkit\src\port\win32\include\csi_platform.h(84) : see previous definition of 'EHOSTUNREACH'
f:/Mozilla/firefox/ELM-REL/media/mtransport/third_party/nrappkit/nrappkit_nrappkit/../../../../../../elm/media/mtransport/third_party/nrappkit/src/util/byteorder.c(57) : error C2084: function 'unsigned __int64 htonll(unsigned
__int64)' already has a body
C:\Program Files (x86)\Windows Kits\8.0\include\um\winsock2.h(1831) : see previous definition of 'htonll'
f:/Mozilla/firefox/ELM-REL/media/mtransport/third_party/nrappkit/nrappkit_nrappkit/../../../../../../elm/media/mtransport/third_party/nrappkit/src/util/byteorder.c(73) : error C2084: function 'unsigned __int64 ntohll(unsigned
__int64)' already has a body
C:\Program Files (x86)\Windows Kits\8.0\include\um\winsock2.h(1839) : see previous definition of 'ntohll'
2
Traceback (most recent call last):
File "f:\Mozilla\firefox\mc\build\pymake\pymake\process.py", line 251, in run
rv = m.__dict__[self.method](self.argv)
File "../../../../../../elm/build\cl.py", line 45, in InvokeClWithDependencyGeneration
sys.exit(ret)
SystemExit: 2
f:\Mozilla\firefox\elm\config\rules.mk:946:0: command 'cl InvokeClWithDependencyGeneration cl -Fosrc/util/byteorder.obj -c -D_WIN32_WINNT=0x0602 -DWINVER=0x0602 -DWIN32 -D_WINDOWS -DNOMINMAX -DPSAPI_VERSION=1 -D_CRT_RAND_S -
DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DWIN32_LEAN_AND_MEAN -D_ATL_NO_OPENGL -D_HAS_EXCEPTIONS=0 -D_SECURE_ATL -DCHROMIUM_BUILD -DTOOLKIT_VIEWS=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_CONFIGURAT
ION_POLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_GPU=1 -DENABLE_EGLIMAGE=1 -DUSE_SKIA=1 -D__STD_C -D_CRT_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_DEPRECATE -DENABLE_TASK_MANAGER=1 -DENABLE_WEB_INTENTS=1 -DENABLE_E
XTENSIONS=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PROTECTOR_SERVICE=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_BACKGROUND=1 -DENABLE_AUTOMATION=1 -DENABLE_PRINTING=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DSANITY
_CHECKS -DR_PLATFORM_INT_TYPES='"mozilla/StandardInteger.h"' -DR_DEFINED_INT2=int16_t -DR_DEFINED_UINT2=uint16_t -DR_DEFINED_INT4=int32_t -DR_DEFINED_UINT4=uint32_t -DR_DEFINED_INT8=int64_t -DR_DEFINED_UINT8=uint64_t -DWIN -D
__UNUSED__="" -DHAVE_STRDUP=1 -DNO_REG_RPC -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DUNICODE -D_UNICODE -I. -I../../../../../../elm/media/mtransport/third_pa
rty/nrappkit//../../../webrtc/trunk/third_party/wtl/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/event -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/log -I../../../../../../e
lm/media/mtransport/third_party/nrappkit//src/port/generic/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/registry -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/share -I../../.
./../../../elm/media/mtransport/third_party/nrappkit//src/stats -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/util -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/util/libekr -I../../..
/../../dist/include -I../../../../../../elm/media/mtransport/third_party/nrappkit//src/port/win32/include -I"C:\Program Files (x86)\Microsoft DirectX SDK (June 2010)/include" -TC -nologo -W3 -Gy -Fdgenerated.pdb -we4553 -D
NDEBUG -DTRIMMED -Zi -UDEBUG -DNDEBUG -O1 -Oy -MD -FI ../../../../../dist/include/mozilla-config.h -DMOZILLA_CLIENT f:/Mozilla/firefox/ELM-REL/media/mtransport/third_party/nrappkit/nrappkit_nrappkit/../../../../
../../elm/media/mtransport/third_party/nrappkit/src/util/byteorder.c' failed, return code -127
f:\Mozilla\firefox\elm\config\makefiles\target_libs.mk:27:0: command 'f:/mozilla-build/python/python.exe f:/Mozilla/firefox/mc/build/pymake/pymake/../make.py -C nrappkit_nrappkit libs' failed, return code 2
f:\Mozilla\firefox\elm\config\rules.mk:560:0: command 'f:/mozilla-build/python/python.exe f:/Mozilla/firefox/mc/build/pymake/pymake/../make.py libs' failed, return code 2
| Assignee | ||
Comment 1•13 years ago
|
||
Hmm, building without webrtc seems broken too -
symbols.def : error LNK2001: unresolved external symbol vpx_codec_vp8_cx
gkmedias.lib : fatal error LNK1120: 1 unresolved externals
Comment 2•13 years ago
|
||
That's fixed by a patch we just landed on m-c in
https://hg.mozilla.org/mozilla-central/rev/129e65729274
| Assignee | ||
Comment 3•13 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #2)
> That's fixed by a patch we just landed on m-c in
> https://hg.mozilla.org/mozilla-central/rev/129e65729274
Ah, thanks!
Updated•13 years ago
|
Whiteboard: [blocking-webrtc+]
Updated•13 years ago
|
Whiteboard: [blocking-webrtc+] → [WebRTC] [blocking-webrtc+]
Comment 4•13 years ago
|
||
I see another problem on Win8 VS2012
e:\mozdev\objdir\media\mtransport\third_party\nrappkit\nrappkit_nrappkit\../../../../../dist/include/mozilla-config.h(108) : warning C4005: 'WINVER' : macro redefinition
command-line arguments : see previous definition of 'WINVER'
e:\mozdev\objdir\media\mtransport\third_party\nrappkit\nrappkit_nrappkit\../../../../../dist/include/mozilla-config.h(116) : warning C4005: '_WIN32_WINNT' : macro redefinition
command-line arguments : see previous definition of '_WIN32_WINNT'
E:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\INCLUDE\errno.h(102) : warning C4005: 'EHOSTUNREACH' : macro redefinition
e:\mozdev\mozilla-central\media\mtransport\third_party\nrappkit\src\port\win32\include\csi_platform.h(84) : see previous definition of 'EHOSTUNREACH'
e:/mozdev/mozilla-central/media/mtransport/third_party/nrappkit/src/util/byteorder.c(57) : error C2084: function 'unsigned __int64 htonll(unsigned __int64)' already has a body
C:\Program Files (x86)\Windows Kits\8.0\include\um\winsock2.h(1831) : see previous definition of 'htonll'
e:/mozdev/mozilla-central/media/mtransport/third_party/nrappkit/src/util/byteord
er.c(73) : error C2084: function 'unsigned __int64 ntohll(unsigned __int64)' already has a body
C:\Program Files (x86)\Windows Kits\8.0\include\um\winsock2.h(1839) : see previous definition of 'ntohll'
make[3]: *** [src/util/byteorder.obj] Error 2
make[3]: Leaving directory `/e/mozdev/objdir/media/mtransport/third_party/nrappkit/nrappkit_nrappkit'
make[2]: *** [nrappkit_nrappkit_libs] Error 2
make[2]: Leaving directory `/e/mozdev/objdir/media/mtransport/third_party/nrappkit'
make[1]: *** [nrappkit_libs] Error 2
make[1]: Leaving directory `/e/mozdev/objdir/media/mtransport/third_party'
make: *** [all] Error 2
make: Leaving directory `/e/mozdev/objdir/media/mtransport/third_party'
| Assignee | ||
Updated•13 years ago
|
Whiteboard: [WebRTC] [blocking-webrtc+] → [WebRTC] [blocking-webrtc+][metro-mvp]
| Assignee | ||
Updated•13 years ago
|
Summary: Build problems with nrappkit and vs 2012 → Build problems with nrappkit and 8.0 sdk
| Assignee | ||
Updated•13 years ago
|
Whiteboard: [WebRTC] [blocking-webrtc+][metro-mvp] → [WebRTC] [blocking-webrtc+][metro-mvp][LOE:1]
| Assignee | ||
Updated•13 years ago
|
Assignee: nobody → jmathies
| Assignee | ||
Comment 5•13 years ago
|
||
This problem doesn't show up if you're building without metro bits. enable-metro ups WINVER, which introduced this. I'm going to try and up the api defines in winrt code so we can avoid the problem.
| Assignee | ||
Updated•13 years ago
|
Blocks: enable-8.0sdk
| Assignee | ||
Comment 6•13 years ago
|
||
| Assignee | ||
Comment 7•13 years ago
|
||
added a comment in the make file on this.
Attachment #677756 -
Attachment is obsolete: true
| Assignee | ||
Updated•13 years ago
|
Whiteboard: [WebRTC] [blocking-webrtc+][metro-mvp][LOE:1] → [metro-mvp][LOE:1]
Updated•13 years ago
|
Whiteboard: [metro-mvp][LOE:1] → [WebRTC][blocking-webrtc+][metro-mvp][LOE:1]
| Assignee | ||
Comment 8•13 years ago
|
||
Jason, do you really want those? This is an issue specific to our upping of the WINVER compile directive for winrt builds. I'm not sure this should block anything related to webrtc landings on mc.
Comment 9•13 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #8)
> Jason, do you really want those? This is an issue specific to our upping of
> the WINVER compile directive for winrt builds. I'm not sure this should
> block anything related to webrtc landings on mc.
Ah okay. I'll move into build config then and remove the whiteboard tags.
Updated•13 years ago
|
Component: WebRTC → Build Config
QA Contact: jsmith
Whiteboard: [WebRTC][blocking-webrtc+][metro-mvp][LOE:1] → [metro-mvp][LOE:1]
| Assignee | ||
Comment 10•13 years ago
|
||
Attachment #677758 -
Attachment is obsolete: true
| Assignee | ||
Comment 11•13 years ago
|
||
Ok, so what this does:
1) downgrade back to WINVER=0x502 for the build
2) replace <wrl.h> with a custom header that deals with various issues
3) up WINVER in key places
with this, you can build with enable-webrtc.
Attachment #677886 -
Attachment is obsolete: true
Attachment #677887 -
Flags: review?(netzen)
| Assignee | ||
Updated•13 years ago
|
Attachment #677887 -
Attachment is patch: true
Comment 12•13 years ago
|
||
Comment on attachment 677887 [details] [diff] [review]
(elm) build fix
Review of attachment 677887 [details] [diff] [review]:
-----------------------------------------------------------------
I'm OK with this but I'm not sure if this is the best approach in general. We'd only be able to use new APIs from within widget.
Attachment #677887 -
Flags: review?(netzen)
Attachment #677887 -
Flags: review+
Attachment #677887 -
Flags: feedback?(ehsan)
| Assignee | ||
Comment 13•13 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #12)
> Comment on attachment 677887 [details] [diff] [review]
> build fix
>
> Review of attachment 677887 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> I'm OK with this but I'm not sure if this is the best approach in general.
> We'd only be able to use new APIs from within widget.
Well we could in other areas, we would just have to up WINVER locally, or bite the bullet and up it for the whole build and deal with issues we might run into.
My biggest concern there is things like newer structs that the apis on WinXP might not understand that could seep into our build. Our testing protects us from hitting those but you never know what might sneak past our tests.
The other solution would be to bring all the wrl header code into our repo and make it work with winver == 0x502. I don't think that's an option due to copyright issues.
There might be other solutions.. maybe ehsan has some ideas.
Comment 14•13 years ago
|
||
Comment on attachment 677887 [details] [diff] [review]
(elm) build fix
Review of attachment 677887 [details] [diff] [review]:
-----------------------------------------------------------------
First things first, I don't like this patch one bit. ;-) In particular, upping WINVER in parts of the tree really worries me. But I can't think of any better solutions. I don't know how urgent this patch is, but if it's not I would feel much better if it landed in the beginning of the 20 cycle as opposed to know.
Plusing the request because I don't have a r=whatever flag to use. :-)
::: toolkit/xre/nsAppRunner.cpp
@@ +4004,5 @@
>
> gArgc = argc;
> gArgv = argv;
>
> NS_ENSURE_TRUE(aAppData, 2);
This change is bad. See the return statement hidden. Oh, wait, you can't see it. But it's there. ;-)
We need our own RAII type if we can't use RoInitializeWrapper.
Attachment #677887 -
Flags: feedback?(ehsan) → feedback+
| Assignee | ||
Comment 15•13 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #14)
> First things first, I don't like this patch one bit. ;-) In particular,
> upping WINVER in parts of the tree really worries me. But I can't think of
> any better solutions. I don't know how urgent this patch is, but if it's
> not I would feel much better if it landed in the beginning of the 20 cycle
> as opposed to know.
This code is encased in MOZ_METRO so it will have plenty of bake time on elm and ultimately mc.
> ::: toolkit/xre/nsAppRunner.cpp
> @@ +4004,5 @@
> >
> > gArgc = argc;
> > gArgv = argv;
> >
> > NS_ENSURE_TRUE(aAppData, 2);
>
> This change is bad. See the return statement hidden. Oh, wait, you can't
> see it. But it's there. ;-)
>
> We need our own RAII type if we can't use RoInitializeWrapper.
I'll file a bug. It's really not a big deal, when XRE_mainMetro returns the app shuts down. But yeah I agree our own wrapper is the right way to do it.
Comment 16•13 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #13)
> Well we could in other areas, we would just have to up WINVER locally, or
> bite the bullet and up it for the whole build and deal with issues we might
> run into.
Well yes I know, but it's still an annoying overhead just to for example use some new constant which often happens with these WINVER guards.
> My biggest concern there is things like newer structs that the apis on WinXP
> might not understand that could seep into our build. Our testing protects us
> from hitting those but you never know what might sneak past our tests.
The Win32 API does this in a C-style so pretty much always includes the struct size as the first member of structs, to avoid situations like that for changed structs.
> The other solution would be to bring all the wrl header code into our repo
> and make it work with winver == 0x502. I don't think that's an option due to
> copyright issues.
Ya, that doesn't sound good.
> There might be other solutions.. maybe ehsan has some ideas.
One solution would be to just live with the warnings for now :)
I think my preference would be to suppress the compiler warnings in the cpp files that are causing the problem with:
http://msdn.microsoft.com/en-us/library/2c8f766e%28v=vs.80%29.aspx
| Assignee | ||
Comment 17•13 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #16)
> > There might be other solutions.. maybe ehsan has some ideas.
>
> One solution would be to just live with the warnings for now :)
>
> I think my preference would be to suppress the compiler warnings in the cpp
> files that are causing the problem with:
> http://msdn.microsoft.com/en-us/library/2c8f766e%28v=vs.80%29.aspx
Ah, so I probably haven't communicated cleanly the reason for this patch. This fix isn't fixing up warnings, it's fixing build errors we run into when downgrading WINVER globally from 0x602 to 0x502. A few issues crop up with WINVER set to 0x602 -
1) the wrl / winrt headers abort the build by checking for a vista+ target.
2) webrtc doesn't build successfully with WINVER set to 602
3) corewrappers.h calls apis that aren't defined when WINVER < 0x600
The only solution I know of to #1 / #3 would be to roll our own headers / wrl templates.
The webrtc issue is a problem with that library. We could solve that if we had to.
Comment 18•13 years ago
|
||
ah ok, ya #2 would be easy to solve.
| Assignee | ||
Comment 19•13 years ago
|
||
- added the winrt init wrapper to nsAppRunner.
| Assignee | ||
Comment 20•13 years ago
|
||
Whiteboard: [metro-mvp][LOE:1] → [metro-mvp][LOE:1][completed-elm]
| Assignee | ||
Comment 21•13 years ago
|
||
same configure.in and nsAppRunner changes for mc, plus some missing app runner metro bits.
Attachment #678046 -
Flags: review?(netzen)
| Assignee | ||
Comment 22•13 years ago
|
||
Pushed this one to try to:
https://tbpl.mozilla.org/?noignore=0&tree=Try&rev=5c2c6d170492
Comment 23•13 years ago
|
||
Comment on attachment 678046 [details] [diff] [review]
mc bits
Review of attachment 678046 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with changes
::: toolkit/xre/nsAppRunner.cpp
@@ +102,5 @@
> #ifdef XP_WIN
> #include "nsIWinAppHelper.h"
> #include <windows.h>
> #include "cairo/cairo-features.h"
> +#ifdef MOZ_METRO
&& defined(XP_WIN)
@@ +4072,5 @@
> + return 0;
> +}
> +
> +void SetWindowsEnvironment(WindowsEnvironmentType aEnvID);
> +#endif // MOZ_METRO
nit: #endif // MOZ_METRO && XP_WIN
@@ +4098,5 @@
> +
> + // Metro
> + NS_ASSERTION(XRE_GetWindowsEnvironment() == WindowsEnvironmentType_Metro,
> + "Unknown Windows environment");
> + return XRE_mainMetro(argc, argv, aAppData);
Before this return:
- return XRE_mainMetro(argc, argv, aAppData);
+ int result = XRE_mainMetro(argc, argv, aAppData);
+ mozilla::RecordShutdownEndTimeStamp();
+ return result;
@@ +4099,5 @@
> + // Metro
> + NS_ASSERTION(XRE_GetWindowsEnvironment() == WindowsEnvironmentType_Metro,
> + "Unknown Windows environment");
> + return XRE_mainMetro(argc, argv, aAppData);
> +#endif // MOZ_METRO
#endif //!MOZ_METRO || !XP_WIN
Attachment #678046 -
Flags: review?(netzen) → review+
Comment 24•13 years ago
|
||
This second patch should maybe be in its own bug btw.
| Assignee | ||
Comment 25•13 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #23)
> ::: toolkit/xre/nsAppRunner.cpp
> @@ +102,5 @@
> > #ifdef XP_WIN
> > #include "nsIWinAppHelper.h"
> > #include <windows.h>
> > #include "cairo/cairo-features.h"
> > +#ifdef MOZ_METRO
>
> && defined(XP_WIN)
This was already in an xpwin block, so no need for this change.
I've updated other nits.
| Assignee | ||
Comment 26•13 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #24)
> This second patch should maybe be in its own bug btw.
I can move the added metro blocks over to bug 807593.
| Assignee | ||
Comment 27•13 years ago
|
||
Attachment #678046 -
Attachment is obsolete: true
| Assignee | ||
Updated•13 years ago
|
Attachment #678015 -
Attachment description: build fix v.2 → (elm) build fix v.2
| Assignee | ||
Updated•13 years ago
|
Attachment #677887 -
Attachment description: build fix → (elm) build fix
| Assignee | ||
Comment 28•13 years ago
|
||
Comment 29•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Updated•8 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•