Merge pull request #29 from linuxlizard/dpoole/fix-multiple-targets

fix bug with multiple targets appending same recipe

Author: linuxlizard

pymake/pymake.py

@@ -294,13 +294,15 @@ def _is_recipe_comment(tok):
         # corner case to handle a line with leading <tab> and a comment
         # which will be a Recipe if we're inside a Rule but is ignored 
         # before we've seen a Rule
+        tok = tok.token_list[0]
         try:
-            tok = tok.token_list[0]
             lit = tok.literal
         except AttributeError:
             # not a literal therefore definitely can't be a comment
             return False
-        return tokenizer.seek_comment(iter(tok.string))
+        s = tok.makefile().lstrip()
+        return s[0] == '#'
+#        return tokenizer.seek_comment(iter(tok.string))
 
     for statement in stmt_list:
         # sanity check; everything has to have a successful get_pos()
@@ -314,18 +316,46 @@ def _is_recipe_comment(tok):
             # If this is just a comment line, ignore it
             # But if we haven't seen a rule, throw the infamous error.
 
-            if not curr_rules and not _is_recipe_comment(statement):
+            # use a better name
+            recipe = statement
+
+            if not curr_rules and not _is_recipe_comment(recipe):
                 # So We're confused. 
                 raise RecipeCommencesBeforeFirstTarget(pos=statement.get_pos())
 
-            [rule.add_recipe(statement) for rule in curr_rules]
+            # The RuleDB contains a Rule instance for each target.  The RuleDB
+            # is key'd by the target string. Given a rule with multiple targets
+            # such as 
+            # a b c d:  ; @echo $@
+            # there will be four separate Rule instances in the RuleDB.  
+            #
+            # Each Rule instance has a RecipeList instance (which is basically
+            # just a wrapper around a python list to automatically support the
+            # .makefile() method).  The Rule's RecipeList is created from the
+            # RuleExpression. 
+            #
+            # Here's the punchline:
+            # The Symbol class hierarchy creates one RuleExpression and one
+            # RecipeList even if there are multiple targets. But the RuleDB
+            # uses a separate Rule instance for each target. The RecipeList
+            # instance is therefore shared between each Rule (basically, all
+            # Rule instances point to the exact same RecipeList instance)
+            #
+            if curr_rules:
+                if len(curr_rules) > 1:
+                    # sanity clause
+                    id_ = id(curr_rules[0].recipe_list)
+                    assert all( id_==id(r.recipe_list) for r in curr_rules)
+                curr_rules[0].add_recipe(recipe)
+#            [rule.add_recipe(recipe) for rule in curr_rules]
 
         elif isinstance(statement,RuleExpression):
             # restart the rules list but maintain the same ref!
             # (need the same ref because this array is passed by the caller and
             # we need to track the values across calls to this function)
             curr_rules.clear()
 
+            # use a better name
             rule_expr = statement
             m = rule_expr.makefile()
 
@@ -602,7 +632,7 @@ def execute(makefile, args):
     # target" error.
     # 
     # Basically, we have context sensitive evaluation.
-    curr_rules = []
+    curr_rules = []  # array of Rule instances
 
     # For handling $(eval)  This is not my proudest moment.  I originally
     # designed my make function implementations to be truly functional (no side

pymake/rules.py

@@ -34,11 +34,12 @@ def __init__(self, target, prereq_list, recipe_list, assignment, pos):
             assert '\t' not in target
             assert target
         assert all( (isinstance(s,str) for s in prereq_list) )
+        _ = recipe_list.makefile
 
         logger.debug("create rule target=%r at %r", target, pos)
         self.target = target
         self.prereq_list = list(prereq_list)
-        self.recipe_list = list(recipe_list)
+        self.recipe_list = recipe_list
         self.assignment_list = [assignment] if assignment else []
 
         _rule_sanity(pos, prereq_list, assignment)
@@ -88,6 +89,8 @@ def graphviz_graph(self):
 
 class RuleDB:
     def __init__(self):
+        # key: target (python string)
+        # value: instance of class Rule
         self.rules = {}
 
         # first rule added becomes the default
@@ -96,6 +99,8 @@ def __init__(self):
     def add(self, target, prereq_list, recipe_list, assignment, pos):
 
         # ha ha type checking
+        _ = recipe_list.makefile
+
         logger.debug("add rule target=%r at %r", target, pos)
 
         if not target:

pymake/symbol.py

@@ -639,6 +639,9 @@ def __init__(self, recipe_list):
         super().__init__(recipe_list)
 
     def append(self, recipe):
+        # ha ha type checking
+        assert isinstance(recipe, Recipe), recipe
+
         self.token_list.append(recipe)
 
     def makefile(self):

tests/test_makefile.py

@@ -8,7 +8,8 @@
 import re
 
 import pytest
-#import pymake
+
+import run
 
 # Find file relative to tests location
 test_dir = os.path.dirname(__file__)
@@ -134,10 +135,17 @@ def test_makefile(infilename):
             outfile.write(ground_truth)
     assert ground_truth == test_output
 
+@pytest.mark.skip(reason="run manually because slow")
+@pytest.mark.parametrize("infilename", infilename_list)
+def test_pymake_options(infilename):
+    # try some pymake args on the test makefiles
+    # (sanity test code paths)
+    filepath = os.path.join(example_dir, infilename)
+    s = run.run_pymake(filepath, extra_args=("--print-rule",))
+    s = run.run_pymake(filepath, extra_args=("-S",))
+    s = run.run_pymake(filepath, extra_args=("-n",))
+    s = run.run_pymake(filepath, extra_args=("--output", "/dev/null"))
 
-    # can I run pymake w/i pytest w/o spawning an additional python?
-#    print(os.getcwd())
-#    makefile = pymake.parse_makefile(infilename)
 
 def test_value():
     # TODO need some way of testing value() function w/o running afoul of my $(P) problem