Last Comment Bug 675084 - PGO builds with pymake are broken because pymake chokes on a builtin with shell metacharacters
: PGO builds with pymake are broken because pymake chokes on a builtin with she...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: x86 Windows 7
: -- normal with 1 vote (vote)
: ---
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
Mentors:
Depends on:
Blocks: 665978
  Show dependency treegraph
 
Reported: 2011-07-28 16:23 PDT by Ryan VanderMeulen [:RyanVM]
Modified: 2011-08-31 17:21 PDT (History)
7 users (show)
khuey: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Implement shell globbing for pymake (4.15 KB, patch)
2011-08-22 12:12 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
ted: review+
Details | Diff | Splinter Review

Description Ryan VanderMeulen [:RyanVM] 2011-07-28 16:23:26 PDT
libs_tier_nspr
c:\mozbuild\mozilla-central\nsprpub\config\rules.mk:384:0:native command 'pymake.builtins rm -f *.pgd *.gcda': shell metacharacter '*' in command line
c:\mozbuild\mozilla-central\nsprpub\config\rules.mk:189:0: command 'c:/mozbuild/python/python.exe c:/mozbuild/mozilla-central/build/pymake/pymake/../make.py -C config export' failed, return code 2
<all>: Found error
<all>: Found error
c:\mozbuild\mozilla-central\objdir-fx\config\nspr\Makefile:64:0: command 'c:/mozbuild/python/python.exe c:/mozbuild/mozilla-central/build/pymake/pymake/../make.py -C ../../nsprpub PROFILE_USE_CFLAGS="-GL -wd4624 -wd4952"' failed, return code 2
c:\mozbuild\mozilla-central\config\rules.mk:710:0: command 'c:/mozbuild/python/python.exe c:/mozbuild/mozilla-central/build/pymake/pymake/../make.py -C config/nspr libs' failed, return code 2
c:\mozbuild\mozilla-central\config\rules.mk:721:0: command 'c:/mozbuild/python/python.exe c:/mozbuild/mozilla-central/build/pymake/pymake/../make.py libs_tier_nspr' failed, return code 2
c:\mozbuild\mozilla-central\config\rules.mk:671:0: command 'c:/mozbuild/python/python.exe c:/mozbuild/mozilla-central/build/pymake/pymake/../make.py  tier_nspr' failed, return code 2
c:\mozbuild\mozilla-central\client.mk:349:0: command 'c:/mozbuild/python/python.exe c:/mozbuild/mozilla-central/build/pymake/pymake/../make.py -s -j 4 -C c:/mozbuild/mozilla-central/objdir-fx' failed, return code 2
c:\mozbuild\mozilla-central\client.mk:212:0: command 'c:/mozbuild/python/python.exe c:/mozbuild/mozilla-central/build/pymake/pymake/../make.py -f c:/mozbuild/mozilla-central/client.mk realbuild MOZ_PROFILE_GENERATE=1' failed, return code 2
c:\mozbuild\mozilla-central\client.mk:174:0: command 'c:/mozbuild/python/python.exe c:/mozbuild/mozilla-central/build/pymake/pymake/../make.py -f c:/mozbuild/mozilla-central/client.mk profiledbuild' failed, return code 2
Comment 1 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-28 16:25:25 PDT
This is a pymake bug.  We should be falling back to the shell rm here.
Comment 2 Ted Mielczarek [:ted.mielczarek] 2011-07-29 04:20:08 PDT
Hrmph. We should just support shell globbing in pymake native commands.
Comment 3 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-22 10:12:22 PDT
Committed a test http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/eb164dd01eb2
Comment 4 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-22 12:12:47 PDT
Created attachment 554925 [details] [diff] [review]
Implement shell globbing for pymake
Comment 5 Ted Mielczarek [:ted.mielczarek] 2011-08-25 13:22:33 PDT
Comment on attachment 554925 [details] [diff] [review]
Implement shell globbing for pymake

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

::: pymake/process.py
@@ +41,5 @@
> +    """
> +    globbedargs = []
> +    for arg in args:
> +        if _needsglob.search(arg):
> +            globbedargs.extend(glob.glob(os.path.join(cwd, arg)))

It seems unfortunate to run this regex for every single argument. Can you either a) run it once over all args or b) find out if glob is smart enough to not iterate over the entire directory unless there are wildcard characters?

::: tests/native-command-shell-glob.mk
@@ +8,4 @@
>  	$(RM) shell-glob-test/*.txt
> +	$(RM) shell-glob-test/?.foo
> +	rmdir shell-glob-test
> +	@echo TEST-PASS

I was confused as to why this was patching an existing test. You landed the test in the pymake repo first!
Comment 6 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-26 03:38:41 PDT
http://hg.python.org/cpython/file/498b03a55297/Lib/glob.py#l24
Comment 7 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-26 03:41:31 PDT
So the problem with globbing everything is that things that don't exist will vanish from the command line.  Do you really think the regex matching will be slow enough for this to matter?
Comment 8 Ted Mielczarek [:ted.mielczarek] 2011-08-26 04:23:34 PDT
Ah, that's a good point, you're right. Too bad that both your code and glob will regex check each argument. :-/
Comment 9 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-26 04:32:14 PDT
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/04fee3d7bdec
Comment 10 Gregory Szorc [:gps] 2011-08-31 17:21:59 PDT
The pymake change in this bug broke builds on OS X and potentially other platforms. See bug 683686.

Note You need to log in before you can comment on or make changes to this bug.