Closed Bug 794472 Opened 8 years ago Closed 8 years ago

WebRTC build error on windows

Categories

(Firefox Build System :: General, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla19

People

(Reporter: bas.schouten, Assigned: glandium)

References

Details

(Whiteboard: [WebRTC] [qa-])

Attachments

(2 files, 3 obsolete files)

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
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.
Component: WebRTC → Build Config
QA Contact: jsmith
This is happening on my Windows 7 machine with VS2010 as well.
OS: Windows 8 → Windows 7
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.
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?
Okay, since bug 783936 is closed, let's use this one for my WIP patch to make pymake more lenient with shell special character.
Assignee: nobody → mh+mozilla
Blocks: 486141
Attached patch WIP (obsolete) — Splinter Review
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.
(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.
Strange -- is your pymake up-to-date (in particular, with the patch for bug 780612)? This shouldn't be happening.
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.
(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.
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.
Attachment #665490 - Flags: review?(ted.mielczarek)
Attachment #665059 - Attachment is obsolete: true
Attachment #665491 - Flags: review?(ted.mielczarek)
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
Attachment #665491 - Flags: review?(ted.mielczarek)
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.
Attachment #665506 - Flags: review?(ted.mielczarek)
Attachment #665491 - Attachment is obsolete: true
BTW, Bas's problem turned out to be stale pycs.
Somehow, the patches here break mochitest-4... :-/
(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.
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)
Attachment #665985 - Flags: review?(ted.mielczarek)
Attachment #665506 - Attachment is obsolete: true
Attachment #665506 - Flags: review?(ted.mielczarek)
Attachment #665490 - Flags: review?(ted.mielczarek) → review+
Whiteboard: [WebRTC]
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.
Attachment #665985 - Flags: review?(ted.mielczarek) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/6511fd29e7d8
https://hg.mozilla.org/mozilla-central/rev/d9bd08b3cfb1
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Whiteboard: [WebRTC] → [WebRTC] [qa-]
Depends on: 814796
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.