WebRTC build error on windows

RESOLVED FIXED in mozilla19

Status

defect
RESOLVED FIXED
7 years ago
Last year

People

(Reporter: bas.schouten, Assigned: glandium)

Tracking

unspecified
mozilla19
x86_64
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [WebRTC] [qa-])

Attachments

(2 attachments, 3 obsolete attachments)

Reporter

Description

7 years ago
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
Reporter

Comment 1

7 years ago
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
Reporter

Comment 2

7 years ago
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.

Comment 4

7 years ago
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?
Assignee

Comment 5

7 years ago
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
Assignee

Comment 6

7 years ago
Posted 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.
Assignee

Comment 7

7 years ago
(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.
Assignee

Comment 10

7 years ago
(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.
Assignee

Updated

7 years ago
Attachment #665059 - Attachment is obsolete: true
Assignee

Comment 13

7 years ago
Attachment #665491 - Flags: review?(ted.mielczarek)
Assignee

Comment 14

7 years ago
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)
Assignee

Comment 15

7 years ago
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)
Assignee

Updated

7 years ago
Attachment #665491 - Attachment is obsolete: true
BTW, Bas's problem turned out to be stale pycs.
Assignee

Comment 18

7 years ago
Somehow, the patches here break mochitest-4... :-/
Assignee

Comment 19

7 years ago
(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.
Assignee

Comment 20

7 years ago
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)
Assignee

Updated

7 years ago
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+
Assignee

Comment 24

7 years ago
(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: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Whiteboard: [WebRTC] → [WebRTC] [qa-]
Assignee

Updated

7 years ago
Depends on: 814796

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.