From c0662aac45adb7949a6d67a4906099d56cf415dd Mon Sep 17 00:00:00 2001 From: akritkbehera Date: Fri, 9 Oct 2026 12:56:34 +0200 Subject: [PATCH 1/2] Fixes to Python Variables and option to set strict expansion from defaults since it's required for all packages in CMS --- bits_helpers/build.py | 22 +++++----- bits_helpers/status.py | 3 +- docs/REFERENCE.md | 1 + tests/test_deps_info.py | 92 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 12 deletions(-) create mode 100644 tests/test_deps_info.py diff --git a/bits_helpers/build.py b/bits_helpers/build.py index 514499a3..b9a7f923 100644 --- a/bits_helpers/build.py +++ b/bits_helpers/build.py @@ -137,7 +137,7 @@ def defaults_store_url(defaults_meta): def prepare_hash_inputs(spec, specs, defaults, default_vars, config_dir, branch_basename="", branch_stream="", devel=None, - devel_version=None) -> None: + devel_version=None, default_expand_recipe=False) -> None: """Resolve what storeHashes() hashes for *spec*: tag and commit, version, sources, patches, variables and recipe text. `bits build` and `bits status` both use it, so that they compute the same package identity. @@ -182,7 +182,7 @@ def prepare_hash_inputs(spec, specs, defaults, default_vars, config_dir, spec.setdefault("variables", OrderedDict(spec.get("variables", {}))) variables = spec["variables"] - if "Python" in spec.get("requires", []): + if "Python" in spec.get("requires", []) or spec["package"] == "Python": # Find the Python package spec safely python_version_str = "" py_spec = specs.get("Python") @@ -204,21 +204,20 @@ def prepare_hash_inputs(spec, specs, defaults, default_vars, config_dir, "python_major_minor_str": f"{major}{minor}", }) for k, v in variables.items(): - variables[k] = resolve_spec_data(spec, v, defaults, branch_basename, branch_stream) + variables[k] = resolve_spec_data(spec, v if isinstance(v, str) else str(v), + args.defaults, branch_basename, branch_stream) if "source" in spec: spec["source"] = resolve_spec_data(spec, spec["source"], defaults, branch_basename, branch_stream) if "sources" in spec: spec["sources"] = [resolve_spec_data(spec, src, defaults, branch_basename, branch_stream) for src in spec["sources"]] if "patches" in spec: spec["patches"] = [resolve_spec_data(spec, p, defaults, branch_basename, branch_stream) for p in spec["patches"]] - # Variables defined in the active --defaults profile's `variables:` block are - # available to every recipe body. When a recipe does not itself opt into - # expansion (no `variables:` / `expand_recipe: true`) we expand it in SOFT - # mode: only known variables are substituted and any other %(...)s / bare % - # is left untouched, so profile-wide variables never clobber or break a - # recipe that happens to contain a literal %(...)s or shell `%`. + # Defaults can opt every recipe into strict expansion with + # `expand_recipe: true`. Recipe-local variables/flag do the same for one + # recipe; otherwise profile variables are applied in soft mode. default_vars = default_vars or None - recipe_opts_in = bool(variables or spec.get("expand_recipe", False)) + recipe_opts_in = bool(variables or spec.get("expand_recipe", False) + or default_expand_recipe) if recipe_opts_in or default_vars: spec["recipe"] = resolve_spec_data(spec, spec["recipe"], defaults, branch_basename, branch_stream, @@ -3818,7 +3817,8 @@ def _devel(spec): spec, specs, args.defaults, defaultsMeta.get("variables"), args.configDir, branch_basename, branch_stream, devel=_devel, devel_version=(args.develPrefix if "develPrefix" in args - and args.develPrefix != "ali-master" else None)) + and args.develPrefix != "ali-master" else None), + default_expand_recipe=defaultsMeta.get("expand_recipe", False)) # Decide what is the main package we are building and at what commit. # diff --git a/bits_helpers/status.py b/bits_helpers/status.py index 5cf3479e..0c74da7d 100644 --- a/bits_helpers/status.py +++ b/bits_helpers/status.py @@ -616,7 +616,8 @@ def devel(spec): # makes status look under a different store key from the build. prepare_hash_inputs( spec, specs, args.defaults, defaults_meta.get("variables"), - args.configDir, branch_basename, branch_stream, devel=devel) + args.configDir, branch_basename, branch_stream, devel=devel, + default_expand_recipe=defaults_meta.get("expand_recipe", False)) # Compute build hashes (same as doBuild main loop) consider_relocation = ( diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index c18cde59..efd7a478 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -2573,6 +2573,7 @@ All sections are optional. `commits` maps a git `tag:` to the **pinned commit SH | `incremental_recipe` | Bash snippet for fast incremental (development) rebuilds. | | `relocate_paths` | Paths to rewrite when relocating an installation. | | `variables` | Custom key-value pairs for `%(name)s` substitution in `version`, `tag`, `source`, `sources`, `patches` and the recipe body. Setting it (or `expand_recipe: true`) makes an unknown `%(name)s` in the body an error; otherwise only known variables are replaced there. | +| `expand_recipe` | Set to `true` in a defaults profile to enable strict `%(name)s` expansion for every recipe body under that profile. Unknown placeholders are errors. A recipe can also set this flag locally. | | `from` | Recipe inheritance: names a recipe directory (relative to the recipes root) whose recipe with the same file name is the parent. The child's header keys replace the parent's and its body is placed before the parent's; `merge_policy:` (`remove`, `inherit`, `merge` key lists) adjusts this. | | `architecture` | Set to `share` to mark a package as architecture-independent (see [§19](#19-architecture-independent-shared-packages)). The older spelling `shared` is no longer recognised. | | `own_hash` | Set to `true` for a package whose output does not depend on the community/build-type defaults — the compiler toolchain — so one build is reused across them. See [Shared toolchains](#shared-toolchains-own_hash) below. | diff --git a/tests/test_deps_info.py b/tests/test_deps_info.py new file mode 100644 index 00000000..1ee12ac1 --- /dev/null +++ b/tests/test_deps_info.py @@ -0,0 +1,92 @@ +# SPDX-FileCopyrightText: 2026 CERN +# SPDX-License-Identifier: GPL-3.0-or-later + +"""create_deps_info(): the runtime dependency graph recorded in each package's +.meta.json as the dependency_graph field.""" + +import json +import os +from types import SimpleNamespace +import unittest + +from bits_helpers.build import create_deps_info, create_provenance_info + + +def _spec(name, **kw): + base = { + "package": name, "version": "1.0", "revision": "1", "hash": "h" + name, + "tag": None, "source": None, "requires": [], + "build_requires": [], "runtime_requires": [], + "full_build_requires": [], "full_runtime_requires": [], + } + base.update(kw) + return base + + +def _specs(): + # c -> b -> a, plus a build-only dep the runtime edges must omit. + return { + "defaults-release": _spec("defaults-release"), + "a": _spec("a"), + "tools": _spec("tools"), + "b": _spec("b", runtime_requires=["a"], requires=["a"], + full_runtime_requires=["a"]), + "c": _spec("c", runtime_requires=["b"], build_requires=["tools"], + requires=["b", "tools"], full_runtime_requires=["a", "b"]), + } + + +_ARGS = SimpleNamespace(architecture="arch", defaults=["release"]) + + +class TestDepsInfo(unittest.TestCase): + + def test_mapping_in_build_order(self): + graph = create_deps_info("c", _specs(), _ARGS) + self.assertEqual(graph, {"defaults-release": [], + "a": ["defaults-release"], + "b": ["defaults-release", "a"], + "c": ["defaults-release", "b"]}) + self.assertEqual(list(graph), ["defaults-release", "a", "b", "c"]) + + def test_same_edges_as_bits_deps_outmake(self): + from bits_helpers.deps import deps_makefile + rules = {line.split(":")[0]: line.partition(":")[2].split() + for line in deps_makefile(_specs(), "c", runtime_only=True).splitlines() + if not line.startswith("#")} + self.assertEqual(create_deps_info("c", _specs(), _ARGS), rules) + + def test_build_only_dependency_excluded(self): + self.assertNotIn("tools", create_deps_info("c", _specs(), _ARGS)) + + +class TestGraphInMeta(unittest.TestCase): + """The graph travels in .meta.json as its own last field.""" + + def _record(self): + args = SimpleNamespace(annotate={}, architecture="arch", + defaults=["release"], reusePolicy="strict") + os.environ["BITS_DIST_HASH"] = "deadbeef" + try: + return json.loads(create_provenance_info("c", _specs(), args)) + finally: + os.environ.pop("BITS_DIST_HASH", None) + + def test_recorded_as_last_field(self): + rec = self._record() + self.assertEqual(list(rec)[-1], "dependency_graph") + self.assertEqual(rec["dependency_graph"]["c"], ["defaults-release", "b"]) + + def test_dependencies_block_untouched(self): + deps = self._record()["dependencies"] + self.assertEqual(sorted(deps), ["direct", "recursive"]) + self.assertEqual([e["name"] for e in deps["direct"]["runtime"]], ["b"]) + + def test_covers_the_recursive_closure(self): + rec = self._record() + closure = {e["name"] for e in rec["dependencies"]["recursive"]["runtime"]} + self.assertTrue(closure <= set(rec["dependency_graph"])) + + +if __name__ == "__main__": + unittest.main() From fad19b0bec69ef4a0222d730b3f611358114d632 Mon Sep 17 00:00:00 2001 From: akritkbehera Date: Fri, 9 Oct 2026 13:22:49 +0200 Subject: [PATCH 2/2] Fix relocation of staged package paths --- bits_helpers/build.py | 17 ++++++++++++++++- bits_helpers/relocate-me.sh | 6 ++++-- pyproject.toml | 4 ++++ tests/test_deps_info.py | 9 +++------ 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/bits_helpers/build.py b/bits_helpers/build.py index b9a7f923..0f6c0ad7 100644 --- a/bits_helpers/build.py +++ b/bits_helpers/build.py @@ -205,7 +205,7 @@ def prepare_hash_inputs(spec, specs, defaults, default_vars, config_dir, }) for k, v in variables.items(): variables[k] = resolve_spec_data(spec, v if isinstance(v, str) else str(v), - args.defaults, branch_basename, branch_stream) + defaults, branch_basename, branch_stream) if "source" in spec: spec["source"] = resolve_spec_data(spec, spec["source"], defaults, branch_basename, branch_stream) if "sources" in spec: @@ -1226,6 +1226,21 @@ def _closure_untracked(): }) +def create_deps_info(package, specs, args): + """Return the runtime dependency graph in dependency-first build order. + + Uses the same edge selection as ``bits deps --outmake --runtime``. The + virtual ``defaults-release`` node is omitted, matching ``deps_graph`` and + the graph stored in package metadata. + """ + from bits_helpers.deps import _makefile_order, deps_graph + + graph = deps_graph(specs, package, runtime_only=True) + order = _makefile_order(graph) + position = {name: index for index, name in enumerate(order)} + return {name: sorted(graph[name], key=position.__getitem__) for name in order} + + # High-signal patterns that usually pinpoint the proximate cause of a build # failure. Used to surface a short excerpt in the failure message so users do # not have to open and grep the full (often huge) log by hand. diff --git a/bits_helpers/relocate-me.sh b/bits_helpers/relocate-me.sh index 65df7325..e0252262 100644 --- a/bits_helpers/relocate-me.sh +++ b/bits_helpers/relocate-me.sh @@ -16,11 +16,13 @@ if [[ -s ${THISDIR}/etc/profile.d/.bits-relocate ]] ; then # goes to directly, no arch/pkg/ver wrapper) sets # BITS_RELOCATE_STRIP_PP=1 so the full …/INSTALLROOT/$PH/$PP prefix collapses to # INSTALL_BASE and the paths don't gain a doubled $PP. + # First reduce staging prefixes to PKG_DIR, then relocate PKG_DIR. + # This avoids rewriting INSTALL_BASE again if it contains PKG_DIR. _pp_strip="" - [ -n "${BITS_RELOCATE_STRIP_PP:-}" ] && _pp_strip="s|${PKG_DIR}/INSTALLROOT/$PH/$PP|$INSTALL_BASE|g;" + [ -n "${BITS_RELOCATE_STRIP_PP:-}" ] && _pp_strip="s|${PKG_DIR}/INSTALLROOT/$PH/$PP|${PKG_DIR}|g;" while IFS= read -r f; do [[ -z "$f" ]] && continue - sed -i.unrelocated -e "${_pp_strip}s|${PKG_DIR}/INSTALLROOT/$PH|$INSTALL_BASE|g;s|${PKG_DIR}|$INSTALL_BASE|g" "${THISDIR}/$f" + sed -i.unrelocated -e "${_pp_strip}s|${PKG_DIR}/INSTALLROOT/$PH|${PKG_DIR}|g;s|${PKG_DIR}|$INSTALL_BASE|g" "${THISDIR}/$f" rm -f "${THISDIR}/${f}.unrelocated" done < "${THISDIR}/etc/profile.d/.bits-relocate" fi diff --git a/pyproject.toml b/pyproject.toml index 7f477a9a..8de0cb33 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,6 +68,10 @@ exclude = ["yaml"] [tool.setuptools.dynamic] readme = {file = ["README.md"], content-type = "text/markdown"} +[tool.pytest.ini_options] +testpaths = ["tests"] +pythonpath = ["."] + [tool.ruff] # Code formatter/linter — adopted per issue #59 (consensus: ruff). # Match the historical .flake8 line length. NOTE: the codebase's hand-aligned diff --git a/tests/test_deps_info.py b/tests/test_deps_info.py index 1ee12ac1..3f0d5fb5 100644 --- a/tests/test_deps_info.py +++ b/tests/test_deps_info.py @@ -43,11 +43,8 @@ class TestDepsInfo(unittest.TestCase): def test_mapping_in_build_order(self): graph = create_deps_info("c", _specs(), _ARGS) - self.assertEqual(graph, {"defaults-release": [], - "a": ["defaults-release"], - "b": ["defaults-release", "a"], - "c": ["defaults-release", "b"]}) - self.assertEqual(list(graph), ["defaults-release", "a", "b", "c"]) + self.assertEqual(graph, {"a": [], "b": ["a"], "c": ["b"]}) + self.assertEqual(list(graph), ["a", "b", "c"]) def test_same_edges_as_bits_deps_outmake(self): from bits_helpers.deps import deps_makefile @@ -75,7 +72,7 @@ def _record(self): def test_recorded_as_last_field(self): rec = self._record() self.assertEqual(list(rec)[-1], "dependency_graph") - self.assertEqual(rec["dependency_graph"]["c"], ["defaults-release", "b"]) + self.assertEqual(rec["dependency_graph"]["c"], ["b"]) def test_dependencies_block_untouched(self): deps = self._record()["dependencies"]