From 799e6d4e1a4e4a4f59e58298bbb9c01d286159dc Mon Sep 17 00:00:00 2001 From: Ulf Magnusson Date: Sun, 24 Sep 2017 09:49:20 +0200 Subject: Fix get_defconfig_filename() $srctree search order Previously, $srctree/path/to/defconfig would be looked up before /path/to/defconfig, and the code wouldn't check if /path/to/defconfig was an absolute path ($srctree is ignored otherwise). Sloppy old oversights. The behavior now fully matches the C implementation. Also fix some related things: - An 'if m' suffices to select a defconfig. We previously required 'y'. - Make the code less hacky and possibly more Windows-friendly by using os.path.relpath() to de-absolutize paths, and stop using os.path.normpath() as it could change the meaning of paths that contain symbolic links. - Explain what happens if 'option defconfig_list' is set on multiple symbols and print a warning in that case. - Fix get_srctree(). It would previously return "." instead of None if $srctree was unset at parse time. Somehow forgot to to test this. The code is now much more straightforward. --- kconfiglib.py | 56 +++++++++++++++++++++++++++++----------------- tests/Kdefconfig_srctree | 5 +++++ tests/sub/defconfig_in_sub | 0 testsuite.py | 32 ++++++++++++++++++++++---- 4 files changed, 68 insertions(+), 25 deletions(-) create mode 100644 tests/Kdefconfig_srctree create mode 100644 tests/sub/defconfig_in_sub diff --git a/kconfiglib.py b/kconfiglib.py index e200242..2eecc3b 100644 --- a/kconfiglib.py +++ b/kconfiglib.py @@ -182,14 +182,15 @@ class Config(object): if self.config_prefix is None: self.config_prefix = "CONFIG_" + self.filename = filename + # See Config.__init__(). We need this for get_defconfig_filename(). self.srctree = os.environ.get("srctree") - if self.srctree is None: - self.srctree = "." - self.filename = filename - self.base_dir = self.srctree if base_dir is None else \ - os.path.expandvars(base_dir) + if base_dir is None: + self.base_dir = "." if self.srctree is None else self.srctree + else: + self.base_dir = os.path.expandvars(base_dir) # The 'mainmenu' text self.mainmenu_text = None @@ -287,13 +288,14 @@ class Config(object): file in the list given in a symbol having 'option defconfig_list' set. $-references to symbols will be expanded ("$FOO bar" -> "foo bar" if FOO has the value "foo"). Returns None in case of no defconfig file. - Setting 'option defconfig_list' on multiple symbols currently results - in undefined behavior. + Setting 'option defconfig_list' on multiple symbols ignores the symbols + past the first one (and prints a warning). If the environment variable 'srctree' was set when the Config was - created, get_defconfig_filename() will first look relative to that - directory before looking in the current directory; see - Config.__init__(). + created, each defconfig specified with an absolute path will be + searched for in $srcdir if it is not found at the specified path (i.e., + if /foo/defconfig is not found, $srctree/foo/defconfig will be looked + up). WARNING: A wart here is that scripts/kconfig/Makefile sometimes uses the --defconfig= option when calling the C implementation of @@ -305,16 +307,22 @@ class Config(object): if self.defconfig_sym is None: return None for filename, cond_expr in self.defconfig_sym.def_exprs: - if self._eval_expr(cond_expr) == "y": + if self._eval_expr(cond_expr) != "n": filename = self._expand_sym_refs(filename) - # We first look in $srctree. os.path.join() won't work here as - # an absolute path in filename would override $srctree. - srctree_filename = os.path.normpath(self.srctree + "/" + - filename) - if os.path.exists(srctree_filename): - return srctree_filename if os.path.exists(filename): return filename + # defconfig not found. If the path is an absolute path and + # $srctree is set, we also look in $srctree. + if os.path.isabs(filename) and self.srctree is not None: + # The os.path.relpath() de-absolutizes the path, e.g. + # /foo/bar/baz -> foo/bar/baz. os.path.join() ignores the + # first argument if the second argument is an absolute + # path. + filename = os.path.join(self.srctree, + os.path.relpath(filename, os.sep)) + if os.path.exists(filename): + return filename + return None def get_symbol(self, name): @@ -587,9 +595,9 @@ class Config(object): "Value of $SRCARCH at creation time : " + ("(not set)" if self.srcarch is None else self.srcarch), - "Source tree (derived from $srctree;", - "defaults to '.' if $srctree isn't set) : " + - self.srctree, + "Value of $srctree at creation time : " + + ("(not set)" if self.srctree is None else + self.srctree), "Most recently loaded .config : " + ("(no .config loaded)" if self.config_filename is None else @@ -975,7 +983,13 @@ class Config(object): stmt.cached_val = os.environ[env_var] elif tokens.check(T_DEFCONFIG_LIST): - self.defconfig_sym = stmt + if self.defconfig_sym is None: + self.defconfig_sym = stmt + else: + self._warn("'option defconfig_list' set on multiple " + "symbols ({0} and {1}). Only {0} will be " + "used." + .format(self.defconfig_sym.name, stmt.name)) elif tokens.check(T_MODULES): # To reduce warning spam, only warn if 'option modules' is diff --git a/tests/Kdefconfig_srctree b/tests/Kdefconfig_srctree new file mode 100644 index 0000000..3aa4505 --- /dev/null +++ b/tests/Kdefconfig_srctree @@ -0,0 +1,5 @@ +config A + string + option defconfig_list + default "/sub/defconfig_in_sub" # Assume this doesn't exist + default "Kconfiglib/tests/defconfig_2" diff --git a/tests/sub/defconfig_in_sub b/tests/sub/defconfig_in_sub new file mode 100644 index 0000000..e69de29 diff --git a/testsuite.py b/testsuite.py index f582fb0..b2f306a 100644 --- a/testsuite.py +++ b/testsuite.py @@ -539,8 +539,7 @@ def run_selftests(): Base directory : . Value of $ARCH at creation time : (not set) Value of $SRCARCH at creation time : (not set) - Source tree (derived from $srctree; - defaults to '.' if $srctree isn't set) : . + Value of $srctree at creation time : (not set) Most recently loaded .config : (no .config loaded) Print warnings : true Print assignments to undefined symbols : false""") @@ -560,8 +559,7 @@ def run_selftests(): Base directory : foobar Value of $ARCH at creation time : foo Value of $SRCARCH at creation time : bar - Source tree (derived from $srctree; - defaults to '.' if $srctree isn't set) : baz + Value of $srctree at creation time : baz Most recently loaded .config : Kconfiglib/tests/empty Print warnings : false Print assignments to undefined symbols : true""") @@ -1465,6 +1463,20 @@ def run_selftests(): "get_defconfig_filename() should return the existent file " "Kconfiglib/tests/defconfig_2") + # Should also look relative to $srctree if the defconfig is an absolute + # path and not found + + del os.environ["srctree"] + c = kconfiglib.Config("Kconfiglib/tests/Kdefconfig_srctree") + verify(c.get_defconfig_filename() == "Kconfiglib/tests/defconfig_2", + "get_defconfig_filename() returned wrong file with $srctree unset") + + os.environ["srctree"] = "Kconfiglib/tests" + c = kconfiglib.Config("Kconfiglib/tests/Kdefconfig_srctree") + verify(c.get_defconfig_filename() == + "Kconfiglib/tests/sub/defconfig_in_sub", + "get_defconfig_filename() returned wrong file with $srctree set") + # # get_mainmenu_text() # @@ -1758,6 +1770,18 @@ def run_selftests(): # get_arch/srcarch/srctree/kconfig_filename() # + del os.environ["ARCH"] + del os.environ["SRCARCH"] + del os.environ["srctree"] + + c = kconfiglib.Config("Kconfiglib/tests/Kmisc", print_warnings = False) + arch = c.get_arch() + verify(arch is None, "Expected None arch, got '{}'".format(arch)) + srcarch = c.get_srcarch() + verify(srcarch is None, "Expected None srcarch, got '{}'".format(srcarch)) + srctree = c.get_srctree() + verify(srctree is None, "Expected None srctree, got '{}'".format(srctree)) + os.environ["ARCH"] = "ARCH value" os.environ["SRCARCH"] = "SRCARCH value" os.environ["srctree"] = "srctree value" -- cgit v1.2.3