From cea316921f7f786977902711c526aa6376075d8a Mon Sep 17 00:00:00 2001 From: David Poole Date: Sun, 5 Jan 2025 16:22:18 -0800 Subject: [PATCH 1/4] start of rule specific variable assignments - add assignment expression list to the Rule class - rearrange adding Rule to RuleDB in order to handle the subtleties of adding assignments to a rule vs overriding a rule (needs more tests) - fix a few places where .makefile() returned bad recipes (and fix the subsequent failing tests) --- pymake/pymake.py | 21 +++-- pymake/rules.py | 72 ++++++++++++++---- pymake/symbol.py | 50 +++++++----- pymake/symtable.py | 27 ++++--- tests/target-specific.mk | 160 +++++++++------------------------------ tests/test_tokenizer.py | 12 +-- 6 files changed, 166 insertions(+), 176 deletions(-) diff --git a/pymake/pymake.py b/pymake/pymake.py index b82aece..6e220c3 100644 --- a/pymake/pymake.py +++ b/pymake/pymake.py @@ -308,18 +308,29 @@ def _is_recipe_comment(tok): curr_rules.clear() rule_expr = statement + m = rule_expr.makefile() # Note a RuleExpression.eval() is very different from all other # eval() methods (so far). A RuleExpression.eval() returns two # arrays of strings: targets, prereqs # All other eval() returns a string. target_list, prereq_list = rule_expr.eval(symtable) - + + if rule_expr.assignment: + # Have a target-specific assignment expression. + # There will be no prereqs or attached recipes. + # + # The assignments will not be eval'd here but rather stored in + # the Rule and eval'd when the Rule is used. + assert not prereq_list + if target_list: for t in target_list: - rule = rules.Rule(t, prereq_list, rule_expr.recipe_list, rule_expr.get_pos()) - rulesdb.add(rule) + rule = rulesdb.add(t, prereq_list, rule_expr.recipe_list, + rule_expr.assignment, rule_expr.get_pos()) curr_rules.append(rule) + # we're in a big confusing loop so get rid of a name I re-use + del rule else: # "We accept and ignore rules without targets for # compatibility with SunOS 4 make." -- GNU Make src/read.c @@ -327,7 +338,7 @@ def _is_recipe_comment(tok): # We need to have a Rule in curr_rules to correctly parse # ambiguous statements with have a leading Recipe Prefix # (aka ) - rule = rules.Rule(None, prereq_list, rule_expr.recipe_list, rule_expr.get_pos()) + rule = rules.Rule(None, prereq_list, rule_expr.recipe_list, rule_expr.assignment, rule_expr.get_pos()) # don't add this Rule to the DB but do let the world know we are in a Rule curr_rules.append(rule) @@ -432,7 +443,7 @@ def check_prefixes(s): s = s[1:] elif s[0] == '+': - raise NotImplementedError + raise NotImplementedError() else: break diff --git a/pymake/rules.py b/pymake/rules.py index 920bcfa..0f10751 100644 --- a/pymake/rules.py +++ b/pymake/rules.py @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2014-2025 David Poole davep@mbuf.com david.poole@ericsson.com import logging import os @@ -12,11 +13,19 @@ _debug = True +def _rule_sanity(pos, prereq_list, assignment): + # if we have a prereq list, we cannot have an assignment + # if we have an assignment, we cannot have a prereq list + if prereq_list: + assert not assignment, pos + elif assignment: + assert not prereq_list, pos + class Rule: # A Rule contains a target (string) and the prereqs (array of string). # Rule contains a RecipeList from the Symbol hierarchy but don't want to # pull symbol.py in here (to keep pymake.py a little more clean). - def __init__(self, target, prereq_list, recipe_list, pos): + def __init__(self, target, prereq_list, recipe_list, assignment, pos): # ha ha type checking # target is string and prereq_list[] is array of strings # target can be None to handle rules without a target which GNU Make allows @@ -30,6 +39,9 @@ def __init__(self, target, prereq_list, recipe_list, pos): self.target = target self.prereq_list = prereq_list self.recipe_list = recipe_list + self.assignment_list = [assignment] if assignment else [] + + _rule_sanity(pos, prereq_list, assignment) # need to pass in an explict pos because the target is a string not a # Symbol. @@ -39,13 +51,22 @@ def __str__(self): target = "" if self.target is None else self.target return "%s <- %s" % (target, ",".join(self.prereq_list)) + def makefile(self): + s = "".join([ "%s:%s\n" % (self.target,a.makefile()) for a in self.assignment_list]) + s += "%s : %s\n%s" % (self.target, " ".join(self.prereq_list), self.recipe_list.makefile()) + return s + def get_pos(self): return self.pos def add_recipe(self, recipe): - logger.debug("add recipe to rule=%r", self) + logger.debug("add recipe to rule=%r", self.target) self.recipe_list.append(recipe) + def add_assignment(self, assignment): + logger.debug("add assignment to rule=%r", self.target) + self.assignment_list.append(assignment) + def graphviz_graph(self): if self.target is None: raise ValueError("cannot build a graphviz for rule without targets at pos=%r" % self.pos) @@ -72,28 +93,51 @@ def __init__(self): # first rule added becomes the default self.default = None - def add(self, rule): + def add(self, target, prereq_list, recipe_list, assignment, pos): + # ha ha type checking - logger.debug("add rule=%s at %r", rule, rule.get_pos()) - assert isinstance(rule,Rule), type(rule) + logger.debug("add rule target=%r at %r", target, pos) - if not rule.target: + if not target: # TODO breakpoint() assert rule.target - if rule.target == ".PHONY": - # TODO - return + if target == ".PHONY": + raise NotImplementedError(target) + + _rule_sanity(pos, prereq_list, assignment) + + # do we currently have a rule already with this target? + rule = self.rules.get(target,None) + if not rule: + # create new + rule = Rule(target, prereq_list, recipe_list, assignment, pos) + else: + # could be an update or could be an overwrite + # + # If we don't have an assignment arg, and we've already seen some recipes, + # then we have a new rule. + if not assignment and len(rule.recipe_list): + # overwrite + warning_message(pos, "overriding recipe for target '%s'" % target ) + warning_message(rule.get_pos(), "ignoring old recipe for target '%s'" % target) + + rule = Rule(target, prereq_list, recipe_list, assignment, pos) + else: + # update existing rule + if recipe_list: + [rule.add_recipe(r) for r in recipe_list] + elif assignment: + rule.add_assignment(assignment) + + self.rules[rule.target] = rule if self.default is None: self.default = rule.target - - # GNU Make doesn't warn on this sort of thing but I want to see it. - if rule.target in self.rules and rule.prereq_list: - warning_message(rule.get_pos(), "overriding rule \"%s\"" % (rule.target, )) - self.rules[rule.target] = rule + return rule + def get(self, target): # allow KeyError to propagate diff --git a/pymake/symbol.py b/pymake/symbol.py index 6961800..6a2a1cd 100644 --- a/pymake/symbol.py +++ b/pymake/symbol.py @@ -466,21 +466,11 @@ def __init__(self, token_list): # add sanity check in constructor Symbol.validate(token_list) + # ha ha type checking assert len(token_list)==3, len(token_list) - assert isinstance(token_list[0], TargetList), (type(token_list[0]),) assert isinstance(token_list[1], RuleOp), (type(token_list[1]),) - if isinstance(token_list[2], PrerequisiteList) : - # all is well - pass - elif isinstance(token_list[2], AssignmentExpression) : - # target specific variable assignment - # see: 6.11 Target-specific Variable Values GNU Make V.4.3 Jan2020 - raise NotImplementedError() - else: - assert 0, (type(token_list[2]),) - # Start with a default empty recipe list (so this object will always # have a RecipeList instance) self.recipe_list = RecipeList([]) @@ -489,7 +479,20 @@ def __init__(self, token_list): self.targets = self.token_list[0] self.rule_op = self.token_list[1] - self.prereqs = self.token_list[2] + self.prereqs = [] + + self.assignment = None + + if isinstance(token_list[2], PrerequisiteList) : + # all is well + self.prereqs = self.token_list[2] + elif isinstance(token_list[2], AssignmentExpression) : + # target specific variable assignment + # see: 6.11 Target-specific Variable Values GNU Make V.4.3 Jan2020 + self.assignment = token_list[2] + else: + assert 0, (type(token_list[2]),) + def makefile(self): # rule-targets rule-op prereq-list @@ -509,7 +512,12 @@ def makefile(self): s += self.rule_op.makefile() # prerequisite(s) - s += self.prereqs.makefile() + "\n" + if self.prereqs: + s += self.prereqs.makefile() + elif self.assignment: + s += self.assignment.makefile() + +# s += "\n" # recipe(s) s += self.recipe_list.makefile() @@ -517,6 +525,8 @@ def makefile(self): def add_recipe( self, recipe ) : assert isinstance(recipe, Recipe) + # assignment Rule Expressions cannot have recipes or it's a parse error + assert self.assignment is None logger.debug("add_recipe() rule=%s", str(self)) self.recipe_list.append(recipe) @@ -537,11 +547,14 @@ def eval(self, symbol_table): # time. targets = self.targets.eval(symbol_table).split() - prereqs = self.prereqs.eval(symbol_table).split() - # throw away empty strings targets = [t for t in targets if t] - prereqs = [p for p in prereqs if p] + + prereqs = [] + if self.prereqs: + prereqs = self.prereqs.eval(symbol_table).split() + # throw away empty strings + prereqs = [p for p in prereqs if p] return targets, prereqs @@ -614,6 +627,8 @@ def eval(self, symbol_table): # e.g., "echo $$PATH" becomes "echo $PATH" sent to the shell return s.replace("$$","$") + def makefile(self): + return "\t" + super().makefile() class RecipeList( Expression ) : # A collection of Recipe objects @@ -630,8 +645,7 @@ def makefile(self): # newline separated, tab prefixed s = "" if len(self.token_list): - s = "\t" + "\n\t".join([t.makefile() for t in self.token_list]) - s += "\n" + s = "\n".join([t.makefile() for t in self.token_list]) return s class Directive(Symbol): diff --git a/pymake/symtable.py b/pymake/symtable.py index fdbef59..e4be4f3 100644 --- a/pymake/symtable.py +++ b/pymake/symtable.py @@ -10,7 +10,6 @@ import pymake.version as version import pymake.constants as constants -from pymake.symbol import Symbol from pymake.error import * logger = logging.getLogger("pymake.symtable") @@ -20,6 +19,17 @@ #_fail_on_undefined = True _fail_on_undefined = False +def _value_is_recursive(v): + # test if a value is something with its own eval method which would + # indicate it's a recursive variable + # for example: + # FOO=foo <-- stored as a Symbol + # FOO:=foo <-- stored as a Python string (no eval method) + try: + return True if v.eval else False + except AttributeError: + return False + class Export(Enum): # by default, var not exported NOEXPORT = 0 @@ -99,7 +109,7 @@ def eval(self, symbol_table): # e.g., a=10 (evaluated whenever $a is used) # vs a:=10 (evaluated immediately and "10" stored in symtable) # - if isinstance(self._value, Symbol): + if _value_is_recursive(self._value): logger.debug("recursive eval %r loop=%d name=%s at pos=%r", self, self.loop, self.name, self.get_pos()) if self.loop > 0: msg = "Recursive variable %r references itself (eventually)." % self.name @@ -123,8 +133,8 @@ def eval(self, symbol_table): return self._value def append_recursive(self, value): - # ha ha type checking - assert isinstance(value,Symbol), type(value) + # ha ha type checking; require a Symbol-ish thing + assert _value_is_recursive(value), type(value) return self._appends.append(value) @@ -227,11 +237,10 @@ def __init__(self, name, value, pos): assert name in constants.builtin_variables, name super().__init__(name, value, pos) - class SymbolTable(object): def __init__(self, **kwargs): # key: variable name - # value: _entry_template dict instance + # value: Entry instance self.symbols = {} # push/pop a name/value so $(foreach) (and other functions) can re-use @@ -446,18 +455,16 @@ def fetch(self, key, pos=None): return "" def append(self, name, value, pos=None): - # "When the variable in question has not been defined before, ‘+=’ acts # just like normal ‘=’: it defines a recursively-expanded variable." # GNU Make 4.3 January 2020 if name not in self.symbols: # ha ha type checking - assert isinstance(value,Symbol), type(value) - + assert _value_is_recursive(value), type(value) return self.add(name, value, pos) entry = self.symbols[name] - if isinstance(entry.value, Symbol): + if _value_is_recursive(entry.value): return entry.append_recursive(value) # simple string append, space separated diff --git a/tests/target-specific.mk b/tests/target-specific.mk index 434efff..e360d30 100644 --- a/tests/target-specific.mk +++ b/tests/target-specific.mk @@ -1,124 +1,38 @@ -# Experiment with target specific rules. -# GNU Make manual 6.11 -# davep 19-Sep-2014 - -CC=gcc -LD=ld - -# avoid the shell from interpretting my test chars -SHELL=/bin/echo - -# SHELLFLAGS is feature new in 3.82 (current device running 3.81) -# Previously $(SHELL) lauched with hardcoded -c -#$(info version=$(MAKE_VERSION)) -.SHELLFLAGS= - -all : normal specific other multiple spaces rules more-spaces confused vbar\ - foovbar whitebar semicolon backslash 2backslash backslash-? \ - 2backslash-? colon - -# make -f target-specific.mk normal -normal : - @! normal CC=gcc LD=ld!$@ CC=$(CC) LD=$(LD) - -# make -f target-specific.mk specific -specific : CC=mycc -specific : LD=myld -specific : - @! specific CC=mycc LD=myld!$@ CC=$(CC) LD=$(LD) - -# make -f target-specific.mk other -other : CC:=$(CC:gcc=egcs) -other : - @! other CC=egcs LD=ld!$@ CC=$(CC) LD=$(LD) - -# multiple assignments ; only the first '=' is significant -# this line creates a string $(CC) == intel-cc LD=intel-ld -# make -f target-specific.mk multiple -multiple: CC=intel-cc LD=intel-ld -multiple: - @! multiple CC=intel-cc LD=intel-ld LD=ld!$@ CC=$(CC) LD=$(LD) - -# double reference -# make -f target-specific.mk spaces -this is a test=CC -spaces: $(this is a test)=this is a test -spaces: - @! spaces CC=this is a test LD=ld CC!$@ CC=$(CC) LD=$(LD) $(this is a test) - -# Anything after the assignment operator is treated as one token. The CC below creates one big string -# from potato-cc to EOL. CROSS_COMPILE is empty -FOO=BAR -rules: CC=potato-cc LD=potato-ld spaces CROSS_COMPILE=potato $(FOO) potato -rules: - @! rules CC=@@potato-cc LD=potato-ld spaces CROSS_COMPILE=potato BAR potato@@ LD=@@ld@@ CROSS_COMPILE= TEST=CC!$@ CC=@@$(CC)@@ LD=@@$(LD)@@ CROSS_COMPILE=$(CROSS_COMPILE) TEST=$(this is a test) - -# Leading spaces are discarded. Trailing spaces are preserved. -# Comment is successfully ignored. -# make -f target-specific.mk more-spaces -more-spaces : CC = single-trailing-space-cc -more-spaces : LD = lots-of-trailing-spaces-ld # I am a comment -more-spaces: - @% more-spaces CC=@@single-trailing-space-cc @@ LD=@@lots-of-trailing-spaces-ld @@ TEST=CC%$@ CC=@@$(CC)@@ LD=@@$(LD)@@ TEST=$(this is a test) - -# who wins? another ":" indicates an implicit pattern rule. "|" indicates -# order-only prerequisite -# make -f target-specific.mk confused -confused: COLON=: -confused: - @= :=$(COLON) - -# note the single quotes to prevent shell from interpretting the | -# make -f target-specific.mk vbar -vbar: BAR=| -vbar: - @= |=$(BAR) - -# anything before the | cause confusion? -# make -f target-specific.mk foovbar -foovbar: BAR=foo| -foovbar: - @= foo|=$(BAR) - -# whitespaces? -# make -f target-specific.mk whitebar -whitebar: BAR=foo | -whitebar: - @= foo |=$(BAR) - -# So where does semicolon fit in? Normally would terminate the prerequisites. -# The ";" still part of assignment's RHS -semicolon: CC=abcc;echo semicolon > /dev/null -semicolon: - @= @@abcc;echo semicolon > /dev/null@@=@@$(CC)@@ - -# Does backspace protect the ";"? Nope. The \ seems to disappear. -# The target-specific rule's recipe happens after the plain rule's recipe. -# make -f target-specific.mk backslash -backslash: CC=abcdcc\; echo semicolon > /dev/null -backslash: - @= abcdcc; echo semicolon > /dev/null=$(CC) - -# two backslashes protect the semicolon (wha?) -# Note the \ in the recipe protects the ; from the shell. -# make -f target-specific.mk 2backslash -2backslash: CC=abcc\\; echo semicolon -2backslash: - @= abcc\; echo semicolon=$(CC) - -# double trouble- weird char in the target -# make -f target-specific.mk backslash-? -backslash-? : CC=whocc\? -backslash-? : - @= whocc\?=$(CC) - -# again, the two backspaces translate to a literal \ -# make -f target-specific.mk 2backslash-? -2backslash-? : CC=xyzcc\\? -2backslash-? : - @= xyzcc\\?=$(CC) - -colon : CC:=: -colon : - @= :=$(CC) +# test target specific assignment +CC:=gcc +CFLAGS:=-Wall +export CC FOO CFLAGS + +$(info $(shell echo $$CC)) +$(info $(shell printenv CC)) + +BAZ:=baz + +all:CC:=clang +all:FOO!=echo foo +all:CFLAGS+=-g +all:PREREQ:=a.txt + +# PREREQ isn't eval'd so 'all' has no prereqs (the var only applies to the +# recipe apparently) +all: $(PREREQ) + @echo BAR=$(BAR) + @printenv CC + @printenv FOO + @printenv CFLAGS + @echo PREREQ=$(PREREQ) +all:BAR=$(BAZ) + +BAZ:=zab + +other:CFLAGS:=-O3 +other: + @echo CC=$${CC} + @echo FOO=$${FOO} + @echo CFLAGS=$${CFLAGS} + +other: + @echo CC=$${CC} + @echo FOO=$${FOO} + @echo CFLAGS=$${CFLAGS} diff --git a/tests/test_tokenizer.py b/tests/test_tokenizer.py index 2af498a..271f51b 100644 --- a/tests/test_tokenizer.py +++ b/tests/test_tokenizer.py @@ -84,13 +84,13 @@ def test_tokenize_simple_recipe(): s = " @echo foo\n" viter = make_viter(s) r = tokenizer.tokenize_recipe(viter) - assert r.makefile() == "@echo foo" + assert r.makefile() == "\t@echo foo" def test_tokenize_simple_whitespace_recipe(): s = " @echo foo\n" viter = make_viter(s) r = tokenizer.tokenize_recipe(viter) - assert r.makefile() == "@echo foo" + assert r.makefile() == "\t@echo foo" def test_tokenize_varref_recipe(): s = " @echo $(foo)\n" @@ -101,7 +101,7 @@ def test_tokenize_varref_recipe(): symbol_table.add("foo", "bar") s = r.eval(symbol_table) - assert r.makefile() == "@echo $(foo)" + assert r.makefile() == "\t@echo $(foo)" assert s == "@echo bar" def test_tokenize_single_letter_varref_recipe(): @@ -115,7 +115,7 @@ def test_tokenize_single_letter_varref_recipe(): s = r.eval(symbol_table) # my round trip code will always add () around varrefs - assert r.makefile() == "@echo $(f)" + assert r.makefile() == "\t@echo $(f)" assert s == "@echo bar" def test_recipe_with_backslash(): @@ -137,7 +137,7 @@ def test_tokenize_bash_var_recipe(): symbol_table = symtable.SymbolTable() s = r.eval(symbol_table) - assert r.makefile() == "@echo $$PATH" + assert r.makefile() == "\t@echo $$PATH" assert s == "@echo $PATH" def test_tokenize_double_dollar_recipe(): @@ -151,7 +151,7 @@ def test_tokenize_double_dollar_recipe(): symbol_table = symtable.SymbolTable() s = r.eval(symbol_table) - assert r.makefile() == "@echo pid=$$$$" + assert r.makefile() == "\t@echo pid=$$$$" assert s == "@echo pid=$$" # 5.1.1 Splitting Recipe Lines From 650614033a64f0dafb2d12b2801490747621ad84 Mon Sep 17 00:00:00 2001 From: David Poole Date: Mon, 13 Jan 2025 10:49:30 -0800 Subject: [PATCH 2/4] add "layers" to symbol table In order to support rule-specific variables, let's add a 'layer' scheme to the symbol table. A "layer" is another dict with key:value as name:Entry. The layers are stored in an array, with [0] as the topmost layer. Variable lookups search starting at [0]. Variables only added to layer [0]. Removed the old push/pop variable code. Need to add test code. --- pymake/functions.py | 19 ++-- pymake/pymake.py | 12 +-- pymake/symtable.py | 196 +++++++++++++++++++++++------------------ tests/test_symtable.py | 25 ++++-- 4 files changed, 139 insertions(+), 113 deletions(-) diff --git a/pymake/functions.py b/pymake/functions.py index 3d127da..63fb4b6 100644 --- a/pymake/functions.py +++ b/pymake/functions.py @@ -121,27 +121,21 @@ class Call(FunctionWithArguments): def eval(self, symbol_table): var = "".join([a.eval(symbol_table) for a in self.args[0]]) + symbol_table.push_layer() + arg_stack = [] for idx, arg_list in enumerate(self.args[1:]): arg = "".join([a.eval(symbol_table) for a in arg_list]) varname = "%d" % (idx+1) - symbol_table.push(varname) symbol_table.add(varname, arg) # save the arg name so we can pop it from the symbol table arg_stack.append(varname) - # all we need to do is fetch() from the symbol table and the expression # will be eval'd s = symbol_table.fetch(var) -# breakpoint() - # pop the args off the symbol table in reverse order because why not - while 1: - try: - symbol_table.pop(arg_stack.pop()) - except IndexError: - break + symbol_table.pop_layer() return s @@ -183,16 +177,15 @@ def eval(self, symbol_table): # array of strings list_ = "".join([a.eval(symbol_table) for a in self.args[1]]).split() -# breakpoint() - out_str_list = [] - symbol_table.push(var) + symbol_table.push_layer() + for item in list_: symbol_table.add(var, item, self.args[0][0].get_pos()) text = "".join([a.eval(symbol_table) for a in self.args[2]]) out_str_list.append(text) - symbol_table.pop(var) + symbol_table.pop_layer() return " ".join(out_str_list) diff --git a/pymake/pymake.py b/pymake/pymake.py index 6e220c3..19b2156 100644 --- a/pymake/pymake.py +++ b/pymake/pymake.py @@ -451,22 +451,18 @@ def check_prefixes(s): return s, ignore_failure, silent # TODO many more automatic variables - symtable.push("@") - symtable.push("^") - symtable.push("+") - symtable.push("<") + symtable.push_layer() symtable.add_automatic("@", rule.target, recipe.get_pos()) symtable.add_automatic("^", " ".join(remove_duplicates(rule.prereq_list)), rule.get_pos()) symtable.add_automatic("+", " ".join(rule.prereq_list), rule.get_pos()) symtable.add_automatic("<", rule.prereq_list[0] if len(rule.prereq_list) else "", rule.get_pos()) + # TODO target specific variables + cmd_s = recipe.eval(symtable) # print("execute_recipe \"%r\"" % cmd_s) - symtable.pop("@") - symtable.pop("^") - symtable.pop("+") - symtable.pop("<") + symtable.pop_layer() # Defining Multi-Line Variables. # "However, note that using two separate lines means make will invoke the shell twice, running diff --git a/pymake/symtable.py b/pymake/symtable.py index e4be4f3..501adcf 100644 --- a/pymake/symtable.py +++ b/pymake/symtable.py @@ -239,9 +239,18 @@ def __init__(self, name, value, pos): class SymbolTable(object): def __init__(self, **kwargs): + # Stack of dictionary to support "hiding" variable definitions. For + # example, foreach loop variables and target specific variables must + # not change existing definitions + # + # The topmost layer will be [0] + # New layers are added using .insert(0) and removed using [1:] + # + # In some places, I skip search layers if the len()==1 + # # key: variable name # value: Entry instance - self.symbols = {} + self.layers = [{},] # push/pop a name/value so $(foreach) (and other functions) can re-use # the var name (and we don't have to make a complete new copy of a @@ -306,12 +315,55 @@ def _init_envvars(self): # GNU Make Manual Version 4.3 January 2020 [ self._add_entry(EnvVarEntry(k,v)) for k,v in os.environ.items() if k != "SHELL" ] + def push_layer(self): + # push top, pop top + self.layers.insert(0, {}) + + def pop_layer(self): + # push top, pop top + self.layers = self.layers[1:] + + def find(self, name, layer_idx=None): + # search the layers for a variable; throw KeyError on failure + if len(self.layers)==1: + return self.layers[0][name] + + if layer_idx is not None: + # only search a specific layer + return self.layers[layer_idx][name] + + # search layers from top to bottom (forward iterator) + for symbols in self.layers: + try: + return symbols[name] + except KeyError: + pass + raise KeyError(name) + + def _get_entries(self, filter_fn=None): + # Build a dict name:entry of entire symbol table contents taking layers + # into account. Created originally to fetch all exported vars and to + # handle global 'export'/'unexport' directives. + entries = {} + + # walk layers bottom to top so higher entries replace lower + # (reverse iterator) + if filter_fn: + for symbols in self.layers[::-1]: + entries.update({ name:entry for name,entry in symbols.items() if filter_fn(entry) }) + else: + for symbols in self.layers[::-1]: + entries.update({ name:entry for name,entry in symbols.items() }) + + return entries + def _add_entry(self, entry): # sanity check the entry fields entry.sanity() - self.symbols[entry.name] = entry + # always add to the top layer + self.layers[0][entry.name] = entry def add(self, name, value, pos=None): logger.debug("store \"%s\"=\"%s\"", name, value) @@ -330,8 +382,10 @@ def add(self, name, value, pos=None): if name in constants.builtin_variables: warning_message(pos, "overwriting built-in variable \"%s\"" % name) + # only search top layer; if not there, we'll add it to mask the + # layer(s) below try: - entry = self.symbols[name] + entry = self.find(name, 0) except KeyError: entry = None @@ -384,7 +438,7 @@ def maybe_add(self, name, value, pos=None): # If name already exists in the table, don't overwrite. # Used with ?= assignments. try: - self.symbols[name] + self.find(name) return except KeyError: pass @@ -444,7 +498,7 @@ def fetch(self, key, pos=None): # pass try: - return self.symbols[key].eval(self) + return self.find(key).eval(self) except KeyError: if self.warn_undefined: warning_message(pos, "undefined variable '%s'" % key) @@ -458,12 +512,16 @@ def append(self, name, value, pos=None): # "When the variable in question has not been defined before, ‘+=’ acts # just like normal ‘=’: it defines a recursively-expanded variable." # GNU Make 4.3 January 2020 - if name not in self.symbols: - # ha ha type checking + + entry = None + try: + entry = self.find(name) + except KeyError: + pass + if entry is None: assert _value_is_recursive(value), type(value) return self.add(name, value, pos) - - entry = self.symbols[name] + if _value_is_recursive(entry.value): return entry.append_recursive(value) @@ -473,60 +531,6 @@ def append(self, name, value, pos=None): except AttributeError: entry.append(value, pos) - def push(self, name): - # save current value of 'name' in secure, undisclosed location - - logger.debug("push name=%s", name) - - # don't use self.fetch() because will eval the var which could lead to - # side effects - if name not in self.symbols: - # nothing to save - return - - entry = self.symbols[name] - # remove the other reference (otherwise 'add' will just update the - # self.symbols[name] entry which also points to our stack entry) - del self.symbols[name] - - # create the dequeue if doesn't exist - if not name in self.stack: - self.stack[name] = collections.deque() - - # push right, pop right (stack) - logger.debug("push entry=%s", entry.name) - self.stack[name].append(entry) - - def pop(self, name): - # restore previous value of 'name' from the secure, undisclosed location - - # push right, pop right (stack) - # allow KeyError and IndexError to propagate (indicates a bug in the - # calling code) - logger.debug("pop name=%s", name) - - # The stack will contain values previously in the symbol table. - # If there was no previous value in the symbol table, there will be no - # entry for it in the stack. In this case, just delete the value from - # the symbol table. - # For example, the $(call) function will push each arg $1 $2 $3 $4 etc - # then pop them after the call. Very likely $1 $2 $3, etc, are not in - # the symbol table originally. - try: - entry = self.stack[name].pop() - except KeyError: - # if the name doesn't exist in the self.symbols{}, we push/popped a - # name but didn't use it between the push/pop (perfectly acceptable) - if name in self.symbols: - del self.symbols[name] - return - - # TODO future memory optimization would be to delete the dequeue from - # self.stack when empty - - # restore previous value - self.symbols[name] = entry - def flavor(self, name): # Support for the $(flavor) function # @@ -537,7 +541,7 @@ def flavor(self, name): # don't use self.fetch() because will eval the var which could lead to # side effects try : - value = self.symbols[name] + value = self.find(name) except KeyError: return "undefined" @@ -566,7 +570,7 @@ def origin(self, name): # automatic -- defined in a rule e.g., $@ TODO try : - entry = self.symbols[name] + entry = self.find(name) assert entry.origin is not None, name return entry.origin except KeyError: @@ -579,7 +583,7 @@ def value(self, name): # side effects try : - entry = self.symbols[name] + entry = self.find(name) value = entry.value if isinstance(value,Symbol): return value.makefile() @@ -589,21 +593,32 @@ def value(self, name): def variables(self, _): # return $(.VARIABLES) - return " ".join(self.symbols.keys()) + # walk the layers, bottom to top, gathering the key strings + + if len(self.layers)==1: + return " ".join(self.layers[0].keys()) + raise NotImplementedError("variables") +# return " ".join(self.symbols.keys()) def is_defined(self, name): # is this varname in our symbol table (or other mechanisms) - return name in self.symbols + try: + _ = self.find(name) + return True + except KeyError: + return False def ifdef(self, name): - if not name in self.symbols: + entry = None + try: + entry = self.find(name) + except KeyError: return False # it's in our table but does it have a value? - value = self.symbols[name] - assert value._value is not None + assert entry._value is not None - if not len(value._value): + if not len(entry._value): return False return True @@ -615,7 +630,7 @@ def export(self, name=None): return try: - value = self.symbols[name].set_export(Export.EXPLICIT) + value = self.find(name).set_export(Export.EXPLICIT) except KeyError: # no such entry pass @@ -627,32 +642,42 @@ def unexport(self, name=None): return try: - self.symbols[name].set_unexport(Export.EXPLICIT) + self.find(name).set_unexport(Export.EXPLICIT) except KeyError: # no such entry pass def undefine(self, name): # support the undefine directive - try: - entry = self.symbols[name] - except KeyError: - # does not exist. no harm, no foul. - return + if len(self.layers)==1: + try: + del self.layers[0][name] + except KeyError: + # does not exist. no harm, no foul. + return # TODO any variables that GNU Make considers an error to undefine? - del self.symbols[name] + # Try in layer order. If we don't find it in a layer, we're done (leave + # lower layers intact) + for symbols in self.layers: + try: + del symbols[name] + except KeyError: + return def _export_all(self): - for k,v in self.symbols.items(): + entries = self._get_entries() + for k,v in entries.items(): v.set_export( Export.IMPLICIT ) # new vars from this point on will be marked as export self.export_start() def _unexport_all(self): - for k,v in self.symbols.items(): + entries = self._get_entries() + for k,v in entries.items(): v.set_unexport(Export.IMPLICIT) + # turn off global export flag self.export_stop() def get_exports(self): @@ -663,8 +688,9 @@ def get_exports(self): # store something in the symbol table that will throw a "dictionary # changed during iteration" error (for example, .SHELLSTATUS is updated # internally every time a shell is run) - exports = { name:entry for name,entry in self.symbols.items() if entry.export } - exports = { name:entry.eval(self) for name,entry in exports.items() } + entries = self._get_entries(lambda e:e.export ) + + exports = { name:entry.eval(self) for name,entry in entries.items() } assert self.env_recursion >= 0 return exports @@ -706,7 +732,7 @@ def update_builtin(self, name, value, pos=None): assert name in constants.builtin_variables, name try: - return self.symbols[name].set_value(value, pos) + return self.find(name).set_value(value, pos) except KeyError: pass diff --git a/tests/test_symtable.py b/tests/test_symtable.py index ce4e740..a8d9ddb 100644 --- a/tests/test_symtable.py +++ b/tests/test_symtable.py @@ -3,6 +3,8 @@ # import logging +import pytest + logger = logging.getLogger("pymake") logging.basicConfig(level=logging.DEBUG) @@ -30,6 +32,7 @@ def test_recursively_expanded(): value = symbol_table.fetch("CFLAGS") assert value=="-g -Wall", value +@pytest.mark.skip(reason="FIXME push/pop") def test_simple_push_pop(): symbol_table = symtable.SymbolTable() symbol_table.add("target", ["abcdefghijklmnopqrstuvwxyz"]) @@ -43,6 +46,7 @@ def test_simple_push_pop(): value = symbol_table.fetch("target") assert value == ["abcdefghijklmnopqrstuvwxyz"] +@pytest.mark.skip(reason="FIXME push/pop") def test_push_push_pop_pop(): symbol_table = symtable.SymbolTable() symbol_table.add("target", ["abcdefghijklmnopqrstuvwxyz"]) @@ -64,6 +68,7 @@ def test_push_push_pop_pop(): value = symbol_table.fetch("target") assert value == ["abcdefghijklmnopqrstuvwxyz"] +@pytest.mark.skip(reason="FIXME push/pop") def test_push_pop_undefined(): # "If var was undefined before the foreach function call, it is undefined after the call." symbol_table = symtable.SymbolTable() @@ -77,6 +82,7 @@ def test_push_pop_undefined(): value = symbol_table.fetch("target") assert value=="" +@pytest.mark.skip(reason="FIXME push/pop") def test_push_pop_pop(): # too many pops symbol_table = symtable.SymbolTable() @@ -99,6 +105,7 @@ def test_push_pop_pop(): # expected IndexError assert 0 +@pytest.mark.skip(reason="FIXME push/pop") def test_env_var(): # environment variables should act like regular vars symbol_table = symtable.SymbolTable() @@ -232,14 +239,15 @@ def test_update(): symbol_table = symtable.SymbolTable() # CFLAGS=-g -Wall symbol_table.add("FOO", Literal("foo")) - value = symbol_table.fetch("FOO") - assert value == "foo" - id1 = id(symbol_table.symbols["FOO"]) + entry = symbol_table.find("FOO") + assert entry._value == "foo" + id1 = id(entry) + + # update value, verify we're still the same entry symbol_table.add("FOO", Literal("oof")) - value = symbol_table.fetch("FOO") - assert value == "oof" - id2 = id(symbol_table.symbols["FOO"]) - assert id1==id2 + entry = symbol_table.find("FOO") + assert entry._value == "oof" + assert id1==id(entry) def test_command_line(): # variable from command line, for example: @@ -277,3 +285,6 @@ def test_command_line_multiple_var(): assert value == "baz" assert symbol_table.origin("FOO") == "command line" +def test_layers(): + pass + From 6e6d80e8dabb04ec4a190501e70a2ca80c80707e Mon Sep 17 00:00:00 2001 From: David Poole Date: Thu, 16 Jan 2025 17:13:55 -0800 Subject: [PATCH 3/4] fixes to target specific variables - add a layer for recipe execution - fix bugs found in new tests - fix export of an undefined variable --- pymake/pymake.py | 14 +++-- pymake/symtable.py | 46 +++++++++++--- tests/target-specific.mk | 4 +- tests/test_export.py | 14 +++++ tests/test_makefile.py | 1 + tests/test_symtable.py | 127 ++++++++++++++++++++++++++++----------- 6 files changed, 155 insertions(+), 51 deletions(-) diff --git a/pymake/pymake.py b/pymake/pymake.py index 19b2156..c9b85b0 100644 --- a/pymake/pymake.py +++ b/pymake/pymake.py @@ -457,13 +457,9 @@ def check_prefixes(s): symtable.add_automatic("+", " ".join(rule.prereq_list), rule.get_pos()) symtable.add_automatic("<", rule.prereq_list[0] if len(rule.prereq_list) else "", rule.get_pos()) - # TODO target specific variables - cmd_s = recipe.eval(symtable) # print("execute_recipe \"%r\"" % cmd_s) - symtable.pop_layer() - # Defining Multi-Line Variables. # "However, note that using two separate lines means make will invoke the shell twice, running # an independent sub-shell for each line. See Section 5.3 [Recipe Execution], page 46." @@ -529,6 +525,8 @@ def check_prefixes(s): break exit_code = 0 + symtable.pop_layer() + return exit_status["error"] if exit_code else exit_status["success"] def execute(makefile, args): @@ -657,10 +655,18 @@ def execute(makefile, args): # this warning catches where I fail to find an implicit rule logger.warning("I didn't find a recipe to build target=\"%s\"", target) + # target specific variables + if rule.assignment_list: + symtable.push_layer() + for asn in rule.assignment_list: + asn.eval(symtable) + for recipe in rule.recipe_list: exit_code = execute_recipe(rule, recipe, symtable, args) if exit_code != 0: break + if rule.assignment_list: + symtable.pop_layer() if exit_code != 0: break diff --git a/pymake/symtable.py b/pymake/symtable.py index 501adcf..260b1d1 100644 --- a/pymake/symtable.py +++ b/pymake/symtable.py @@ -259,6 +259,11 @@ def __init__(self, **kwargs): self.export_default_value = Export.NOEXPORT + # A name can be marked export before it's been defined. For example, + # export FOO + # FOO:=bar + self.exported_names = set() + self.warn_undefined = kwargs.get("warn_undefined_variables", False) self._init_builtins() @@ -321,6 +326,9 @@ def push_layer(self): def pop_layer(self): # push top, pop top + if len(self.layers)==1: + # don't allow pop of last layer + raise IndexError(1) self.layers = self.layers[1:] def find(self, name, layer_idx=None): @@ -404,7 +412,12 @@ def add(self, name, value, pos=None): entry = DefaultEntry(name, value, pos) else: entry = FileEntry(name, value, pos) - entry.set_export(self.export_default_value) + + # check if the name was exported before a variable was defined + if name in self.exported_names: + entry.set_export(Export.EXPLICIT) + else: + entry.set_export(self.export_default_value) self._add_entry(entry) else: @@ -597,8 +610,9 @@ def variables(self, _): if len(self.layers)==1: return " ".join(self.layers[0].keys()) - raise NotImplementedError("variables") -# return " ".join(self.symbols.keys()) + + entries = self._get_entries() + return " ".join(entries.keys()) def is_defined(self, name): # is this varname in our symbol table (or other mechanisms) @@ -630,11 +644,13 @@ def export(self, name=None): return try: - value = self.find(name).set_export(Export.EXPLICIT) + self.find(name).set_export(Export.EXPLICIT) except KeyError: # no such entry pass + self.exported_names.update((name,)) + def unexport(self, name=None): if name is None: # export everything @@ -647,6 +663,12 @@ def unexport(self, name=None): # no such entry pass + try: + self.exported_names.remove(name) + except KeyError: + # no such entry, no big deal + pass + def undefine(self, name): # support the undefine directive if len(self.layers)==1: @@ -681,14 +703,20 @@ def _unexport_all(self): self.export_stop() def get_exports(self): + # Fetch all the exported variables and their values. + # Used when launching shell commands so need to eval() the expressions + # to get their actual value. + logger.debug("get_exports") assert self.env_recursion >= 0 - # cannot eval() during the self.symbols iteration because we could - # store something in the symbol table that will throw a "dictionary - # changed during iteration" error (for example, .SHELLSTATUS is updated - # internally every time a shell is run) - entries = self._get_entries(lambda e:e.export ) + # Separate key:value fetch from .eval() because we could store + # something in the symbol table during eval. For example, .SHELLSTATUS + # is updated internally every time a shell is run. + # + # The .eval() during iteration will throw a "dictionary changed during + # iteration" error. + entries = self._get_entries(lambda e:e.export) exports = { name:entry.eval(self) for name,entry in entries.items() } diff --git a/tests/target-specific.mk b/tests/target-specific.mk index e360d30..3c8ac54 100644 --- a/tests/target-specific.mk +++ b/tests/target-specific.mk @@ -11,7 +11,7 @@ BAZ:=baz all:CC:=clang all:FOO!=echo foo all:CFLAGS+=-g -all:PREREQ:=a.txt +all:export PREREQ:=a.txt # PREREQ isn't eval'd so 'all' has no prereqs (the var only applies to the # recipe apparently) @@ -20,7 +20,7 @@ all: $(PREREQ) @printenv CC @printenv FOO @printenv CFLAGS - @echo PREREQ=$(PREREQ) + @printenv PREREQ all:BAR=$(BAZ) BAZ:=zab diff --git a/tests/test_export.py b/tests/test_export.py index 4a8e238..1515085 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -295,4 +295,18 @@ def test_export_commandline_var_explicit(): # use explicity assign instead of recursive assign run_test(makefile, expect, extra_args=("FOO:=bar",)) +def test_export_before_define(): + makefile=""" +export FOO +FOO:=foo +@:; printenv FOO +""" + run.simple_test(makefile) + +def test_unexpert_unknown(): + makefile=""" +unexport FOO +@:;@: +""" + run.simple_test(makefile) diff --git a/tests/test_makefile.py b/tests/test_makefile.py index 299b141..b6ddcea 100644 --- a/tests/test_makefile.py +++ b/tests/test_makefile.py @@ -113,6 +113,7 @@ def _run_pymake(infilename): "ignoreandsilent.mk", "rule_without_target.mk", "shellstatus.mk", + "target-specific.mk", # FIXME fails with GNU Make 4.3 # "env_recursion.mk", diff --git a/tests/test_symtable.py b/tests/test_symtable.py index a8d9ddb..a1c9683 100644 --- a/tests/test_symtable.py +++ b/tests/test_symtable.py @@ -32,80 +32,75 @@ def test_recursively_expanded(): value = symbol_table.fetch("CFLAGS") assert value=="-g -Wall", value -@pytest.mark.skip(reason="FIXME push/pop") -def test_simple_push_pop(): +def test_simple_layer(): symbol_table = symtable.SymbolTable() - symbol_table.add("target", ["abcdefghijklmnopqrstuvwxyz"]) + symbol_table.add("target", "abcdefghijklmnopqrstuvwxyz") - symbol_table.push("target") - symbol_table.add("target", ["12345"]) + symbol_table.push_layer() + symbol_table.add("target", "12345") value = symbol_table.fetch("target") - assert value == ["12345"] + assert value == "12345" - symbol_table.pop("target") + symbol_table.pop_layer() value = symbol_table.fetch("target") - assert value == ["abcdefghijklmnopqrstuvwxyz"] + assert value == "abcdefghijklmnopqrstuvwxyz" -@pytest.mark.skip(reason="FIXME push/pop") def test_push_push_pop_pop(): symbol_table = symtable.SymbolTable() - symbol_table.add("target", ["abcdefghijklmnopqrstuvwxyz"]) + symbol_table.add("target", "abcdefghijklmnopqrstuvwxyz") - symbol_table.push("target") - symbol_table.add("target", ["12345"]) + symbol_table.push_layer() + symbol_table.add("target", "12345") - symbol_table.push("target") - symbol_table.add("target", ["67890"]) + symbol_table.push_layer() + symbol_table.add("target", "67890") value = symbol_table.fetch("target") - assert value == ["67890"] + assert value == "67890" - symbol_table.pop("target") + symbol_table.pop_layer() value = symbol_table.fetch("target") - assert value == ["12345"] + assert value == "12345" - symbol_table.pop("target") + symbol_table.pop_layer() value = symbol_table.fetch("target") - assert value == ["abcdefghijklmnopqrstuvwxyz"] + assert value == "abcdefghijklmnopqrstuvwxyz" -@pytest.mark.skip(reason="FIXME push/pop") def test_push_pop_undefined(): # "If var was undefined before the foreach function call, it is undefined after the call." symbol_table = symtable.SymbolTable() - symbol_table.push("target") - symbol_table.add("target", ["12345"]) + symbol_table.push_layer() + symbol_table.add("target", "12345") value = symbol_table.fetch("target") - assert value == ["12345"] + assert value == "12345" - symbol_table.pop("target") + symbol_table.pop_layer() value = symbol_table.fetch("target") assert value=="" -@pytest.mark.skip(reason="FIXME push/pop") def test_push_pop_pop(): # too many pops symbol_table = symtable.SymbolTable() - symbol_table.add("target", ["abcdefghijklmnopqrstuvwxyz"]) + symbol_table.add("target", "abcdefghijklmnopqrstuvwxyz") - symbol_table.push("target") - symbol_table.add("target", ["12345"]) + symbol_table.push_layer() + symbol_table.add("target", "12345") value = symbol_table.fetch("target") - assert value == ["12345"] + assert value == "12345" - symbol_table.pop("target") + symbol_table.pop_layer() value = symbol_table.fetch("target") - assert value == ["abcdefghijklmnopqrstuvwxyz"] + assert value == "abcdefghijklmnopqrstuvwxyz" try: - symbol_table.pop("target") + symbol_table.pop_layer() except IndexError: pass else: # expected IndexError assert 0 -@pytest.mark.skip(reason="FIXME push/pop") def test_env_var(): # environment variables should act like regular vars symbol_table = symtable.SymbolTable() @@ -115,12 +110,12 @@ def test_env_var(): assert save_path assert symbol_table.origin("PATH") == "environment" - symbol_table.push("PATH") + symbol_table.push_layer() symbol_table.add("PATH", "a:b:c:") value = symbol_table.fetch("PATH") assert value == "a:b:c:" - symbol_table.pop("PATH") + symbol_table.pop_layer() path = symbol_table.fetch("PATH") assert path==save_path @@ -144,6 +139,13 @@ def test_is_defined(): symbol_table.add("FOO", "BAR") assert symbol_table.is_defined("FOO") + assert not symbol_table.is_defined("DAVE") + symbol_table.push_layer() + symbol_table.add("DAVE","dave") + assert symbol_table.is_defined("DAVE") + symbol_table.pop_layer() + assert not symbol_table.is_defined("DAVE") + def test_maybe_add(): symbol_table = symtable.SymbolTable() @@ -286,5 +288,58 @@ def test_command_line_multiple_var(): assert symbol_table.origin("FOO") == "command line" def test_layers(): - pass + symbol_table = symtable.SymbolTable() + symbol_table.add("FOO", Literal("foo")) + symbol_table.push_layer() + value = symbol_table.fetch("FOO") + assert value == "foo" # unchanged + +def test_layers_variables(): + symbol_table = symtable.SymbolTable() + + symbol_table.add("FOO", Literal("foo")) + varlist = symbol_table.variables(None) + assert "FOO" in varlist + + symbol_table.push_layer() + varlist = symbol_table.variables(None) + assert "FOO" in varlist + +@pytest.mark.skip("undefine FIXME") +def test_layers_undefine(): + symbol_table = symtable.SymbolTable() + + symbol_table.add("FOO", Literal("foo")) + symbol_table.push_layer() + symbol_table.add("FOO", Literal("bar")) + + symbol_table.undefine("FOO") + varlist = symbol_table.variables(None) + assert "FOO" not in varlist + symbol_table.pop_layer() + varlist = symbol_table.variables(None) + assert "FOO" in varlist + +@pytest.mark.skip("foo") +def test_layer_append(): + symbol_table = symtable.SymbolTable() + + symbol_table.add("FOO", Literal("foo")) + symbol_table.push_layer() + symbol_table.append("FOO", Literal("bar")) + value = symbol_table.fetch("FOO") +# assert value=="foobar" + symbol_table.pop_layer() + value = symbol_table.fetch("FOO") +# assert value=="foo" + +def test_global_export_layers(): + makefile=""" +export + +all:FOO:=bar +all: + printenv FOO +""" + run.simple_test(makefile) From 17f6bfc9ca685af7d858781f9f605041cc8e179c Mon Sep 17 00:00:00 2001 From: David Poole Date: Sat, 18 Jan 2025 06:20:10 -0800 Subject: [PATCH 4/4] turn off failing code GNU Make 4.1 (Used in Ubuntu systems in Github CI) doesn't export variables to $(shell) calls. The feature was added in GNU Make 4.4 --- tests/target-specific.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/target-specific.mk b/tests/target-specific.mk index 3c8ac54..2f197fd 100644 --- a/tests/target-specific.mk +++ b/tests/target-specific.mk @@ -3,8 +3,8 @@ CC:=gcc CFLAGS:=-Wall export CC FOO CFLAGS -$(info $(shell echo $$CC)) -$(info $(shell printenv CC)) +#$(info $(shell echo $$CC)) +#$(info $(shell printenv CC)) BAZ:=baz