diff --git a/kernelci/kbuild.py b/kernelci/kbuild.py index a7b97767e9..e31ec257fe 100644 --- a/kernelci/kbuild.py +++ b/kernelci/kbuild.py @@ -57,6 +57,8 @@ # Prefix marking a fragment entry as a kernel make target generating # config (e.g. 'make:kselftest-merge') rather than a config symbol. MAKE_FRAGMENT_PREFIX = "make:" +TREE_FRAGMENT_PREFIX = "tree:" +TREE_FRAGMENT_PATH = re.compile(r"[A-Za-z0-9._/-]+") DTBS_DISABLED = { "i386": True, @@ -621,40 +623,69 @@ def add_fragment(self, fragname): @staticmethod def _split_fragment(content): - """Split fragment content into make targets and config symbols + """Split fragment content into directives and config symbols A fragment entry prefixed with 'make:' names a kernel make target - generating config, such as 'make:kselftest-merge', rather than a - config symbol. Those entries must be kept out of the fragment file: - kconfig does not understand them and merges them as "unexpected - data", silently dropping the config the fragment is meant to add. + generating config, such as 'make:kselftest-merge'. One prefixed + with 'tree:' names a config fragment file in the kernel source + tree, such as 'tree:tools/testing/selftests/arm64/config', merged + as that tree ships it. Neither is a config symbol, so both must be + kept out of the fragment file: kconfig does not understand them + and merges them as "unexpected data", silently dropping the config + the fragment is meant to add. Returns: - tuple: (list of make targets, config symbol text) + tuple: (list of 'make:' and 'tree:' directives, config symbol + text) """ - make_targets = [] + directives = [] config_lines = [] for line in content.splitlines(): entry = line.strip() - if entry.startswith(MAKE_FRAGMENT_PREFIX): - target = entry[len(MAKE_FRAGMENT_PREFIX) :] - if target: - make_targets.append(target) + if entry.startswith((MAKE_FRAGMENT_PREFIX, TREE_FRAGMENT_PREFIX)): + directives.append(entry) else: config_lines.append(line) config = "\n".join(config_lines).strip() if config: config += "\n" - return make_targets, config + return directives, config + + def _tree_fragment_path(self, directive): + """Return the tree-relative path a 'tree:' directive names + + The path is merged from the kernel source tree and written into + the build script unquoted, so one that is empty, absolute, leaves + the tree or holds anything but letters, digits, '.', '_', '-' and + '/' is refused as a job error. + """ + path = directive[len(TREE_FRAGMENT_PREFIX) :] + normalised = os.path.normpath(path) if path else "" + if ( + not normalised + or normalised == "." + or not TREE_FRAGMENT_PATH.fullmatch(normalised) + or os.path.isabs(normalised) + or normalised == ".." + or normalised.startswith(".." + os.sep) + ): + message = ( + f"Fragment directive {directive} is not a path in the tree" + ) + print(f"[_parse_fragments] {message}") + self.submit_failure(message) + sys.exit(1) + return normalised def _parse_fragments(self, firmware=False): """Parse fragments kbuild config and create config fragments Returns: - list: List of kconfig additions, each either a fragment file - path or a 'make:' directive, in merge order + list: List of kconfig additions, each a fragment file path, a + 'make:' directive or a 'tree:' directive, in + merge order """ kconfig_adds = [] @@ -678,7 +709,7 @@ def _parse_fragments(self, firmware=False): ) continue - make_targets, config = self._split_fragment(content) + directives, config = self._split_fragment(content) if config: fragfile = os.path.join(self._fragments_dir, f"{idx}.config") @@ -698,7 +729,18 @@ def _parse_fragments(self, firmware=False): frag_rel = os.path.relpath(fragfile, self._af_dir) self._artifacts.append(frag_rel) - for target in make_targets: + for directive in directives: + if directive.startswith(TREE_FRAGMENT_PREFIX): + path = self._tree_fragment_path(directive) + print( + f"[_parse_fragments] Fragment {fragment_name} merges " + f"{path} from the tree" + ) + kconfig_adds.append(TREE_FRAGMENT_PREFIX + path) + continue + target = directive[len(MAKE_FRAGMENT_PREFIX) :] + if not target: + continue print( f"[_parse_fragments] Fragment {fragment_name} runs " f"make target {target}" @@ -766,6 +808,14 @@ def _merge_frags(self, kconfig_adds): target = entry[len(MAKE_FRAGMENT_PREFIX) :] self.addcmd(f"make {target}") continue + if entry.startswith(TREE_FRAGMENT_PREFIX): + path = entry[len(TREE_FRAGMENT_PREFIX) :] + self.addcmd( + f"if [ -f {path} ]; then " + f"./scripts/kconfig/merge_config.sh -m .config {path}; " + f'else echo "{path} not in this tree, skipping"; fi' + ) + continue self.addcmd(f"./scripts/kconfig/merge_config.sh -m .config {entry}") # TODO: olddefconfig should be optional/configurable # TODO: log all warnings/errors of olddefconfig to separate file @@ -990,6 +1040,15 @@ def _tuxmake_base(self, output_dir, defconfig, extra_defconfigs): # tuxmake runs the make target during config preparation parts.append(f"--kconfig-add={entry}") print(f"[_tuxmake_base] Adding make target: {entry}") + elif entry.startswith(TREE_FRAGMENT_PREFIX): + path = os.path.join( + self._srcdir, entry[len(TREE_FRAGMENT_PREFIX) :] + ) + parts.append( + f"$(if [ -f {path} ]; then echo --kconfig-add={path}; " + f'else echo "{path} not in this tree, skipping" >&2; fi)' + ) + print(f"[_tuxmake_base] Adding tree fragment: {path}") elif os.path.exists(entry): parts.append(f"--kconfig-add={entry}") print( diff --git a/tests/test_kbuild.py b/tests/test_kbuild.py index 599201922e..4899d679b8 100644 --- a/tests/test_kbuild.py +++ b/tests/test_kbuild.py @@ -3,9 +3,12 @@ import json import os +import subprocess import sys import types +import pytest + from kernelci.kbuild import KBuild @@ -156,6 +159,176 @@ def test_make_target_is_run_by_the_make_backend(self, tmp_path): # kselftest-merge needs a .config to merge into assert steps.index("make defconfig") < merge + def test_tree_file_is_not_written_to_a_fragment_file(self, tmp_path): + kbuild = self._fragments( + tmp_path, + ["kselftest-arm64"], + { + "kselftest-arm64": { + "configs": ["tree:tools/testing/selftests/arm64/config"] + } + }, + ) + + kconfig_adds = kbuild._parse_fragments() + + assert kconfig_adds == ["tree:tools/testing/selftests/arm64/config"] + assert os.listdir(kbuild._fragments_dir) == [] + assert kbuild._artifacts == [] + assert kbuild._config_full == "+kselftest-arm64" + + def test_tree_files_are_split_from_config_symbols(self, tmp_path): + kbuild = self._fragments( + tmp_path, + ["kselftest-arm64"], + { + "kselftest-arm64": { + "configs": [ + "tree:tools/testing/selftests/arm64/config", + "CONFIG_KUNIT=y", + ] + } + }, + ) + + kconfig_adds = kbuild._parse_fragments() + + fragfile = os.path.join(kbuild._fragments_dir, "0.config") + assert kconfig_adds == [ + fragfile, + "tree:tools/testing/selftests/arm64/config", + ] + with open(fragfile) as f: + assert f.read() == "CONFIG_KUNIT=y\n" + + @pytest.mark.parametrize( + "path", + [ + "../outside.config", + "/etc/passwd", + "tools/../../x", + "", + "tools/x; rm -rf /", + "tools/$(id)", + "tools/`id`", + "tools/a b", + "tools/x|y", + ".", + "tools/..", + ], + ) + def test_tree_file_outside_the_tree_is_refused(self, tmp_path, path): + kbuild = self._fragments( + tmp_path, + ["bad"], + {"bad": {"configs": [f"tree:{path}"]}}, + ) + failures = [] + kbuild.submit_failure = failures.append + + with pytest.raises(SystemExit): + kbuild._parse_fragments() + + assert failures + + def test_tree_file_is_merged_by_the_make_backend(self, tmp_path): + kbuild = _kbuild(tmp_path) + kbuild._backend = "make" + + kbuild._merge_frags(["tree:tools/testing/selftests/arm64/config"]) + + steps = kbuild._steps + merge = next( + i + for i, s in enumerate(steps) + if "merge_config.sh -m .config tools/testing/selftests/arm64/config" + in s + ) + assert steps.index(f"cd {kbuild._srcdir}") < merge + assert steps.index("make defconfig") < merge + + @pytest.mark.parametrize("present", [True, False]) + def test_make_backend_merges_a_tree_file_only_when_present( + self, tmp_path, present + ): + kbuild = _kbuild(tmp_path) + kbuild._backend = "make" + kbuild._merge_frags(["tree:tools/testing/selftests/arm64/config"]) + merge = next(s for s in kbuild._steps if "merge_config.sh" in s) + src = tmp_path / "linux" + (src / "tools/testing/selftests/arm64").mkdir(parents=True) + if present: + (src / "tools/testing/selftests/arm64/config").write_text( + "CONFIG_X=y\n" + ) + merge_config = src / "scripts/kconfig/merge_config.sh" + merge_config.parent.mkdir(parents=True) + merge_config.write_text('#!/bin/sh\necho "$@" > merged\n') + merge_config.chmod(0o755) + script = tmp_path / "merge.sh" + script.write_text(f"set -eE -o pipefail\ncd {src}\n{merge}\n") + + result = subprocess.run( + ["bash", str(script)], capture_output=True, text=True + ) + + assert result.returncode == 0 + merged = src / "merged" + if present: + assert merged.read_text().split() == [ + "-m", + ".config", + "tools/testing/selftests/arm64/config", + ] + else: + assert not merged.exists() + assert "not in this tree" in result.stdout + + def test_tree_file_is_passed_to_tuxmake_from_the_tree(self, tmp_path): + kbuild = _kbuild(tmp_path) + kbuild._kconfig_adds = ["tree:tools/testing/selftests/arm64/config"] + + parts = kbuild._tuxmake_base(kbuild._af_dir, "defconfig", []) + + path = os.path.join( + kbuild._srcdir, "tools/testing/selftests/arm64/config" + ) + assert not os.path.exists(path) + added = [p for p in parts if path in p] + assert len(added) == 1 + assert f"--kconfig-add={path}" in added[0] + + @pytest.mark.parametrize("present", [True, False]) + def test_tuxmake_adds_a_tree_file_only_when_present( + self, tmp_path, present + ): + kbuild = _kbuild(tmp_path) + kbuild._kconfig_adds = ["tree:tools/testing/selftests/arm64/config"] + path = os.path.join( + kbuild._srcdir, "tools/testing/selftests/arm64/config" + ) + if present: + os.makedirs(os.path.dirname(path)) + with open(path, "w") as f: + f.write("CONFIG_X=y\n") + parts = kbuild._tuxmake_base(kbuild._af_dir, "defconfig", []) + added = next(p for p in parts if path in p) + + result = subprocess.run( + [ + "bash", + "-c", + f"set -eE -o pipefail; set -- {added}; " + 'echo $#; for a; do echo "$a"; done', + ], + capture_output=True, + text=True, + ) + + assert result.returncode == 0 + expected = [f"--kconfig-add={path}"] if present else [] + assert result.stdout.splitlines() == [str(len(expected))] + expected + class TestKselftestSuiteResults: def test_names_identify_build_results(self, tmp_path):