794472 - WebRTC build error on windows

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Bas Schouten (:bas.schouten)]

At least on Windows 8 64-bit. I'm getting a build error when trying to build with WebRTC:

make.py[3]: Entering directory 'c:\Users\Bas\Dev\mi-debug\media/webrtc'
make.py[4]: Entering directory 'c:\Users\Bas\Dev\mi-debug\media\webrtc\trunk'
make.py[5]: Entering directory 'c:\Users\Bas\Dev\mi-debug\media\webrtc\trunk\src
/modules/modules_webrtc_video_coding'
codec_database.cc
c:\Users\Bas\Dev\mozilla-inbound\config\rules.mk:976:0:native command 'cl Invoke
ClWithDependencyGeneration cl -Fovideo_coding/main/source/codec_database.obj -c
  -D_WIN32_WINNT=0x0601 -DWINVER=0x0601 -DWIN32 -D_WINDOWS -DNOMINMAX -DPSAPI_VE
RSION=1 -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DWIN32_LEAN_AND_MEAN -
D_ATL_NO_OPENGL -D_HAS_TR1=0 -D_HAS_EXCEPTIONS=0 -D_SECURE_ATL -DCHROMIUM_BUILD
-DTOOLKIT_VIEWS=1 -DENABLE_REMOTING=1 -DENABLE_P2P_APIS=1 -DENABLE_CONFIGURATION
_POLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_GPU=1 -DENABLE_EGL
IMAGE=1 -DUSE_SKIA=1 -D__STD_C -D_CRT_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_DEPRE
CATE -DENABLE_REGISTER_PROTOCOL_HANDLER=1 -DENABLE_WEB_INTENTS=1 -DENABLE_PLUGIN
_INSTALLATION=1 -DWEBRTC_WIN -DWEBRTC_EXPORT -D__STDC_FORMAT_MACROS -DDYNAMIC_AN
NOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DUNICODE -D_UNICODE -I. -I.
./../../../../../../mozilla-inbound/media/webrtc/trunk/src/modules/.. -I../../..
/../../../../mozilla-inbound/media/webrtc/trunk/src/modules/../.. -I../../../../
../../../mozilla-inbound/media/webrtc/trunk/src/modules/../../third_party/wtl/in
clude -I../../../../../../../mozilla-inbound/media/webrtc/trunk/src/modules/vide
o_coding/main/interface -I../../../../../../../mozilla-inbound/media/webrtc/trun
k/src/modules/interface -I../../../../../../../mozilla-inbound/media/webrtc/trun
k/src/modules/video_coding/codecs/interface -I../../../../../../../mozilla-inbou
nd/media/webrtc/trunk/src/modules/../common_video/interface -I../../../../../../
../mozilla-inbound/media/webrtc/trunk/src/modules/video_coding/codecs/i420/main/
interface -I../../../../../../../mozilla-inbound/media/webrtc/trunk/src/modules/
video_coding/codecs/vp8/main/interface -I../../../../../../../mozilla-inbound/me
dia/webrtc/trunk/src/modules/../../src/common_video/interface -I../../../../../.
./../mozilla-inbound/media/webrtc/trunk/src/modules/../../src/modules/video_codi
ng/codecs/interface -I../../../../../../../mozilla-inbound/media/webrtc/trunk/sr
c/modules/../system_wrappers/interface -I"C:\Program Files (x86)\Microsoft Direc
tX SDK (June 2010)/include"    -TP -nologo -W3 -Gy -Fdgenerated.pdb -wd4800 -we4
553 -GR- -DDEBUG -D_DEBUG -DTRACING -Zi -Oy-   -MDd           -FI ../../../../..
/../dist/include/mozilla-config.h -DMOZILLA_CLIENT  c:/Users/Bas/Dev/mi-debug/me
dia/webrtc/trunk/src/modules/modules_webrtc_video_coding/../../../../../../../mo
zilla-inbound/media/webrtc/trunk/src/modules/video_coding/main/source/codec_data
base.cc': shell metacharacter '(' in command line
c:\Users\Bas\Dev\mozilla-inbound\config\makefiles\target_libs.mk:27:0: command '
c:/Users/Bas/Dev/mozilla-build/python/python.exe c:/Users/Bas/Dev/graphics/build
/pymake/pymake/../make.py -C src/modules/modules_webrtc_video_coding libs' faile
d, return code 2
c:\Users\Bas\Dev\mozilla-inbound\config\makefiles\target_libs.mk:60:0: command '
c:/Users/Bas/Dev/mozilla-build/python/python.exe c:/Users/Bas/Dev/graphics/build
/pymake/pymake/../make.py -C trunk libs' failed, return code 2
c:\Users\Bas\Dev\mozilla-inbound\config\makefiles\target_libs.mk:18:0: command '
c:/Users/Bas/Dev/mozilla-build/python/python.exe c:/Users/Bas/Dev/graphics/build
/pymake/pymake/../make.py -C media/webrtc libs' failed, return code 2
c:\Users\Bas\Dev\mozilla-inbound\config\rules.mk:587:0: command 'c:/Users/Bas/De
v/mozilla-build/python/python.exe c:/Users/Bas/Dev/graphics/build/pymake/pymake/
../make.py libs_tier_platform' failed, return code 2
c:\Users\Bas\Dev\mozilla-inbound\config\rules.mk:552:0: command 'c:/Users/Bas/De
v/mozilla-build/python/python.exe c:/Users/Bas/Dev/graphics/build/pymake/pymake/
../make.py  tier_platform' failed, return code 2

Comment 1

[Bas Schouten (:bas.schouten)]

It appears if I disable WebRTC this just happens further down inside LibGLES. Might have something to do with this being the VS2012 environment, it's always worked fine so far though.

Comment 2

[Bas Schouten (:bas.schouten)]

This is happening on my Windows 7 machine with VS2010 as well.

Comment 3

[(not currently active) Ted Mielczarek]

This is Pymake barfing on -I"C:\Program Files (x86)\Microsoft DirectX SDK (June 2010)/include"

I know we've seen this in the past, but I can't remember what we did about it.

Comment 4

[Benjamin Smedberg]

So really the only bug here is that pymake sees the parens and freaks out, even though there is nothing to freak out about. Perhaps we should parse slightly more of the string and ignore lots of the metachars within quoted strings?

Comment 5

[Mike Hommey [:glandium]]

Okay, since bug 783936 is closed, let's use this one for my WIP patch to make pymake more lenient with shell special character.

Comment 6

[Mike Hommey [:glandium]]

Attachment: WIP

This is my current WIP. It's been sitting in my mq for a while, i don't remember the exact status (whether it works fine for everything or not). I do remember I had pymake testsuite failures, i haven't rerun them to check if that was still the case, but i also seem to remember it was building m-c fine.

Comment 7

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #6)
> Created attachment 665059 [details] [diff] [review]
> WIP
> 
> This is my current WIP. It's been sitting in my mq for a while, i don't
> remember the exact status (whether it works fine for everything or not). I
> do remember I had pymake testsuite failures, i haven't rerun them to check
> if that was still the case, but i also seem to remember it was building m-c
> fine.

It looks like it passes the test suite *and* is green on win32. I'll further check if it's landable tomorrow.

Comment 8

[Rain [:rain1] (inactive)]

Strange -- is your pymake up-to-date (in particular, with the patch for bug 780612)? This shouldn't be happening.

Comment 9

[Rain [:rain1] (inactive)]

So I just did a full VS2012 build on Windows 8 with default settings and didn't encounter this. My DirectX SDK is installed in the same location as Bas's, in a path with parentheses.

As I said, the patch for bug 780612 was written to handle precisely this error. It is impossible that '(' is detected as a shell metacharacter, because it is specifically excluded via setting blacklist_gray to false at <http://hg.mozilla.org/mozilla-central/file/tip/build/pymake/pymake/data.py#l1408>. I suspect that the Pymake Bas is using might be modified in some way -- perhaps one of the patches in Bas's patch queue accidentally reverts that change or something.

Comment 10

[Mike Hommey [:glandium]]

(In reply to Siddharth Agarwal [:sid0] from comment #9)
> As I said, the patch for bug 780612 was written to handle precisely this
> error. It is impossible that '(' is detected as a shell metacharacter,
> because it is specifically excluded via setting blacklist_gray to false at
> <http://hg.mozilla.org/mozilla-central/file/tip/build/pymake/pymake/data.
> py#l1408>.

There are still many places where pymake is going through msys when it really shouldn't, and that blocks bug 486141, so we might as well use this bug to fix the underlying issue.

Comment 11

[Rain [:rain1] (inactive)]

Sure. But if something that's supposed to be impossible is happening now, there's no reason to believe it won't happen with your patch. We still need to figure out the root issue.

Comment 12

[Mike Hommey [:glandium]]

Attachment: Add quoting for some commands to make pymake happier

Comment 13

[Mike Hommey [:glandium]]

Attachment: Allow pymake to run more commands without sending them to a shell

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 665491 [details] [diff] [review]
Allow pymake to run more commands without sending them to a shell

Looks like there's a path problem when running jsapi-tests.exe

Comment 15

[Mike Hommey [:glandium]]

Attachment: Allow pymake to run more commands without sending them to a shell

I think I have it right this time.

Note that on top of making less things bail out to a shell, it also allows $(shell) commands to be called without a shell, but it doesn't try to be smart about $(MAKE), it still runs a subprocess, although it calls python directly.

Comment 16

[Rain [:rain1] (inactive)]

BTW, Bas's problem turned out to be stale pycs.

Comment 17

[Mike Hommey [:glandium]]

https://tbpl.mozilla.org/?tree=Try&rev=f6a663d3e5e7

Comment 18

[Mike Hommey [:glandium]]

Somehow, the patches here break mochitest-4... :-/

Comment 19

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #18)
> Somehow, the patches here break mochitest-4... :-/

Apparently, it's the part that makes $(shell) not use a shell in most cases.

Comment 20

[Mike Hommey [:glandium]]

Attachment: Allow pymake to run more commands without sending them to a shell

Only change since previous iteration is to shellwords. The failure came from the use of find in a $(shell) command, and running find directly from pymake, without a shell, fails blatantly (and it also fails outside $(shell), it just happens we don't use find in recipes, so we never noticed)

Comment 21

[(not currently active) Ted Mielczarek]

Comment on attachment 665985 [details] [diff] [review]
Allow pymake to run more commands without sending them to a shell

Review of attachment 665985 [details] [diff] [review]:
-----------------------------------------------------------------

After staring at this patch a few times I'm generally okay with it. This could really use some tests though.

::: build/pymake/pymake/functions.py
@@ +773,5 @@
> +        # subprocess.Popen doesn't use the PATH set in the env argument for
> +        # finding the executable on some platforms (but strangely it does on
> +        # others!), so set os.environ['PATH'] explicitly.
> +        oldpath = os.environ['PATH']
> +        if makefile.env is not None and makefile.env.has_key('PATH'):

Usually written as 'PATH' in makefile.env

::: build/pymake/pymake/process.py
@@ +18,5 @@
>  
> +def tokens2re(tokens):
> +    # Create a pattern for non-escaped tokens, in the form:
> +    #   (?<!\\)(?:a|b|c...)
> +    # This is meant to matchs patterns a, b, or c, or ... if they are not

"match"

@@ +54,5 @@
> +class ClineSplitter(list):
> +    '''
> +    Parses a given command line string and creates a list of command
> +    and arguments, with wildcard expansion.
> +    '''

We typically use double quote characters for triple quoting.

Comment 22

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6511fd29e7d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9bd08b3cfb1

Comment 23

[Mike Hommey [:glandium]]

http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/3aa21f3c918d

Comment 24

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #21)
> This could really use some tests though.

I'm really not sure how this can be tested in the current pymake test infrastructure.

Comment 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6511fd29e7d8
https://hg.mozilla.org/mozilla-central/rev/d9bd08b3cfb1