Conversation
Kconfig locations reached users with whatever separator the host used, so the same tree produced 'drivers/foo/Kconfig' on Linux but 'drivers/foo\bar/Kconfig' on Windows. The mixed form comes from the parser itself: 'rsource' builds its glob pattern with join(dirname(self.filename), pattern), and ntpath.join() inserts a backslash while the pattern text read from the Kconfig file keeps its forward slashes. Put the rule in one place, _norm_loc(): a location is slash-separated unless it is absolute, in which case it is kept exactly as supplied. Absolute paths are exempt because they are platform-specific anyway and are the ones likely to be compared against os.path output, which kconfiglib itself does when converting a location for a warning. Apply it at the two sites where a filesystem path becomes a stored location. _enter_file() covers sourced files, on both the $srctree branch and the branch taken by absolute 'source' targets and by search_paths hits, which are relative and so must be normalized too. _init() covers the top-level file. Without the latter, that one entry keeps the caller's separators while every sourced file is normalized, which leaves kconfig_filenames internally inconsistent and defeats the recursive 'source' check in _enter_file(): it compares _include_path entries against rel_filename, so 'menuconfig sub\Kconfig' on a self-sourcing file recursed until the stack blew instead of raising KconfigError. Compare the glob result and $srctree in one separator form so the prefix strip cannot miss on Windows, where join() and the Kconfig file's own text disagree. glob() and join() both accept forward slashes there, so _srctree_prefix can stay slash-separated for its other job as the glob root. Take os.fspath() on the Kconfig() argument. Passing a pathlib.Path has always worked, and str.replace() is not Path.replace(); locations are now reported as str either way, instead of one Path among strings. Correct the location documentation while adding the slash guarantee to it. kconfig_filenames and MenuNode.loc both claimed filenames are $srctree-relative "except absolute paths are used for paths outside $srctree", which is not the rule the code implements: _enter_file() matches a string prefix, so a redundant absolute path inside $srctree is reduced, and a search_paths hit stays relative without being $srctree-relative. State what actually happens, and cross-reference it from Kconfig.filename and MenuNode.include_path, the other two surfaces carrying these strings. Fix the _enter_file() parameter comment too: it still promised an absolute path, which the search_paths branch disproves. Test _norm_loc() by injecting ntpath's sep and isabs(), which exercises the real Windows rule from POSIX, where os.sep is already '/' and nothing else can observe it. Cover the search_paths branch with tests/Ksearchtop, the absolute top-level filename, and the pathlib argument. Hoist the shared tests/Klocation environment setup into a klocation_env fixture, replacing the copy in test_node_iter() as well. Replace the _ordered_unique() seen-set idiom with list(dict.fromkeys(lst)). Dict insertion order is guaranteed on every supported version, and both call sites hold Symbol/Choice objects that use identity hashing, so ordering and membership are unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Kconfig locations reached users with whatever separator the host used, so the same tree produced 'drivers/foo/Kconfig' on Linux but 'drivers/foo\bar/Kconfig' on Windows. The mixed form comes from the parser itself: 'rsource' builds its glob pattern with join(dirname(self.filename), pattern), and ntpath.join() inserts a backslash while the pattern text read from the Kconfig file keeps its forward slashes.
Normalize the $srctree-relative filename in _enter_file() so that Kconfig.filename, Kconfig.kconfig_filenames and MenuNode.filename are slash-separated everywhere. Absolute paths stay untouched, as kconfig_filenames has always documented, because they are inherently platform-specific and are the ones likely to be compared against os.path.realpath() output.
Apply the same rule to the top-level filename in _init(). Without it that one entry keeps the caller's separators while every sourced file is normalized, which leaves kconfig_filenames internally inconsistent and defeats the recursive 'source' check in _enter_file(): it compares _include_path entries against rel_filename, so 'menuconfig sub\Kconfig' on a self-sourcing file recursed until the stack blew instead of raising KconfigError.
Document the guarantee on the two public docstrings describing these attributes, and add test_filenames_are_slash_separated. Its slash assertions are inert on POSIX, where os.sep is already '/', but they pin the contract for the Windows runner, and the absolute-path assertion exercises the isabs() branch everywhere. Hoist the shared tests/Klocation environment setup into a klocation_env fixture instead of copying it into the new test.
Replace the _ordered_unique() seen-set idiom with
list(dict.fromkeys(lst)). Dict insertion order is guaranteed on every supported version, and both call sites hold Symbol/Choice objects that use identity hashing, so ordering and membership are unchanged.
Summary by cubic
Makes relative Kconfig file locations slash-separated on every platform, so the same tree yields
drivers/foo/Kconfigon Windows instead ofdrivers/foo\bar/Kconfig._enter_file()and the top-level file in_init()via_norm_loc(), and keeps$srctreeprefix comparisons slash-separated; absolute paths stay untouched.pathlib.PathinKconfig()and now always reports locations as strings.KconfigError.kconfig_filenames,MenuNode.filename, andKconfig.filename._ordered_unique()withlist(dict.fromkeys(lst)).ntpathinto_norm_loc(), and covers thesearch_pathsbranch and absolute top-level filenames.Written for commit 807577e. Summary will update on new commits.