Closed Bug 799121 Opened 7 years ago Closed 7 years ago

Build problems with nrappkit and 8.0 sdk

Categories

(Firefox Build System :: General, defect)

x86_64
Windows 7
defect
Not set

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)

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
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
(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!
Whiteboard: [blocking-webrtc+]
Whiteboard: [blocking-webrtc+] → [WebRTC] [blocking-webrtc+]
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'
Whiteboard: [WebRTC] [blocking-webrtc+] → [WebRTC] [blocking-webrtc+][metro-mvp]
Summary: Build problems with nrappkit and vs 2012 → Build problems with nrappkit and 8.0 sdk
Whiteboard: [WebRTC] [blocking-webrtc+][metro-mvp] → [WebRTC] [blocking-webrtc+][metro-mvp][LOE:1]
Blocks: 805553
Assignee: nobody → jmathies
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.
Blocks: 806799
No longer blocks: 805553
Attached patch build fix (obsolete) — Splinter Review
Attached patch build fix (obsolete) — Splinter Review
added a comment in the make file on this.
Attachment #677756 - Attachment is obsolete: true
Whiteboard: [WebRTC] [blocking-webrtc+][metro-mvp][LOE:1] → [metro-mvp][LOE:1]
Whiteboard: [metro-mvp][LOE:1] → [WebRTC][blocking-webrtc+][metro-mvp][LOE:1]
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.
(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.
Component: WebRTC → Build Config
QA Contact: jsmith
Whiteboard: [WebRTC][blocking-webrtc+][metro-mvp][LOE:1] → [metro-mvp][LOE:1]
Attached patch build fix (obsolete) — Splinter Review
Attachment #677758 - Attachment is obsolete: true
Attached patch (elm) build fixSplinter Review
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)
Attachment #677887 - Attachment is patch: true
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)
(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 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+
(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.
(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
(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.
ah ok, ya #2 would be easy to solve.
- added the winrt init wrapper to nsAppRunner.
https://hg.mozilla.org/projects/elm/rev/b90309aca4a4
Whiteboard: [metro-mvp][LOE:1] → [metro-mvp][LOE:1][completed-elm]
Attached patch mc bits (obsolete) — Splinter Review
same configure.in and nsAppRunner changes for mc, plus some missing app runner metro bits.
Attachment #678046 - Flags: review?(netzen)
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+
This second patch should maybe be in its own bug btw.
(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.
(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.
Attached patch mc winver bitsSplinter Review
Attachment #678046 - Attachment is obsolete: true
Attachment #678015 - Attachment description: build fix v.2 → (elm) build fix v.2
Attachment #677887 - Attachment description: build fix → (elm) build fix
https://hg.mozilla.org/mozilla-central/rev/cc7db2e7e54c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.