diff --git a/pymake/pargs.py b/pymake/pargs.py index db91540..7d9a8dc 100644 --- a/pymake/pargs.py +++ b/pymake/pargs.py @@ -113,15 +113,16 @@ def __init__(self): def __str__(self): # useful for debugging the sub-make - s = "" - s += "-B " if self.always_make else "" - s += "-C %s " % self.directory if self.directory else "" - s += "-d " if self.debug else "" - s += "-f %s " % self.filename if self.filename else "" - s += "-n " if self.dry_run else "" - s += "-s " if self.silent else "" - s += " ".join(self.argslist) - return s + a = [ + "-B" if self.always_make else "", + "-C %s" % self.directory if self.directory else "", + "-d" if self.debug else "", + "-f %s" % self.filename if self.filename else "", + "-n" if self.dry_run else "", + "-s" if self.silent else "", + *self.argslist + ] + return " ".join( [s for s in a if s] ) def _parse_debug_flags(s): flags = s.split(',') diff --git a/pymake/parser.py b/pymake/parser.py index 288fec6..cbfb656 100644 --- a/pymake/parser.py +++ b/pymake/parser.py @@ -752,116 +752,6 @@ def parse_directive(directive_vstr, virt_line, vline_iter): return lut[str(directive_vstr)](directive_vstr, virt_line, vline_iter) -def parse_expression(expr, virt_line, vline_iter): - # davep 20241123 ; obsolete - assert 0, "TODO" - - # This is a second pass through an Expression. - # An Expression could be something like: - # $(info blah blah) # fn call ; most are invalid in standalone context - # export - # export something - # define foo # start of multi-line variable - - logger.debug("parse %s", expr.__class__.__name__) -# if get_line_number(expr) > 151: -# breakpoint() - assert isinstance(expr,Expression), type(expr) - - # If we do find a directive, we'll wind up re-parsing the entire line as a - # directive. Unnecessary and ugly but I first tried to handle directives - # before assignment and rules which didn't work (too much confusion in - # handling the case of directive names as as rule or assignments). So I'm - # parsing the line first, determining if it's a rule or statement or - # expression. If expression, look for a directive string, then connect - # to the original Directive handling code here. - - assign_expr = None - if isinstance(expr, AssignmentExpression): - # weird case showing GNU Make's lack of reserved words and the - # sloppiness of my grammar tokenparser. - # define xyzzy := <-- multi-line variable masquerading as an Assignment - # ifdef := 123 <-- totally legal but I want to throw a warning - # export CC=gcc <-- looks like an assignment - # Need to dig into the assignment to get the LHS. - assign_expr = expr - expr = expr.token_list[0] - - # We're only interested in Directives at this point. A Directive will be - # inside string Literal. - # - # Leave anything else alone. Invalid context function calls will error out - # during execute(). For example, $(subst ...) alone on a line will error - # with "*** missing separator. Stop." - - # If the first token isn't a Literal, we're done. - tok = expr.token_list[0] - if not isinstance(tok, Literal): - return assign_expr if assign_expr else expr - - # seek_directive() needs a character iterator - viter = ScannerIterator(tok.string, tok.string.get_pos()[0]) - - # peek at the first character of the line. If it's a aka recipeprefix then - # WOW does our life get complicated. GNU Make allows directives (with a - # couple exceptions) to be recipeprefix'd. - # TODO not handling recipeprefix yet - first_vchar = viter.lookahead() - - directive_vstr = seek_directive(viter) - if not directive_vstr: - # nope, not a directive. Ignore this expression and let execute figure it out - return assign_expr if assign_expr else expr - - # at this point, we definitely have some sort of directive. - assert viter.is_empty() - - # delete the whitespace token after the directive if it exists - if len(expr.token_list) > 1 and expr.token_list[1].is_whitespace(): - del expr.token_list[1] - - dir_str = str(directive_vstr) - - if assign_expr: - # If we've peeked into an assignment and decided this is re-using a - # directive name as an assign, throw a warning about using directive - # name in a weird context. - # - # yet another corner case of totally legit directive syntax: - # define = foo - # define = - # export CC=gcc - # override CC=xcc - # override define foo= - - if dir_str in ("define", "export", "override") and len(expr.token_list) > 1: - # These directives can validly be an assignment so parse them as - # a directive. The len() above checks that there's multiple - # symbols on the LHS before the assignment operator. - - # extract the directive from the expression - # We want to throw away the first Literal containing the directive - # expr is a ref into assign_expr.token_list[0] - # so I should be able to delete expr.token_list[0] and kill the directive literal - assert assign_expr.token_list[0].token_list[0] is expr.token_list[0] - - del expr.token_list[0] - - dir_ = parse_directive(assign_expr, directive_vstr, virt_line, vline_iter) - else: - # GNU Make doesn't warn like this (pats self on back). - logger.warning("re-using a directive name \"%s\" in an assignment at %r", dir_str, assign_expr.get_pos()) - return assign_expr - else: - # extract the directive from the expression - # We want to throw away the first Literal containing the directive - del expr.token_list[0] - - dir_ = parse_directive(expr, directive_vstr, virt_line, vline_iter) - -# print("parse directive=%s" % dir_.makefile()) - return dir_ - def parse_rule(vchar_scanner): lhs = tokenizer.tokenize_rule(vchar_scanner) if lhs is None: diff --git a/pymake/pymake.py b/pymake/pymake.py index 0c0ccbb..3b913bb 100644 --- a/pymake/pymake.py +++ b/pymake/pymake.py @@ -639,6 +639,7 @@ def execute(makefile, args): return exit_status["error"] if exit_code else exit_status["success"] def _run_it(args): + logger.debug("run_it args=\"%s\"", args) # -C option if args.directory: os.chdir(os.path.join(*args.directory)) diff --git a/pymake/symbol.py b/pymake/symbol.py index 2f6f76e..d1597f2 100644 --- a/pymake/symbol.py +++ b/pymake/symbol.py @@ -144,8 +144,7 @@ class Literal(Symbol): def __init__(self, vstring): # catch bugs where I create empty Literals if not len(vstring): - breakpoint() - assert len(vstring) + raise InternalError("attempt to create empty vstring") # cache the literal string self._str = str(vstring) @@ -349,7 +348,12 @@ def assign(lhs, op, rhs, symbol_table, flags=0): pos = lhs.get_pos() logger.debug("assignment rhs=%s", rhs) -# breakpoint() + + if flags & AssignmentExpression.FLAG_OVERRIDE: + raise NotImplementedError("override") + + if flags & AssignmentExpression.FLAG_PRIVATE: + raise NotImplementedError("private") if op_str == "?=": symbol_table.maybe_add(key, rhs, pos) @@ -422,7 +426,6 @@ def add_modifiers(self, modifier_list): self.modifier_flags |= self.FLAG_OVERRIDE else: assert 0, s - self.sanity() def __str__(self): diff --git a/pymake/symtable.py b/pymake/symtable.py index f124ae4..fdbef59 100644 --- a/pymake/symtable.py +++ b/pymake/symtable.py @@ -82,6 +82,7 @@ def value(self): def set_value(self, value, pos): # TODO add a stack where the values get changed + logger.debug("overwrite value name=%s at pos=%r", self.name, pos) self.pos = pos self._value = value @@ -151,9 +152,21 @@ def sanity(self): # -- GNU Make manual Version 4.3 Jan 2020 class DefaultEntry(Entry): origin = "default" - never_export = True -class CallbackEntry(DefaultEntry): +# An Immutable Entry is one where we cannot change the value BUT we can +# completely change the entry. For example, .VARIABLES has a special meaning in +# Make (a string containing names of all the defined variables) BUT we can do something bone-headed like: +# +# .VARIABLES:=foo # kill the .VARIABLES variable +# +# When am Immutable Entry is updated, the set_value() will throw an exception. +# The Symbol Table add() method will then create a new entry instead. +# +class ImmutablemixIn: + def set_value(self, value, pos): + raise ValueError("cannot update CallbackEntry name=%s" % self.name) + +class CallbackEntry(ImmutablemixIn, DefaultEntry): # Some built-in variables have complex requirements that can't be stored as # a simple string in the symbol table. For example .VARIABLES returns a # list of the variables currently defined. @@ -167,12 +180,13 @@ def __init__(self, name, callback_fn): def eval(self, symbol_table): return self._value(symbol_table) + # Environment Variables are higher precedence than File variables. But Command # Line args are higher precedence than Environment Variables. # "By default, only variables that came from the environment or the # command line are passed to recursive invocations." # -- GNU Make manual Version 4.3 Jan 2020 -class EnvVarEntry(Entry): +class EnvVarEntry(ImmutablemixIn, Entry): origin = "environment" def __init__(self, name, value): @@ -188,16 +202,13 @@ def __init__(self, name, value): # "By default, only variables that came from the environment or the # command line are passed to recursive invocations." # -- GNU Make manual Version 4.3 Jan 2020 -class CommandLineEntry(Entry): +class CommandLineEntry(ImmutablemixIn, Entry): origin = "command line" def __init__(self, name, value): super().__init__(name,value) self.set_export(Export.SUBMAKE) - def set_value(self, value, pos): - pass - class AutomaticEntry(Entry): origin = "automatic" @@ -312,29 +323,49 @@ def add(self, name, value, pos=None): try: entry = self.symbols[name] - logger.debug("overwrite value name=%s at pos=%r", entry.name, pos) except KeyError: - entry = None - - # if we didn't find it, make it - if self.command_line_flag: - # this flag is a weird hack to support vars created by eval()'ing - # expressions from the command line - new_entry = CommandLineEntry(name, value) - # command line vars are always exported until explicitly unexported + entry = None + + if entry is None: + # easy case: if we didn't find it, make it + if self.command_line_flag: + # this flag is a weird hack to support vars created by eval()'ing + # expressions from the command line + # - command line vars are always exported until explicitly + # marked 'unexport' + # - command line vars cannot be overwritten by file variables + # except by the 'override' directive (not yet implemented) + entry = CommandLineEntry(name, value) + else: + if pos and pos[0] == "@defaults": + entry = DefaultEntry(name, value, pos) + else: + entry = FileEntry(name, value, pos) + entry.set_export(self.export_default_value) + + self._add_entry(entry) else: - # command line > file - if isinstance(entry,CommandLineEntry): - return - - new_entry = FileEntry(name, value, pos) - new_entry.set_export(self.export_default_value) - - # XXX not sure read-only is a good idea yet -# if entry.read_only: -# raise PermissionDenied(name) - - self._add_entry(new_entry) + overwrite = False + try: + entry.set_value(value, pos) + except ValueError: + overwrite = True + + if overwrite: + # Found an immutable variable that refuses to be overwritten. + # So (maybe) create a new one. + overwrite_entry = None + if self.command_line_flag: + # command line entry can happily override command line entry + overwrite_entry = CommandLineEntry(name, value) + else: + # do not overwrite a command line entry when we're + # no longer processing command line variables + if not isinstance(entry, CommandLineEntry): + overwrite_entry = FileEntry(name, value, pos) + + if overwrite_entry: + self._add_entry(overwrite_entry) def add_automatic(self, name, value, pos): entry = AutomaticEntry(name, value, pos) diff --git a/tests/test_assign.py b/tests/test_assign.py index d0cbc81..5953025 100644 --- a/tests/test_assign.py +++ b/tests/test_assign.py @@ -1,3 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2014-2024 David Poole davep@mbuf.com david.poole@ericsson.com + import run # Test all the ways we can assign a variable @@ -37,3 +40,14 @@ def test_add(): expect = "FOO=foo bar" run_test(makefile, expect) +def test_update(): + makefile = """ +FOO:=foo +FOO:=$(FOO) foo +FOO:=bar $(FOO) + +@:; @echo $(FOO) +""" + expect = "bar foo foo" + run_test(makefile, expect) + diff --git a/tests/test_export.py b/tests/test_export.py index 0d8ab1c..4a8e238 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -264,12 +264,35 @@ def test_specific_unexport(): expect=("gcc","no CFLAGS for you") run_test(makefile,expect) -@pytest.mark.skip(reason="FIXME") -def test_circular_export_bug(): +def test_circular_export(): makefile=""" -VERSION=$(shell cat /etc/os-release) -export VERSION -@:;@: +FOO=$(shell echo foo) +export FOO +@:; @printenv FOO """ - expect = ("",) + expect = ("foo",) run_test(makefile,expect) + +def test_export_commandline_var(): + # command line variable assignments always override export status of file + # variables + makefile=""" +export FOO:=foo +@:; @echo $(FOO) +""" + expect = ("bar",) + run_test(makefile, expect, extra_args=("FOO=bar",)) + + +def test_export_commandline_var_explicit(): + # command line variable assignments always override export status of file + # variables + makefile=""" +export FOO:=foo +@:; @echo $(FOO) +""" + expect = ("bar",) + # use explicity assign instead of recursive assign + run_test(makefile, expect, extra_args=("FOO:=bar",)) + + diff --git a/tests/test_origin.py b/tests/test_origin.py index ec81580..faec9e9 100644 --- a/tests/test_origin.py +++ b/tests/test_origin.py @@ -87,3 +87,13 @@ def test_command_line(): output = run.gnumake_string(makefile, extra_args=("FOO=BAR",)) assert output.strip() == "command line", output +def test_default(): + makefile=""" +ifneq ($(origin CC),default) +$(error CC origin should be 'default' not '$(origin CC)') +endif + +@:;@: +""" + run.simple_test(makefile) + diff --git a/tests/test_symtable.py b/tests/test_symtable.py index bee3c9f..ce4e740 100644 --- a/tests/test_symtable.py +++ b/tests/test_symtable.py @@ -1,3 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2014-2024 David Poole davep@mbuf.com david.poole@ericsson.com +# import logging logger = logging.getLogger("pymake") @@ -101,7 +104,9 @@ def test_env_var(): symbol_table = symtable.SymbolTable() save_path = symbol_table.fetch("PATH") + # should always find PATH in the env no matter the test environment assert save_path + assert symbol_table.origin("PATH") == "environment" symbol_table.push("PATH") symbol_table.add("PATH", "a:b:c:") @@ -113,6 +118,9 @@ def test_env_var(): path = symbol_table.fetch("PATH") assert path==save_path + symbol_table.add("PATH", Literal("/bin")) + assert symbol_table.origin("PATH") == "file" + def test_is_defined(): # verify we check all the ways a symbol can be defined symbol_table = symtable.SymbolTable() @@ -215,7 +223,57 @@ def test_append_recursive(): value = symbol_table.fetch("CFLAGS") assert value == "-g -Wall" symbol_table.append("CFLAGS", Expression([Literal("-Wextra")])) + value = symbol_table.fetch("CFLAGS") + assert value == "-g -Wall -Wextra" + +def test_update(): + # I had a bug where I was creating a new entry for every 'add' + # so let's make sure I'm actually _UPDATING_ the dang thing now. + 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"]) + symbol_table.add("FOO", Literal("oof")) + value = symbol_table.fetch("FOO") + assert value == "oof" + id2 = id(symbol_table.symbols["FOO"]) + assert id1==id2 -if __name__ == '__main__': -# test_push_push_pop_pop() - test_maybe_add() +def test_command_line(): + # variable from command line, for example: + # make FOO:=foo + symbol_table = symtable.SymbolTable() + symbol_table.command_line_start() + symbol_table.add("FOO", Literal("foo")) + symbol_table.command_line_stop() + value = symbol_table.fetch("FOO") + assert value == "foo" + assert symbol_table.origin("FOO") == "command line" + + # command line vars are always marked for export + exports = symbol_table.get_exports() + assert exports["FOO"] == "foo" + + # should not be able to update a command line var + symbol_table.add("FOO", Literal("oof")) + value = symbol_table.fetch("FOO") + assert value == "foo" # unchanged + assert symbol_table.origin("FOO") == "command line" + +def test_command_line_multiple_var(): + # multiple variable with same name from command line, for example: + # make FOO:=foo FOO:=bar FOO:=baz + # These can update the value. + symbol_table = symtable.SymbolTable() + symbol_table.command_line_start() + symbol_table.add("FOO", Literal("foo")) + symbol_table.add("FOO", Literal("bar")) + symbol_table.add("FOO", Literal("baz")) + symbol_table.command_line_stop() + value = symbol_table.fetch("FOO") + # last value wins + assert value == "baz" + assert symbol_table.origin("FOO") == "command line" +