Fixes the simple testsuite/make.py problems:
authorBenjamin Smedberg <benjamin@smedbergs.us>
Thu, 05 Feb 2009 14:15:06 -0500
changeset 37 f945469861a575166ad7c94e8d7c69d658cd7029
parent 36 ce053a18a4242e15ef33576f501ae1076f23477f
child 38 084bcc01b9961bf86d7027c7b2dd90b56df7092a
push id20
push userbsmedberg@mozilla.com
push dateThu, 05 Feb 2009 20:06:48 +0000
Fixes the simple testsuite/make.py problems: * Catch Data/Syntax errors and print them simply with an exit code of 2, to match GNU make * minor fixups in the parser and data model
make.py
pymake/data.py
pymake/parser.py
--- a/make.py
+++ b/make.py
@@ -1,20 +1,20 @@
 #!/usr/bin/env python
 
 """
 make.py
 
 A drop-in or mostly drop-in replacement for GNU make.
 """
 
-import os
+import os, subprocess, sys
 from optparse import OptionParser
-from pymake.data import Makefile
-from pymake.parser import parsestream, parsecommandlineargs
+from pymake.data import Makefile, DataError
+from pymake.parser import parsestream, parsecommandlineargs, SyntaxError
 
 op = OptionParser()
 op.add_option('-f', '--file', '--makefile',
               action='append',
               dest='makefiles',
               default=[])
 
 options, arguments = op.parse_args()
@@ -23,23 +23,27 @@ m = Makefile()
 targets = parsecommandlineargs(m, arguments)
 
 if len(options.makefiles) == 0:
     if os.path.exists('Makefile'):
         options.makefiles.append('Makefile')
     else:
         raise Error("No makefile found")
 
-for f in options.makefiles:
-    parsestream(open(f), f, m)
+try:
+    for f in options.makefiles:
+        parsestream(open(f), f, m)
 
-m.finishparsing()
+    m.finishparsing()
 
-if len(targets) == 0:
-    if m.defaulttarget is None:
-        raise Error("No target specified and no default target found.")
-    targets = [m.defaulttarget]
+    if len(targets) == 0:
+        if m.defaulttarget is None:
+            raise Error("No target specified and no default target found.")
+        targets = [m.defaulttarget]
 
-tlist = [m.gettarget(t) for t in targets]
-for t in tlist:
-    t.resolvedeps(m, [], [])
-for t in tlist:
-    t.make(m)
+    tlist = [m.gettarget(t) for t in targets]
+    for t in tlist:
+        t.resolvedeps(m, [], [])
+    for t in tlist:
+        t.make(m)
+except (DataError, SyntaxError, subprocess.CalledProcessError), e:
+    print e
+    sys.exit(2)
--- a/pymake/data.py
+++ b/pymake/data.py
@@ -495,34 +495,36 @@ class Target(object):
                dependencies. A rule chain cannot use the same implicit rule twice.
         """
         assert makefile.parsingfinished
 
         if self.target in targetstack:
             raise DataError("Recursive dependency: %s -> %s" % (
                     " -> ".join(targetstack), self.target))
 
+        targetstack = targetstack + [self.target]
+
         self.resolvevpath(makefile)
 
         # Sanity-check our rules. If we're single-colon, only one rule should have commands
         ruleswithcommands = reduce(lambda i, rule: i + len(rule.commands) > 0, self.rules, 0)
-        if not self.isdoublecolon():
+        if len(self.rules) and not self.isdoublecolon():
             if ruleswithcommands > 1:
                 # In GNU make this is a warning, not an error. I'm going to be stricter.
                 # TODO: provide locations
                 raise DataError("Target '%s' has multiple rules with commands." % self.target)
 
         if ruleswithcommands == 0:
             if len(makefile.implicitrules) > 0:
                 raise NotImplementedError("No rules to make '%s', and implicit rules aren't implemented yet!")
 
         for r in self.rules:
             newrulestack = rulestack + [r]
             for d in r.prerequisitesfor(self.target):
-                makefile.gettarget(d).resolvedeps(makefile, targetstack + [d], newrulestack)
+                makefile.gettarget(d).resolvedeps(makefile, targetstack, newrulestack)
 
     def resolvevpath(self, makefile):
         if self.isphony(makefile):
             self.vpathtarget = self.target
             self.mtime = None
 
         if self.vpathtarget is None:
             # TODO: the vpath is a figment of your imagination
@@ -540,34 +542,34 @@ class Target(object):
         if self.isdoublecolon():
             for r in self.rules:
                 remake = False
                 depcount = 0
                 for p in r.prerequisitesfor(self.target):
                     depcount += 1
                     dep = makefile.gettarget(p)
                     dep.make(makefile)
-                    if mtimeislater(dep(p).mtime, self.mtime):
+                    if mtimeislater(dep.mtime, self.mtime):
                         remake = True
                 if remake or depcount == 0:
                     rule.execute(self, makefile)
         else:
             commandrule = None
             remake = False
             depcount = 0
 
             for r in self.rules:
                 if len(r.commands):
                     assert commandrule is None, "Two command rules for a single-colon target?"
                     commandrule = r
                 for p in r.prerequisitesfor(self.target):
                     depcount += 1
                     dep = makefile.gettarget(p)
                     dep.make(makefile)
-                    if mtimeislater(dep(p).mtime, self.mtime):
+                    if mtimeislater(dep.mtime, self.mtime):
                         remake = True
 
             if remake or depcount == 0:
                 commandrule.execute(self, makefile)
                 
 class Rule(object):
     """
     A rule contains a list of prerequisites and a list of commands. It may also
--- a/pymake/parser.py
+++ b/pymake/parser.py
@@ -145,17 +145,17 @@ class Data(object):
             return None
 
     def append(self, data, loc):
         self._locs.append( (len(self.data), loc) )
         self.data += data
 
     def stripcomment(self):
         cloc = findcommenthash(self.data)
-        if cloc > 0:
+        if cloc != -1:
             self.data = self.data[:cloc]
 
     def getloc(self, offset):
         """
         Get the location of an offset within data.
         """
         if offset >= len(self.data):
             raise IndexError("Invalid offset", offset)
@@ -221,17 +221,18 @@ def parsecommandlineargs(makefile, args)
             if a[eqpos-1] == ':':
                 vname = a[:eqpos-1]
             else:
                 vname = a[:eqpos]
             vname = vname.strip()
             valtext = a[eqpos+1:].lstrip()
             d = Data()
             d.append(valtext, Location('<command-line>', 1, eqpos + 1))
-            value = parsemakesyntax(d, 0, '')
+            value, offset = parsemakesyntax(d, 0, '')
+            assert offset == -1
             setvariable(makefile.variables, vname, a[eqpos-1] == ':', value)
         else:
             r.append(a)
 
     return r
 
 def parsestream(fd, filename, makefile):
     """