From cbf32e29a130d22bc734b7778e6304ac9df2a3e8 Mon Sep 17 00:00:00 2001 From: Ulf Magnusson Date: Mon, 14 May 2018 18:00:40 +0200 Subject: Expand environment variables in strings directly Make "$FOO" directly reference the environment variable $FOO in e.g. 'source' statements, instead of the symbol FOO. Use os.path.expandvars() to expand strings (which preserves "$FOO" as-is if no environment variable FOO exists). This gets rid of the 'option env' "bounce" symbols, which are mostly just spam and are buggy in the C tools (dependencies aren't always respected, due to parsing and evaluation getting mixed up). The same change will probably appear soon in the C tools as well. Keep accepting 'option env' to preserve some backwards compatibility, but ignore it when expanding strings. For compatibility with the C tools, bounce symbols will need to be named the same as the environment variables they reference (which is the case for the Linux kernel). This is a compatibility break, so the major version will be bumped to 6 at the next release. The main motivation for adding this now is to allow recording properties on each MenuNode in a clean way. 'option env' symbols interact badly with delayed dependency propagation. Side note: I have a feeling that recording environment variable values might be redundant to trigger rebuilds if sync_deps() is run at each compile. It should detect all changes to symbol values due to environment variables changing value. --- kconfiglib.py | 80 ++++++++++++++++++----------------------- tests/Kdefconfig_existent | 5 --- tests/Kdefconfig_existent_but_n | 4 --- tests/Klocation | 24 ------------- testsuite.py | 13 +++++-- 5 files changed, 44 insertions(+), 82 deletions(-) diff --git a/kconfiglib.py b/kconfiglib.py index 6a7e0ed..083ef8e 100644 --- a/kconfiglib.py +++ b/kconfiglib.py @@ -79,8 +79,8 @@ Using Kconfiglib without the Makefile targets ============================================= The make targets are only needed for a trivial reason: The Kbuild makefiles -export environment variables which are referenced inside the Kconfig files (via -'option env="ENV_VARIABLE"'). +export environment variables which are referenced inside the Kconfig files +(via e.g. 'source "arch/$SRCARCH/Kconfig"). In practice, the only variables referenced (as of writing, and for many years) are ARCH, SRCARCH, and KERNELVERSION. To run Kconfiglib without the Makefile @@ -384,6 +384,7 @@ import os import platform import re import sys +import textwrap # File layout: # @@ -745,7 +746,7 @@ class Kconfig(object): """ See the class documentation. """ - return self._expand_syms(self.top_node.prompt[0]) + return os.path.expandvars(self.top_node.prompt[0]) @property def defconfig_filename(self): @@ -758,7 +759,7 @@ class Kconfig(object): for filename, cond in self.defconfig_list.defaults: if expr_value(cond): try: - with self._open(self._expand_syms(filename.str_value)) as f: + with self._open(os.path.expandvars(filename.str_value)) as f: return f.name except IOError: continue @@ -1449,7 +1450,7 @@ class Kconfig(object): # https://docs.python.org/3/reference/compound_stmts.html#the-try-statement e = e2 - raise IOError( + raise IOError(textwrap.fill( "Could not open '{}' ({}: {}). Perhaps the $srctree " "environment variable (which was {}) is set incorrectly. Note " "that the current value of $srctree is saved when the Kconfig " @@ -1457,7 +1458,8 @@ class Kconfig(object): "separate instances)." .format(filename, errno.errorcode[e.errno], e.strerror, "unset" if self.srctree is None else - '"{}"'.format(self.srctree))) + '"{}"'.format(self.srctree)), + 80)) def _enter_file(self, filename): # Jumps to the beginning of a sourced Kconfig file, saving the previous @@ -1481,13 +1483,18 @@ class Kconfig(object): self._file = self._open(filename) except IOError as e: # Extend the error message a bit in this case - raise IOError( - "{}:{}: {} Also note that e.g. $FOO in a 'source' " - "statement does not refer to the environment " - "variable FOO, but rather to the Kconfig Symbol FOO " - "(which would commonly have 'option env=\"FOO\"' in " - "its definition)." - .format(self._filename, self._linenr, str(e))) + raise IOError(textwrap.fill( + "{}:{}: {} Also note that Kconfiglib expands references to " + "environment variables directly (via os.path.expandvars()), " + "meaning you do not need \"bounce\" symbols with " + "'option env=\"FOO\"'. For compatibility with the C tools, " + "name the bounce symbols the same as the environment variable " + "they reference (like the Linux kernel does). This change is " + "likely to soon appear in the C tools as well, and simplifies " + "the parsing implementation (symbols no longer need to be " + "evaluated during parsing)." + .format(self._filename, self._linenr, str(e)), + 80)) self._filename = filename self._linenr = 0 @@ -1941,20 +1948,20 @@ class Kconfig(object): prev.next = prev = node elif t0 == _T_SOURCE: - self._enter_file(self._expand_syms(self._expect_str_and_eol())) + self._enter_file(os.path.expandvars(self._expect_str_and_eol())) prev = self._parse_block(None, parent, prev, visible_if_deps) self._leave_file() elif t0 == _T_RSOURCE: self._enter_file(os.path.join( os.path.dirname(self._filename), - self._expand_syms(self._expect_str_and_eol()) + os.path.expandvars(self._expect_str_and_eol()) )) prev = self._parse_block(None, parent, prev, visible_if_deps) self._leave_file() elif t0 in (_T_GSOURCE, _T_GRSOURCE): - pattern = self._expand_syms(self._expect_str_and_eol()) + pattern = os.path.expandvars(self._expect_str_and_eol()) if t0 == _T_GRSOURCE: # Relative gsource pattern = os.path.join(os.path.dirname(self._filename), @@ -2271,10 +2278,9 @@ class Kconfig(object): node.item.env_var = env_var if env_var not in os.environ: - self._warn("'option env=\"{0}\"' on symbol {1} has " - "no effect, because the environment " - "variable {0} is not set" - .format(env_var, node.item.name), + self._warn("{1} has 'option env=\"{0}\"', " + "but the environment variable {0} is not " + "set".format(node.item.name, env_var), self._filename, self._linenr) else: defaults.append( @@ -2571,21 +2577,6 @@ class Kconfig(object): # Misc. # - def _expand_syms(self, s): - # Expands $-references to symbols in 's' to symbol values, or to the - # empty string for undefined symbols - - while 1: - sym_ref_match = _sym_ref_re_search(s) - if not sym_ref_match: - return s - - sym = self.syms.get(sym_ref_match.group(1)) - - s = s[:sym_ref_match.start()] + \ - (sym.str_value if sym else "") + \ - s[sym_ref_match.end():] - def _parse_error(self, msg): if self._filename is None: loc = "" @@ -2803,18 +2794,18 @@ class Symbol(object): env_var: If the Symbol has an 'option env="FOO"' option, this contains the name - ("FOO") of the environment variable. None for symbols that aren't set - from the environment. + ("FOO") of the environment variable. None for symbols without no + 'option env'. - 'option env="FOO"' acts as a 'default' property whose value is the value - of $FOO. + 'option env="FOO"' acts like a 'default' property whose value is the + value of $FOO. env_var is set to "" for the predefined symbol UNAME_RELEASE, which holds the 'release' field from uname. - Symbols with an 'option env' option are never written out to .config - files, even if they are visible. env_var corresponds to a flag called - SYMBOL_AUTO in the C implementation. + Symbols with 'option env' are never written out to .config files, even if + they are visible. env_var corresponds to a flag called SYMBOL_AUTO in the + C implementation. is_allnoconfig_y: True if the symbol has 'option allnoconfig_y' set on it. This has no @@ -3192,7 +3183,7 @@ class Symbol(object): if self.env_var is not None: self.kconfig._warn("ignored attempt to assign user value to " - "{}, which gets its value from the environment" + "{}, which is set from the environment" .format(_name_and_loc(self))) return False @@ -5063,9 +5054,6 @@ _initial_token_re_match = \ # Matches an identifier/keyword, also eating trailing whitespace _id_keyword_re_match = re.compile(r"([A-Za-z0-9_/.-]+)\s*", _RE_ASCII).match -# Regular expression for finding $-references to symbols in strings -_sym_ref_re_search = re.compile(r"\$([A-Za-z0-9_]+)", _RE_ASCII).search - # Matches a valid right-hand side for an assignment to a string symbol in a # .config file, including escaped characters. Extracts the contents. _conf_string_re_match = re.compile(r'"((?:[^\\"]|\\.)*)"', _RE_ASCII).match diff --git a/tests/Kdefconfig_existent b/tests/Kdefconfig_existent index 8dde443..304cae6 100644 --- a/tests/Kdefconfig_existent +++ b/tests/Kdefconfig_existent @@ -1,9 +1,4 @@ # $FOO is "defconfig_2" -# Should produce "Kconfiglib/tests/defconfig_2" - -config FOO - string - option env="BAR" config A string diff --git a/tests/Kdefconfig_existent_but_n b/tests/Kdefconfig_existent_but_n index 2d55e97..2fdaaa9 100644 --- a/tests/Kdefconfig_existent_but_n +++ b/tests/Kdefconfig_existent_but_n @@ -1,10 +1,6 @@ # $FOO is "defconfig_2" # Should produce None due to the "depends on n" -config FOO - string - option env="BAR" - config A string depends on n diff --git a/tests/Klocation b/tests/Klocation index d3ac09c..6438739 100644 --- a/tests/Klocation +++ b/tests/Klocation @@ -33,30 +33,6 @@ config MULTI_DEF endif endif -config TESTS_DIR_FROM_ENV - string - option env="TESTS_DIR_FROM_ENV" - -config SUB_DIR_FROM_ENV - string - option env="SUB_DIR_FROM_ENV" - -config _SOURCED - string - default "_sourced" - -config _RSOURCED - string - default "_rsourced" - -config _GSOURCED - string - default "_gsourced" - -config _GRSOURCED - string - default "_grsourced" - # Expands to "tests/Klocation_sourced" source "$TESTS_DIR_FROM_ENV/Klocation$_SOURCED" diff --git a/testsuite.py b/testsuite.py index fbc511b..3f29ae3 100644 --- a/testsuite.py +++ b/testsuite.py @@ -808,10 +808,17 @@ g .format(repr(node), expected_loc, node_loc)) # Expanded in the 'source' statement in Klocation + os.environ["TESTS_DIR_FROM_ENV"] = "tests" os.environ["SUB_DIR_FROM_ENV"] = "sub" os.environ["srctree"] = "Kconfiglib/" + os.environ["_SOURCED"] = "_sourced" + os.environ["_RSOURCED"] = "_rsourced" + os.environ["_GSOURCED"] = "_gsourced" + os.environ["_GRSOURCED"] = "_grsourced" + + # Has symbol with empty help text, so disable warnings c = Kconfig("tests/Klocation", warn=False) @@ -830,7 +837,7 @@ g "tests/sub/Klocation_gsourced2:1", "tests/sub/Klocation_grsourced1:1", "tests/sub/Klocation_grsourced2:1", - "tests/Klocation:78") + "tests/Klocation:54") verify_locations(c.named_choices["CHOICE"].nodes, "tests/Klocation_sourced:5") @@ -1258,7 +1265,7 @@ g "defconfig_list symbol exist") # Referenced in Kdefconfig_existent(_but_n) - os.environ["BAR"] = "defconfig_2" + os.environ["FOO"] = "defconfig_2" c = Kconfig("Kconfiglib/tests/Kdefconfig_existent_but_n") verify(c.defconfig_filename is None, @@ -1267,7 +1274,7 @@ g c = Kconfig("Kconfiglib/tests/Kdefconfig_existent") verify(c.defconfig_filename == "Kconfiglib/tests/defconfig_2", - "defconfig_filename should return the existent file " + "defconfig_filename should return the existing file " "Kconfiglib/tests/defconfig_2") # Should also look relative to $srctree if the specified defconfig is a -- cgit v1.2.3