* The parsing of functions depends on their argument count: once the last argument has been reached, any commas are included in the argument, rather than starting a new argument. Some functions have variable or unlimited argument counts, so this is slightly complicated
authorBenjamin Smedberg <benjamin@smedbergs.us>
Wed, 11 Feb 2009 10:35:34 -0500
changeset 92 7950a2d51eda46ac6f48e567eca11e8e7254d8e9
parent 91 3a4d76dcfbc1b194b445901ab615ac82746c7bc6
child 93 c6223e135ac65bc958eb19189ff49ad7b895cd45
push id49
push userbsmedberg@mozilla.com
push dateWed, 11 Feb 2009 15:35:47 +0000
* The parsing of functions depends on their argument count: once the last argument has been reached, any commas are included in the argument, rather than starting a new argument. Some functions have variable or unlimited argument counts, so this is slightly complicated * Testcase for a rule with no targets * implement +-@ prefixes for commands to properly ignore failing commands (+ doesn't actually do anything yet, since parallel make isn't yet) * minor fixups for invocation and MAKEFLAGS
make.py
pymake/data.py
pymake/functions.py
pymake/parser.py
tests/functions.mk
tests/ignore-error.mk
tests/notargets.mk
--- a/make.py
+++ b/make.py
@@ -8,17 +8,20 @@ A drop-in or mostly drop-in replacement 
 
 import os, subprocess, sys, logging
 from optparse import OptionParser
 from pymake.data import Makefile, DataError
 from pymake.parser import parsestream, parsecommandlineargs, SyntaxError
 
 def parsemakeflags():
     makeflags = os.environ.get('MAKEFLAGS', '')
-    makeflags.strip()
+    makeflags = makeflags.strip()
+
+    if makeflags == '':
+        return []
 
     opts = []
     curopt = ''
 
     i = 0
     while i < len(makeflags):
         c = makeflags[i]
         if c.isspace():
@@ -38,55 +41,76 @@ def parsemakeflags():
         curopt += c
         i += 1
 
     if curopt != '':
         opts.append(curopt)
 
     return opts
 
+def version(*args):
+    print """pymake: GNU-compatible make program
+Copyright (C) 2009 The Mozilla Foundation <http://www.mozilla.org/>
+This is free software; see the source for copying conditions.
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+DEALINGS IN THE SOFTWARE."""
+    sys.exit(0)
+
 log = logging.getLogger('pymake.execution')
 
+makelevel = int(os.environ.get('MAKELEVEL', '0'))
+
 op = OptionParser()
 op.add_option('-f', '--file', '--makefile',
               action='append',
               dest='makefiles',
               default=[])
-op.add_option('-v', '--verbose',
+op.add_option('-d', '--verbose',
               action="store_true",
-              dest="verbose", default=True)
+              dest="verbose", default=False)
 op.add_option('-C', '--directory',
               dest="directory", default=None)
+op.add_option('-v', '--version',
+              action="callback", callback=version)
+op.add_option('-j', '--jobs', type="int",
+              dest="jobcount", default=1)
 
 arglist = sys.argv[1:] + parsemakeflags()
 
 options, arguments = op.parse_args(arglist)
 
 makeflags = ''
 
 loglevel = logging.WARNING
 if options.verbose:
     loglevel = logging.DEBUG
     makeflags += 'v'
 
+if options.jobcount:
+    log.info("pymake doesn't implement -j yet. ignoring")
+    makeflags += 'j%i' % options.jobcount
+
 logging.basicConfig(level=loglevel)
 
 if options.directory:
     log.info("Switching to directory: %s" % options.directory)
     os.chdir(options.directory)
 
 if len(options.makefiles) == 0:
     if os.path.exists('Makefile'):
         options.makefiles.append('Makefile')
     else:
         print "No makefile found"
         sys.exit(2)
 
-makelevel = int(os.environ.get('MAKELEVEL', '0'))
-
 try:
     i = 0
     while True:
         m = Makefile(restarts=i, make='%s %s' % (sys.executable, sys.argv[0]),
                      makeflags=makeflags, makelevel=makelevel)
         targets = parsecommandlineargs(m, arguments)
 
         for f in options.makefiles:
--- a/pymake/data.py
+++ b/pymake/data.py
@@ -544,17 +544,17 @@ class Target(object):
         # If a target is mentioned, but doesn't exist, has no commands and no
         # prerequisites, it is special and exists just to say that targets which
         # depend on it are always out of date. This is like .FORCE but more
         # compatible with other makes.
         # Otherwise, we don't know how to make it.
         if not len(self.rules) and self.mtime is None and not any((len(rule.prerequisitesfor(self.target)) > 0
                                                                    for rule in self.rules)):
             if required:
-                raise ResolutionError("No rule to make %s needed by %s" % (self.target,
+                raise ResolutionError("No rule to make target '%s' needed by %s" % (self.target,
                     ' -> '.join(targetstack[:-1])))
 
         for r in self.rules:
             newrulestack = rulestack + [r]
             for d in r.prerequisitesfor(self.target):
                 makefile.gettarget(d).resolvedeps(makefile, targetstack, newrulestack)
 
         for v in makefile.getpatternvariablesfor(self.target):
@@ -681,16 +681,44 @@ def splitcommand(command):
             start = i
             continue
 
         i += 1
 
     if i > start:
         yield command[start:i]
 
+def findmodifiers(command):
+    """
+    Find any of +-@ prefixed on the command.
+    @returns (command, isHidden, isRecursive, ignoreErrors)
+    """
+
+    isHidden = False
+    isRecursive = False
+    ignoreErrors = False
+
+    while len(command):
+        c = command[0]
+        if c == '@' and not isHidden:
+            command = command[1:]
+            isHidden = True
+        elif c == '+' and not isRecursive:
+            command = command[1:]
+            isRecursive = True
+        elif c == '-' and not ignoreErrors:
+            command = command[1:]
+            ignoreErrors = True
+        elif c.isspace():
+            command = command[1:]
+        else:
+            break
+
+    return command, isHidden, isRecursive, ignoreErrors
+
 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.
     """
 
     def __init__(self, prereqs, doublecolon, loc):
         self.prerequisites = prereqs
@@ -713,21 +741,24 @@ class Rule(object):
         setautomaticvariables(v, makefile, target, self.prerequisites)
         # TODO: $* in non-pattern rules sucks
 
         env = makefile.getsubenvironment(v)
 
         for c in self.commands:
             cstring = c.resolve(v)
             for cline in splitcommand(cstring):
-                if cline[0:1] == '@':
-                    cline = cline[1:]
+                cline, isHidden, isRecursive, ignoreErrors = findmodifiers(cline)
                 if not len(cline) or cline.isspace():
                     continue
-                subprocess.check_call(cline, shell=True, env=env)
+                if not isHidden:
+                    print "%s $ %s" % (self.loc, cline)
+                r = subprocess.call(cline, shell=True, env=env)
+                if r != 0 and not ignoreErrors:
+                    raise DataError("command '%s' failed, return code was %s" % (cline, r), self.loc)
 
 class PatternRule(object):
     """
     An implicit rule or static pattern rule containing target patterns, prerequisite patterns,
     and a list of commands.
     """
 
     def __init__(self, targetpatterns, prerequisites, doublecolon, loc):
@@ -780,21 +811,27 @@ class PatternRule(object):
 
         dir, stem = self.matchfor(target.target)
 
         v = Variables(parent=target.variables)
         setautomaticvariables(v, makefile, target,
                               self.prerequisitesfor(stem=stem, dir=dir))
         v.set('*', Variables.FLAVOR_SIMPLE, Variables.SOURCE_AUTOMATIC, stem)
 
+        env = makefile.getsubenvironment(v)
+
         for c in self.commands:
             cstring = c.resolve(v)
-            if cstring[0:1] == '@':
-                cstring = cstring[1:]
-            subprocess.check_call(cstring, shell=True)
+            for cline in splitcommand(cstring):
+                cline, isHidden, isRecursive, ignoreErrors = findmodifiers(cline)
+                if not len(cline) or cline.isspace():
+                    continue
+                r = subprocess.call(cline, shell=True, env=env)
+                if r != 0 and not ignoreErrors:
+                    raise DataError("command '%s' failed, return code was %s" % (cline, r), self.loc)
 
 class Makefile(object):
     def __init__(self, workdir=None, restarts=0, make=None, makeflags=None, makelevel=0):
         self.defaulttarget = None
 
         self.variables = Variables()
         self.variables.readfromenvironment()
 
@@ -854,16 +891,22 @@ class Makefile(object):
             if p.match(target):
                 yield v
 
     def hastarget(self, target):
         return target in self._targets
 
     def gettarget(self, target):
         assert isinstance(target, str)
+
+        assert target != '', "empty target?"
+
+        if target.find('*') != -1 or target.find('?') != -1 or target.find('[') != -1:
+            raise DataError("pymake doesn't implement wildcards in targets/prerequisites.")
+
         t = self._targets.get(target, None)
         if t is None:
             t = Target(target, self)
             self._targets[target] = t
         return t
 
     def appendimplicitrule(self, rule):
         assert isinstance(rule, PatternRule)
@@ -932,23 +975,23 @@ class Makefile(object):
         for vname in self.exportedvars:
             flavor, source, val = variables.get(vname)
             if val is None:
                 strval = ''
             else:
                 strval = val.resolve(variables, [vname])
             env[vname] = strval
 
+        makeflags = ''
+
         flavor, source, val = variables.get('MAKEFLAGS')
-        if val is None:
-            makeflags = ''
-        else:
-            makeflags = '-' + val.resolve(variables, ['MAKEFLAGS'])
+        if val is not None:
+            flagsval = val.resolve(variables, ['MAKEFLAGS'])
+            if flagsval != '':
+                makeflags = '-' + flagsval
 
         makeflags += ' -- '
         makeflags += ' '.join((self.flagescape.sub(r'\\\1', o) for o in self.overrides))
 
-        print "makeflags: %r" % makeflags
-
         env['MAKEFLAGS'] = makeflags
 
         env['MAKELEVEL'] = str(self.makelevel + 1)
         return env
--- a/pymake/functions.py
+++ b/pymake/functions.py
@@ -5,44 +5,47 @@ Makefile functions.
 from pymake import data
 import subprocess, os, glob
 
 log = data.log
 
 class Function(object):
     """
     An object that represents a function call. This class is always subclassed
-    with the following two methods:
+    with the following methods and attributes:
 
-    def setup(self)
-        validates the number of arguments to a function
-        no return value
+    minargs = minimum # of arguments
+    maxargs = maximum # of arguments (0 means unlimited)
+
     def resolve(self, variables, setting)
         Calls the function
         @returns string
     """
     def __init__(self, loc):
         self._arguments = []
         self.loc = loc
+        assert self.minargs > 0
 
     def __getitem__(self, key):
         return self._arguments[key]
 
     def setup(self):
-        self.expectargs(self.expectedargs)
+        argc = len(self._arguments)
+
+        if argc < self.minargs:
+            raise data.DataError("Not enough arguments to function %s, requires %s" % (self.name, self.minargs), self.loc)
+
+        assert self.maxargs == 0 or argc <= self.maxargs, "Parser screwed up, gave us too many args"
 
     def append(self, arg):
         assert isinstance(arg, data.Expansion)
         self._arguments.append(arg)
 
-    def expectargs(self, argc):
-        if len(self._arguments) < argc:
-            raise data.DataError("Not enough arguments to function %s" % self.name, self.loc)
-        if len(self._arguments) > argc:
-            log.warning("%s: %s function takes three arguments, got %i" % (self.loc, self.name, len(self._arguments)))
+    def __len__(self):
+        return len(self._arguments)
 
 class VariableRef(Function):
     def __init__(self, loc, vname):
         self.loc = loc
         assert isinstance(vname, data.Expansion)
         self.vname = vname
         
     def setup(self):
@@ -50,17 +53,17 @@ class VariableRef(Function):
 
     def resolve(self, variables, setting):
         vname = self.vname.resolve(variables, setting)
         if vname in setting:
             raise data.DataError("Setting variable '%s' recursively references itself." % (vname,), self.loc)
 
         flavor, source, value = variables.get(vname)
         if value is None:
-            log.warning("%s: variable '%s' has no value" % (self.loc, vname))
+            log.info("%s: variable '%s' was not set" % (self.loc, vname))
             return ''
 
         return value.resolve(variables, setting + [vname])
 
 class SubstitutionRef(Function):
     """$(VARNAME:.c=.o) and $(VARNAME:%.c=%.o)"""
     def __init__(self, loc, varname, substfrom, substto):
         self.loc = loc
@@ -76,128 +79,137 @@ class SubstitutionRef(Function):
         if vname in setting:
             raise data.DataError("Setting variable '%s' recursively references itself." % (vname,), self.loc)
 
         substfrom = self.substfrom.resolve(variables, setting)
         substto = self.substto.resolve(variables, setting)
 
         flavor, source, value = variables.get(vname)
         if value is None:
-            log.warning("%s: variable '%s' has no value" % (self.loc, vname))
+            log.info("%s: variable '%s' was not set" % (self.loc, vname))
             return ''
 
         evalue = value.resolve(variables, setting + [vname])
         words = data.splitwords(evalue)
 
         f = data.Pattern(substfrom)
         if not f.ispattern():
             f = data.Pattern('%' + substfrom)
             substto = '%' + substto
 
         return " ".join((f.subst(substto, word, False)
                          for word in words))
 
 class SubstFunction(Function):
     name = 'subst'
-    expectedargs = 3
+    minargs = 3
+    maxargs = 3
 
     def resolve(self, variables, setting):
         s = self._arguments[0].resolve(variables, setting)
         r = self._arguments[1].resolve(variables, setting)
         d = self._arguments[2].resolve(variables, setting)
         return d.replace(s, r)
 
 class PatSubstFunction(Function):
     name = 'patsubst'
-    expectedargs = 3
+    minargs = 3
+    maxargs = 3
 
     def resolve(self, variables, setting):
         s = self._arguments[0].resolve(variables, setting)
         r = self._arguments[1].resolve(variables, setting)
         d = self._arguments[2].resolve(variables, setting)
 
         p = data.Pattern(s)
         return ' '.join((p.subst(r, word, False)
                          for word in data.splitwords(d)))
 
 class StripFunction(Function):
     name = 'strip'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         return ' '.join(data.splitwords(self._arguments[0].resolve(variables, setting)))
 
 class FindstringFunction(Function):
     name = 'findstring'
-    expectedargs = 2
+    minargs = 2
+    maxargs = 2
 
     def resolve(self, variables, setting):
         s = self._arguments[0].resolve(variables, setting)
         r = self._arguments[1].resolve(variables, setting)
         if r.find(s) == -1:
             return ''
         return s
 
 class FilterFunction(Function):
     name = 'filter'
-    expectedargs = 2
+    minargs = 2
+    maxargs = 2
 
     def resolve(self, variables, setting):
         ps = self._arguments[0].resolve(variables, setting)
         d = self._arguments[1].resolve(variables, setting)
         plist = [data.Pattern(p) for p in data.splitwords(ps)]
         r = []
         for w in data.splitwords(d):
             if any((p.match(w) for p in plist)):
                     r.append(w)
                 
         return ' '.join(r)
 
 class FilteroutFunction(Function):
     name = 'filter-out'
-    expectedargs = 2
+    minargs = 2
+    maxargs = 2
 
     def resolve(self, variables, setting):
         ps = self._arguments[0].resolve(variables, setting)
         d = self._arguments[1].resolve(variables, setting)
         plist = [data.Pattern(p) for p in data.splitwords(ps)]
         r = []
         for w in data.splitwords(d):
             found = False
             if not any((p.match(w) for p in plist)):
                 r.append(w)
 
         return ' '.join(r)
 
 class SortFunction(Function):
     name = 'sort'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         d = self._arguments[0].resolve(variables, setting)
         w = data.splitwords(d)
         w.sort()
         return ' '.join((w for w in data.withoutdups(w)))
 
 class WordFunction(Function):
     name = 'word'
-    expectedargs = 2
+    minargs = 2
+    maxargs = 2
 
     def resolve(self, variables, setting):
         n = self._arguments[0].resolve(variables, setting)
         # TODO: provide better error if this doesn't convert
         n = int(n)
         words = data.splitwords(self._arguments[1].resolve(variables, setting))
         if n < 1 or n > len(words):
             return ''
         return words[n - 1]
 
 class WordlistFunction(Function):
     name = 'wordlist'
-    expectedargs = 3
+    minargs = 3
+    maxargs = 3
 
     def resolve(self, variables, setting):
         nfrom = self._arguments[0].resolve(variables, setting)
         nto = self._arguments[1].resolve(variables, setting)
         # TODO: provide better errors if this doesn't convert
         nfrom = int(nfrom)
         nto = int(nto)
 
@@ -207,34 +219,37 @@ class WordlistFunction(Function):
             nfrom = 1
         if nto < 1:
             nto = 1
 
         return ' '.join(words[nfrom - 1:nto])
 
 class WordsFunction(Function):
     name = 'words'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         return str(len(data.splitwords(self._arguments[0].resolve(variables, setting))))
 
 class FirstWordFunction(Function):
     name = 'firstword'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         wl = data.splitwords(self._arguments[0].resolve(variables, setting))
         if len(wl) == 0:
             return ''
         return wl[0]
 
 class LastWordFunction(Function):
     name = 'lastword'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         wl = data.splitwords(self._arguments[0].resolve(variables, setting))
         if len(wl) == 0:
             return ''
         return wl[0]
 
 def pathsplit(path, default='./'):
@@ -245,179 +260,186 @@ def pathsplit(path, default='./'):
     dir, slash, file = path.rpartition('/')
     if dir == '':
         return default, file
 
     return dir + slash, file
 
 class DirFunction(Function):
     name = 'dir'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         return ' '.join((pathsplit(path)[0]
                          for path in data.splitwords(self._arguments[0].resolve(variables, setting))))
 
 class NotDirFunction(Function):
     name = 'notdir'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         return ' '.join((pathsplit(path)[1]
                          for path in data.splitwords(self._arguments[0].resolve(variables, setting))))
 
 class SuffixFunction(Function):
     name = 'suffix'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     @staticmethod
     def suffixes(words):
         for w in words:
             dir, file = pathsplit(w)
             base, dot, suffix = file.rpartition('.')
             if base != '':
                 yield dot + suffix
 
     def resolve(self, variables, setting):
         return ' '.join(self.suffixes(data.splitwords(self._arguments[0].resolve(variables, setting))))
 
 class BasenameFunction(Function):
     name = 'basename'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     @staticmethod
     def basenames(words):
         for w in words:
             dir, file = pathsplit(w, '')
             base, dot, suffix = file.rpartition('.')
             if dot == '':
                 base = suffix
 
             yield dir + base
 
     def resolve(self, variables, setting):
         return ' '.join(self.basenames(data.splitwords(self._arguments[0].resolve(variables, setting))))
 
 class AddSuffixFunction(Function):
     name = 'addprefix'
-    expectedargs = 2
+    minargs = 2
+    maxargs = 2
 
     def resolve(self, variables, setting):
         suffix = self._arguments[0].resolve(variables, setting)
 
         return ' '.join((w + suffix for w in data.splitwords(self._arguments[1].resolve(variables, setting))))
 
 class AddPrefixFunction(Function):
     name = 'addsuffix'
-    expectedargs = 2
+    minargs = 2
+    maxargs = 2
 
     def resolve(self, variables, setting):
         prefix = self._arguments[0].resolve(variables, setting)
 
         return ' '.join((prefix + w for w in data.splitwords(self._arguments[1].resolve(variables, setting))))
 
 class JoinFunction(Function):
     name = 'join'
-    expectedargs = 2
+    minargs = 2
+    maxargs = 2
 
     @staticmethod
     def iterjoin(l1, l2):
         for i in xrange(0, max(len(l1), len(l2))):
             i1 = i < len(l1) and l1[i] or ''
             i2 = i < len(l2) and l2[i] or ''
             yield i1 + i2
 
     def resolve(self, variables, setting):
         list1 = data.splitwords(self._arguments[0].resolve(variables, setting))
         list2 = data.splitwords(self._arguments[1].resolve(variables, setting))
 
         return ' '.join(self.iterjoin(list1, list2))
 
 class WildcardFunction(Function):
     name = 'wildcard'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         # TODO: will need work when we support -C without actually changing the OS cwd
         pattern = self._arguments[0].resolve(variables, setting)
         return ' '.join(glob.glob(pattern))
 
 class RealpathFunction(Function):
     name = 'realpath'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         # TODO: will need work when we support -C without actually changing the OS cwd
         return ' '.join((os.path.realpath(f)
                          for f in data.splitwords(self._arguments[0].resolve(variables, setting))))
 
 class AbspathFunction(Function):
     name = 'abspath'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         # TODO: will need work when we support -C without actually changing the OS cwd
         return ' '.join((os.path.abspath(f)
                          for f in data.splitwords(self._arguments[0].resolve(variables, setting))))
 
 class IfFunction(Function):
     name = 'if'
+    minargs = 1
+    maxargs = 3
 
     def setup(self):
-        if len(self._arguments) < 2:
-            raise data.DataError("Not enough arguments to function if", self.loc)
-        if len(self._arguments) > 3:
-            log.warning("%s: if function takes no more than three arguments, got %i" % (self.loc,))
-
+        Function.setup(self)
         self._arguments[0].lstrip()
         self._arguments[0].rstrip()
 
     def resolve(self, variables, setting):
         condition = self._arguments[0].resolve(variables, setting)
         if len(condition):
             return self._arguments[1].resolve(variables, setting)
 
         if len(self._arguments) > 2:
             return self._arguments[2].resolve(variables, setting)
 
         return ''
 
 class OrFunction(Function):
     name = 'or'
-
-    def setup(self):
-        pass
+    minargs = 1
+    maxargs = 0
 
     def resolve(self, variables, setting):
         for arg in self._arguments:
             r = arg.resolve(variables, setting)
             if r != '':
                 return r
 
         return ''
 
 class AndFunction(Function):
     name = 'and'
-
-    def setup(self):
-        pass
+    minargs = 1
+    maxargs = 0
 
     def resolve(self, variables, setting):
         r = ''
 
         for arg in self._arguments:
             r = arg.resolve(variables, setting)
             if r == '':
                 return ''
 
         return r
 
 class ForEachFunction(Function):
     name = 'foreach'
-    expectedargs = 3
+    minargs = 3
+    maxargs = 3
 
     def resolve(self, variables, setting):
         vname = self._arguments[0].resolve(variables, setting)
 
         words = data.splitwords(self._arguments[1].resolve(variables, setting))
         e = self._arguments[2]
 
         results = []
@@ -426,19 +448,18 @@ class ForEachFunction(Function):
         for w in words:
             v.set(vname, data.Variables.FLAVOR_SIMPLE, data.Variables.SOURCE_AUTOMATIC, w)
             results.append(e.resolve(v, setting))
 
         return ' '.join(results)
 
 class CallFunction(Function):
     name = 'call'
-
-    def setup(self):
-        assert len(self._arguments) > 0
+    minargs = 1
+    maxargs = 0
 
     def resolve(self, variables, setting):
         vname = self._arguments[0].resolve(variables, setting)
         if vname in setting:
             raise data.DataError("Recursively setting variable '%s'" % (vname,))
 
         v = data.Variables(parent=variables)
         v.set('0', data.Variables.FLAVOR_SIMPLE, data.Variables.SOURCE_AUTOMATIC, vname)
@@ -453,37 +474,40 @@ class CallFunction(Function):
         if flavor == data.Variables.FLAVOR_SIMPLE:
             log.warning("%s: calling variable '%s' which is simply-expanded" % (self.loc, vname))
 
         # but we'll do it anyway
         return e.resolve(v, setting + [vname])
 
 class ValueFunction(Function):
     name = 'value'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         varname = self._arguments[0].resolve(variables, setting)
 
         flavor, source, value = variables.get(varname, expand=False)
         if value is None:
             return ''
 
         return value
 
 class EvalFunction(Function):
     name = 'eval'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         raise NotImplementedError('no eval yet')
 
 class OriginFunction(Function):
     name = 'origin'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         vname = self._arguments[0].resolve(variables, setting)
 
         flavor, source, value = variables.get(vname)
         if source is None:
             return 'undefined'
 
@@ -501,17 +525,18 @@ class OriginFunction(Function):
 
         if source == data.Variables.SOURCE_AUTOMATIC:
             return 'automatic'
 
         assert False, "Unexpected source value: %s" % source
 
 class FlavorFunction(Function):
     name = 'flavor'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         varname = self._arguments[0].resolve(variables, setting)
         
         flavor, source, value = variables.get(varname)
         if flavor is None:
             return 'undefined'
 
@@ -519,51 +544,55 @@ class FlavorFunction(Function):
             return 'recursive'
         elif flavor == data.Variables.FLAVOR_SIMPLE:
             return 'simple'
 
         assert False, "Neither simple nor recursive?"
 
 class ShellFunction(Function):
     name = 'shell'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         cline = self._arguments[0].resolve(variables, setting)
 
         p = subprocess.Popen(cline, shell=True, stdout=subprocess.PIPE)
         stdout, stderr = p.communicate()
 
         stdout.replace('\r\n', '\n')
         if len(stdout) > 1 and stdout[-1] == '\n':
             stdout = stdout[:-1]
         stdout.replace('\n', ' ')
 
         return stdout
 
 class ErrorFunction(Function):
     name = 'error'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         v = self._arguments[0].resolve(variables, setting)
         raise data.DataError(v, self.loc)
 
 class WarningFunction(Function):
     name = 'warning'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         v = self._arguments[0].resolve(variables, setting)
         log.warning(v)
         return ''
 
 class InfoFunction(Function):
     name = 'info'
-    expectedargs = 1
+    minargs = 1
+    maxargs = 1
 
     def resolve(self, variables, setting):
         v = self._arguments[0].resolve(variables, setting)
         log.info(v)
         return ''
 
 functionmap = {
     'subst': SubstFunction,
--- a/pymake/parser.py
+++ b/pymake/parser.py
@@ -428,16 +428,17 @@ def ifeq(d, offset, makefile):
             raise SyntaxError("Unexpected text in conditional", d.getloc(offset))
 
         arg2, t, offset = parsemakesyntax(d, offset + 1, (token,), itermakefilechars)
 
         ensureend(d, offset, "Unexpected text after conditional")
 
     val1 = arg1.resolve(makefile.variables)
     val2 = arg2.resolve(makefile.variables)
+
     return val1 == val2
 
 def ifneq(d, offset, makefile):
     return not ifeq(d, offset, makefile)
 
 def ifdef(d, offset, makefile):
     e, t, offset = parsemakesyntax(d, offset, (), itermakefilechars)
     e.rstrip()
@@ -542,16 +543,25 @@ def parsestream(fd, filename, makefile):
                     if kword not in conditionkeywords:
                         raise SyntaxError("Unexpected condition after 'else' directive.",
                                           d.getloc(offset))
                         
                     m = conditionkeywords[kword](d, offset, makefile)
                     condstack[-1].makeactive(m)
                 continue
 
+            if kword in conditionkeywords:
+                m = conditionkeywords[kword](d, offset, makefile)
+                condstack.append(Condition(m, d.getloc(offset)))
+                continue
+
+            if any((not c.active for c in condstack)):
+                log.info('%s: skipping line because of active conditions' % (d.getloc(0),))
+                continue
+
             if kword == 'endef':
                 raise SyntaxError("Unmatched endef", d.getloc(offset))
 
             if kword == 'define':
                 e, t, i = parsemakesyntax(d, offset, (), itermakefilechars)
 
                 d = Data(fdlines, filename)
                 d.readline()
@@ -564,21 +574,16 @@ def parsestream(fd, filename, makefile):
 
             if kword in ('include', '-include'):
                 incfile, t, offset = parsemakesyntax(d, offset, (), itermakefilechars)
                 files = data.splitwords(incfile.resolve(makefile.variables))
                 for f in files:
                     makefile.include(f, kword == 'include')
                 continue
 
-            if kword in conditionkeywords:
-                m = conditionkeywords[kword](d, offset, makefile)
-                condstack.append(Condition(m, d.getloc(offset)))
-                continue
-
             if kword == 'override':
                 e, token, offset = parsemakesyntax(d, offset, varsettokens, itermakefilechars)
                 e.lstrip()
                 e.rstrip()
 
                 if token is None:
                     raise SyntaxError("Malformed override directive, need =", d.getloc(offset))
 
@@ -609,20 +614,16 @@ def parsestream(fd, filename, makefile):
 
                 continue
 
             if kword == 'unexport':
                 raise SyntaxError("unexporting variables is not supported", d.getloc(offset))
 
             assert kword is None, "unexpected kword: %r" % (kword,)
 
-            if any((not c.active for c in condstack)):
-                log.info('%s: skipping line because of active conditions' % (d.getloc(0),))
-                continue
-
             e, token, offset = parsemakesyntax(d, offset, varsettokens + ('::', ':'), itermakefilechars)
             if token is None:
                 v = e.resolve(makefile.variables)
                 if v.strip() != '':
                     raise SyntaxError("Bad syntax: non-empty line is not a variable assignment or rule.", loc=d.getloc(0))
                 continue
 
             # if we encountered real makefile syntax, the current rule is over
@@ -789,20 +790,22 @@ def parsemakesyntax(d, startat, stopon, 
             di = iterfunc(d, offset)
 
             if stacktop.parsestate == PARSESTATE_TOPLEVEL:
                 assert len(stack) == 1
                 return stacktop.expansion, token, offset
 
             if stacktop.parsestate == PARSESTATE_FUNCTION:
                 if token == ',':
+                    stacktop.expansion = data.Expansion()
                     stacktop.function.append(stacktop.expansion)
-                    stacktop.expansion = data.Expansion()
+
+                    if len(stacktop.function) == stacktop.function.maxargs:
+                        stacktop.stopon = (')',)
                 elif token == ')':
-                    stacktop.function.append(stacktop.expansion)
                     stacktop.function.setup()
                     stack.pop()
                     stack[-1].expansion.append(stacktop.function)
                 else:
                     assert False, "Not reached, PARSESTATE_FUNCTION"
             elif stacktop.parsestate == PARSESTATE_VARNAME:
                 if token == ':':
                     stacktop.varname = stacktop.expansion
@@ -852,19 +855,25 @@ def parsemakesyntax(d, startat, stopon, 
                 stacktop.expansion.append('$')
                 continue
 
             if c == '(':
                 # look forward for a function name
                 fname, offset = d.findtoken(offset + 1, functiontokens, True)
                 if fname is not None:
                     fn = functions.functionmap[fname](loc)
+                    e = data.Expansion()
+                    fn.append(e)
+                    if len(fn) == fn.maxargs:
+                        stopon = (')',)
+                    else:
+                        stopon = (',', ')')
+
                     stack.append(ParseStackFrame(PARSESTATE_FUNCTION,
-                                                 data.Expansion(), ',)',
-                                                 function=fn))
+                                                 e, stopon, function=fn))
                     di = iterfunc(d, offset)
                     continue
 
                 e = data.Expansion()
                 stack.append(ParseStackFrame(PARSESTATE_VARNAME, e, (':', ')'), loc=loc))
                 continue
 
             fe = data.Expansion()
--- a/tests/functions.mk
+++ b/tests/functions.mk
@@ -1,16 +1,17 @@
 all:
 	test "$(subst e,EE,hello)" = "hEEllo"
 	test "$(strip $(NULL)  test data  )" = "test data"
 	test "$(findstring hell,hello)" = "hell"
 	test "$(findstring heaven,hello)" = ""
 	test "$(filter foo/%.c b%,foo/a.c b.c foo/a.o)" = "foo/a.c b.c"
 	test "$(filter foo,foo bar)" = "foo"
 	test "$(filter-out foo/%.c b%,foo/a.c b.c foo/a.o)" = "foo/a.o"
+	test "$(filter-out %.c,foo,bar.c foo,bar.o)" = "foo,bar.o"
 	test "$(sort .go a b aa A c cc)" = ".go A a aa b c cc"
 	test "$(word 1, hello )" = "hello"
 	test "$(word 2, hello )" = ""
 	test "$(wordlist 1, 2, foo bar baz )" = "foo bar"
 	test "$(words 1 2 3)" = "3"
 	test "$(words )" = "0"
 	test "$(firstword $(NULL) foo bar baz)" = "foo"
 	test "$(firstword )" = ""
new file mode 100644
--- /dev/null
+++ b/tests/ignore-error.mk
@@ -0,0 +1,13 @@
+all:
+	-rm foo
+	+-rm bar
+	-+rm baz
+	@-rm bah
+	-@rm humbug
+	+-@rm sincere
+	+@-rm flattery
+	@+-rm will
+	@-+rm not
+	-+@rm save
+	-@+rm you
+	@echo TEST-PASS
new file mode 100644
--- /dev/null
+++ b/tests/notargets.mk
@@ -0,0 +1,5 @@
+$(NULL): foo.c
+	@echo TEST-FAIL
+
+all:
+	@echo TEST-PASS