Skip the shell when there aren't any special metacharacters to worry about.
authorBenjamin Smedberg <benjamin@smedbergs.us>
Wed, 18 Feb 2009 17:27:01 -0500
changeset 133 7df5ab3b364fac50aea2c025d3a973801ca7670e
parent 132 f20d24d3c1069c8f700cc43fed20b6e6eeee04b8
child 134 ca40dbbeb55a00109b5c3d76c64c2d23b8364550
push id75
push userbsmedberg@mozilla.com
push dateWed, 18 Feb 2009 23:21:03 +0000
Skip the shell when there aren't any special metacharacters to worry about.
pymake/command.py
pymake/data.py
pymake/functions.py
pymake/parser.py
pymake/process.py
--- a/pymake/command.py
+++ b/pymake/command.py
@@ -1,16 +1,15 @@
 """
 Logic to execute a command
 """
 
 import os, subprocess, sys, logging, time
 from optparse import OptionParser
-from pymake.data import Makefile, DataError
-from pymake.parser import parsestream, parsecommandlineargs, SyntaxError
+import pymake.data, pymake.parser
 
 def parsemakeflags(env):
     makeflags = env.get('MAKEFLAGS', '')
     makeflags = makeflags.strip()
 
     if makeflags == '':
         return []
 
@@ -29,17 +28,17 @@ def parsemakeflags(env):
             i += 1
             while i < len(makeflags) and makeflags[i].isspace():
                 i += 1
             continue
 
         if c == '\\':
             i += 1
             if i == len(makeflags):
-                raise DataError("MAKEFLAGS has trailing backslash")
+                raise pymake.data.DataError("MAKEFLAGS has trailing backslash")
             c = makeflags[i]
             
         curopt += c
         i += 1
 
     if curopt != '':
         opts.append(curopt)
 
@@ -127,21 +126,21 @@ def main(args, env, cwd):
             print "No makefile found"
             return 2
 
     try:
         def parse():
             i = 0
 
             while True:
-                m = Makefile(restarts=i, make='%s %s' % (sys.executable, sys.argv[0]),
-                             makeflags=makeflags, makelevel=makelevel, workdir=workdir)
+                m = pymake.data.Makefile(restarts=i, make='%s %s' % (sys.executable, sys.argv[0]),
+                                         makeflags=makeflags, makelevel=makelevel, workdir=workdir)
 
                 starttime = time.time()
-                targets = parsecommandlineargs(m, arguments)
+                targets = pymake.parser.parsecommandlineargs(m, arguments)
                 for f in options.makefiles:
                     m.include(f)
 
                 log.info("Parsing[%i] took %f seconds" % (i, time.time() - starttime,))
 
                 m.finishparsing()
                 if m.remakemakefiles():
                     log.info("restarting makefile parsing")
@@ -165,17 +164,17 @@ def main(args, env, cwd):
         else:
             tstack = ['<command-line>']
 
         starttime = time.time()
         for t in targets:
             m.gettarget(t).make(m, ['<command-line>'], [])
         log.info("Execution took %f seconds" % (time.time() - starttime,))
 
-    except (DataError, SyntaxError, subprocess.CalledProcessError), e:
+    except (pymake.data.DataError, pymake.parser.SyntaxError), e:
         print e
         if options.printdir:
             print "make.py[%i]: Leaving directory '%s'" % (makelevel, workdir)
         sys.stdout.flush()
         return 2
 
     if options.printdir:
         print "make.py[%i]: Leaving directory '%s'" % (makelevel, workdir)
--- a/pymake/data.py
+++ b/pymake/data.py
@@ -1,14 +1,16 @@
 """
 A representation of makefile data structures.
 """
 
-import logging, re, os, subprocess
-import pymake
+import logging, re, os
+import pymake.parser
+import pymake.functions
+import pymake.process
 
 log = logging.getLogger('pymake.data')
 
 class DataError(Exception):
     def __init__(self, message, loc=None):
         self.message = message
         self.loc = loc
 
@@ -821,17 +823,17 @@ def executecommands(rule, target, makefi
     for c in rule.commands:
         cstring = c.resolve(makefile, v)
         for cline in splitcommand(cstring):
             cline, isHidden, isRecursive, ignoreErrors = findmodifiers(cline)
             if not len(cline) or cline.isspace():
                 continue
             if not isHidden:
                 print "%s $ %s" % (c.loc, cline)
-            r = subprocess.call(cline, shell=True, env=env, cwd=makefile.workdir)
+            r = pymake.process.call(cline, env=env, cwd=makefile.workdir, loc=c.loc)
             if r != 0 and not ignoreErrors:
                 raise DataError("command '%s' failed, return code was %s" % (cline, r), c.loc)
 
 class Rule(object):
     """
     A rule contains a list of prerequisites and a list of commands. It may also
     contain rule-specific variables. This rule may be associated with multiple targets.
     """
--- a/pymake/functions.py
+++ b/pymake/functions.py
@@ -1,19 +1,19 @@
 """
 Makefile functions.
 """
 
-import pymake
-from pymake import data
-import subprocess, os
+import parser
+import data
+import subprocess, os, logging
 from pymake.globrelative import glob
 from cStringIO import StringIO
 
-log = data.log
+log = logging.getLogger('pymake.data')
 
 class Function(object):
     """
     An object that represents a function call. This class is always subclassed
     with the following methods and attributes:
 
     minargs = minimum # of arguments
     maxargs = maximum # of arguments (0 means unlimited)
@@ -507,17 +507,17 @@ class EvalFunction(Function):
 
     def resolve(self, makefile, variables, setting):
         if makefile.parsingfinished:
             # GNU make allows variables to be set by recursive expansion during
             # command execution. This seems really dumb to me, so I don't!
             raise data.DataError("$(eval) not allowed via recursive expansion after parsing is finished", self.loc)
 
         text = StringIO(self._arguments[0].resolve(makefile, variables, setting))
-        pymake.parser.parsestream(text, 'evaluation from %s' % self.loc, makefile)
+        parser.parsestream(text, 'evaluation from %s' % self.loc, makefile)
         return ''
 
 class OriginFunction(Function):
     name = 'origin'
     minargs = 1
     maxargs = 1
 
     def resolve(self, makefile, variables, setting):
--- a/pymake/parser.py
+++ b/pymake/parser.py
@@ -14,18 +14,18 @@ Lines with command syntax do not condens
 Lines with an initial tab are commands if they can be (there is a rule or a command immediately preceding).
 Otherwise, they are parsed as makefile syntax.
 
 After splitting data into parseable chunks, we use a recursive-descent parser to
 nest parenthesized syntax.
 """
 
 import logging, re
+import data, functions
 from pymake.globrelative import hasglob, glob
-from pymake import data, functions
 from cStringIO import StringIO
 
 tabwidth = 4
 
 log = logging.getLogger('pymake.parser')
 
 class SyntaxError(Exception):
     def __init__(self, message, loc):
@@ -393,24 +393,27 @@ def iterlines(fd):
         lineno += 1
 
         if line.endswith('\r\n'):
             line = line[:-2] + '\n'
 
         yield (lineno, line)
 
 def setvariable(resolvevariables, setvariables, makefile, vname, token, d, offset,
-                iterfunc=itermakefilechars, source=data.Variables.SOURCE_MAKEFILE,
+                iterfunc=itermakefilechars, source=None,
                 skipwhitespace=True):
     """
     Parse what's left in a data iterator di into a variable.
     """
     assert isinstance(resolvevariables, data.Variables)
     assert isinstance(setvariables, data.Variables)
 
+    if source is None:
+        source = data.Variables.SOURCE_MAKEFILE
+
     # print "setvariable: %r resvariables: %r setvariables: %r" % (vname, resolvevariables, setvariables)
 
     if len(vname) == 0:
         raise SyntaxError("Empty variable name", loc=d.getloc(offset))
 
     if token == '+=':
         val = ''.join((c for c, t, o, oo in iterfunc(d, offset, emptytokenlist)))
         if skipwhitespace:
new file mode 100644
--- /dev/null
+++ b/pymake/process.py
@@ -0,0 +1,28 @@
+"""
+Skipping shell invocations is good, when possible. This wrapper around subprocess does dirty work of
+parsing command lines into argv and making sure that no shell magic is being used.
+"""
+
+import subprocess, shlex, re, logging
+
+_log = logging.getLogger('pymake.execution')
+
+blacklist = re.compile(r'[=\\$><;*?[{~`]')
+def clinetoargv(cline):
+    """
+    If this command line can safely skip the shell, return an argv array.
+    """
+
+    if blacklist.search(cline) is not None:
+        return None
+
+    return shlex.split(cline, comments=True)
+
+def call(cline, env, cwd, loc):
+    argv = clinetoargv(cline)
+    if argv is None:
+        _log.debug("%s: Running command through shell because of shell metacharacters" % (loc,))
+        return subprocess.call(cline, shell=True, env=env, cwd=cwd)
+
+    _log.debug("%s: skipping shell, no metacharacters found" % (loc,))
+    return subprocess.call(argv, shell=False, env=env, cwd=cwd)