From a278147f211aad4730fec9bfb8a8eed7b6e07847 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 14:42:44 +0100 Subject: [PATCH 1/8] rpm: Move the staging of a spec out of build.py build.py stages the spec and builds the rpmbuild command line inline. The BuildRequires query needs to do the same, so let's move both into rpmbuild.py. Pinned packaging now comes in as a directory in sources, and a strict_subpackages flag replaces the packaging field. Signed-off-by: Daan De Meyer --- docs/design/architecture.md | 5 +- package_system/rpm/BUCK | 6 +- package_system/rpm/build.py | 110 ++++------------------ package_system/rpm/build_test.py | 68 +++++++++++-- package_system/rpm/rpmbuild.py | 157 +++++++++++++++++++++++++++++++ package_system/rpm/rules.bzl | 19 +++- 6 files changed, 259 insertions(+), 106 deletions(-) create mode 100644 package_system/rpm/rpmbuild.py diff --git a/docs/design/architecture.md b/docs/design/architecture.md index ba7dc40b..912d8951 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -746,8 +746,9 @@ names none, and gets no per-release aliases either. - Debuginfo/debugsource outputs are not first-class declared sub-targets. - The bootstrap extractor supports the pinned v4/newc payload form, not RPM v6 metadata. -Entry points: `package_system/rpm/{rules,catalog,generated}.bzl` and -`package_system/rpm/{snapshot,plan,install,pkgdb,createrepo,build,extract,rpmfile}.py`. +Entry points: `package_system/rpm/{rules,catalog,generated}.bzl`, +`package_system/rpm/{snapshot,plan,install,pkgdb,createrepo,build,extract,rpmfile}.py` and +`package_system/rpm/rpmbuild.py`. ### The alpm package system diff --git a/package_system/rpm/BUCK b/package_system/rpm/BUCK index 389a2a3d..a4ccce6f 100644 --- a/package_system/rpm/BUCK +++ b/package_system/rpm/BUCK @@ -38,13 +38,15 @@ python.binary( deps = [":macros", "//:specs", "//:util", "//package:transaction", "//rootfs:rootfs"], ) -python.binary(name = "build", main = "build.py", deps = ["//:specs", "//:util", "//package:builder", "//package:subpackage"]) +python.library(name = "rpmbuild", srcs = ["rpmbuild.py"], deps = ["//:util", "//package:builder"]) + +python.binary(name = "build", main = "build.py", deps = [":rpmbuild", "//:specs", "//:util", "//package:builder", "//package:subpackage"]) python.test( name = "build-test", box = _BOX, srcs = ["build.py", "build_test.py"], - deps = ["//:specs", "//:util", "//package:builder", "//package:subpackage"], + deps = [":rpmbuild", "//:specs", "//:util", "//package:builder", "//package:subpackage"], ) python.binary( diff --git a/package_system/rpm/build.py b/package_system/rpm/build.py index 8a468fac..1f82a80d 100755 --- a/package_system/rpm/build.py +++ b/package_system/rpm/build.py @@ -20,24 +20,16 @@ import util import builder +import rpmbuild import subpackage -# A persistent build directory marks a local iteration build; skip the release-only costs of -# optimization, debug packaging, and payload compression. -_INCREMENTAL_RPMBUILD_OPTIONS = [ - "--without", "lto", - "--undefine", "_lto_cflags", - "--undefine", "_annotated_build", - "--define", "debug_package %{nil}", - "--define", "_binary_payload w.ufdio", - "--define", "_source_payload w.ufdio", -] # fmt: skip - class Spec(builder.Spec): """Buck's generated build invocation, distinct from the package's RPM spec.""" spec_file: str + # `sources` lists the Source and Patch files, and directories whose top-level files are Source and Patch + # files. sources: list[str] dist: str source_date_epoch: int @@ -45,9 +37,9 @@ class Spec(builder.Spec): # Extra switches for a build against `source_tree`, in addition to the common ones below. in_place_rpmbuild_options: list[str] rpmbuild_options: list[str] - # `packaging` is the dist-git tree at the commit that `source_tree` pins, or None. Its files replace - # `sources`. - packaging: str | None + # `strict_subpackages` fails the build when the produced subpackages differ from the declared ones. The + # spec of pinned packaging can declare other subpackages than the imported metadata. + strict_subpackages: bool def build_rpm(spec: Spec) -> int: @@ -56,96 +48,32 @@ def build_rpm(spec: Spec) -> int: def _build_rpm(spec: Spec, topdir: Path, out: Path, subpackages: Path) -> int: - for d in ("SOURCES", "SPECS", "BUILDROOT", "RPMS", "SRPMS"): - (topdir / d).mkdir(parents=True) - # readonly_project() already created BUILD if it mounted the persistent build directory there. - (topdir / builder.BUILD).mkdir(exist_ok=True) - spec_file = Path(spec["spec_file"]) source_tree = spec["source_tree"] - checkout = topdir / builder.CHECKOUT - if source_tree is not None and spec_file.is_relative_to(source_tree): - relative_spec = spec_file.relative_to(source_tree) - if ".." in relative_spec.parts: - util.fail(f"build_rpm: RPM spec must stay within the source tree: {relative_spec}") - staged_spec = checkout / relative_spec - # Freezing runs before chroot: an escaping spec symlink must not write into the checkout - # through another path in the outer sandbox. - if not staged_spec.resolve().is_relative_to(checkout.resolve()): - util.fail(f"build_rpm: RPM spec symlink escapes the source tree: {relative_spec}") - source_spec = staged_spec - chroot_spec = builder.CHROOT_TOPDIR / builder.CHECKOUT / relative_spec - sourcedir = chroot_spec.parent - else: - source_spec = spec_file - staged_spec = topdir / "SPECS" / spec_file.name - chroot_spec = builder.CHROOT_TOPDIR / "SPECS" / spec_file.name - sourcedir = None - sources = [Path(src) for src in spec["sources"]] - if spec["packaging"] is not None: - # rpmbuild -bb does not check that Source and Patch files exist, but the spec's scriptlets - # and macros read them from SOURCES. A dist-git repository contains them at its top level, - # and its subdirectories, such as tests/, contain no sources. - sources = sorted(path for path in Path(spec["packaging"]).iterdir() if path.is_file()) - for s in sources: - # A spec may modify SOURCES, so it must not share the source artifact's inode. - util.clone_file(s, topdir / "SOURCES" / s.name) - - if not source_spec.is_file(): - util.fail(f"build_rpm: RPM spec does not exist: {spec_file}") - - # Freeze rpmautospec macros so builds need neither Git nor rpmautospec. Keep an in-place spec beside - # its auxiliary files in the disposable tree, preserving relative includes as well. - frozen = ( - f"%global autorelease {spec['release']}%{{?dist}}\n%global autochangelog %{{nil}}\n" - ) + source_spec.read_text() - staged_spec.write_text(frozen) + staged = rpmbuild.stage(topdir, Path(spec["spec_file"]), spec["sources"], source_tree, spec["release"]) incremental = spec["build_dir"] is not None - # The ephemeral upper discards buildroot writes; use the package-specific epoch. - env = os.environ | { - "HOME": str(builder.CHROOT_TOPDIR), - "SOURCE_DATE_EPOCH": str(spec["source_date_epoch"]), - } + env = os.environ | rpmbuild.environment(spec["source_date_epoch"]) with builder.buildroot(spec, topdir): - defines = [ - "--define", f"_topdir {builder.CHROOT_TOPDIR}", - "--define", f"dist {spec['dist']}", - "--define", "_buildhost reproducible", - # rpm otherwise ignores SOURCE_DATE_EPOCH for the BUILDTIME header. - "--define", "use_source_date_epoch_as_buildtime 1", - ] # fmt: skip - if sourcedir is not None: - defines += ["--define", f"_sourcedir {sourcedir}"] - if incremental: - defines += ["--define", f"_vpath_builddir {builder.CHROOT_TOPDIR / builder.BUILD}"] + arguments = rpmbuild.arguments( + staged, + dist=spec["dist"], + incremental=incremental, + rpmbuild_options=spec["rpmbuild_options"], + in_place_rpmbuild_options=spec["in_place_rpmbuild_options"], + ) mode = ["-ba"] - options = spec["rpmbuild_options"] + (_INCREMENTAL_RPMBUILD_OPTIONS if incremental else []) - cwd = None + cwd = staged.cwd if source_tree is not None: mode = ["-bb", "--noprep", "--build-in-place"] - options += spec["in_place_rpmbuild_options"] - cwd = builder.CHROOT_TOPDIR / builder.CHECKOUT - # rpm runs an in-place build in %{builddir}/%{NAME}-%{VERSION}, a symlink to the checkout. - # find-debuginfo collects the sources under the path of the symlink only. A compiler that ninja - # or make -C starts in a subdirectory records the real path of the checkout instead. The - # debugsource package then has no files, and rpmbuild fails. rpm expands the flags when it parses - # the spec, before it defines %{buildsubdir}. The map therefore names the symlink with the macros - # that rpm uses to create it. Fedora appends the _distro_extra_* macros to %{build_cflags}, - # %{build_cxxflags} and %{build_fflags} only. A compiler that a spec runs with %{optflags} or - # $RPM_OPT_FLAGS still records the real path. rustc records the real path as well. - prefix_map = f"-fdebug-prefix-map={cwd}=%{{builddir}}/%{{NAME}}-%{{VERSION}}" - for lang in ("cflags", "cxxflags", "fflags"): - defines += ["--define", f"_distro_extra_{lang} {prefix_map}"] rc = subprocess.run( [ "/usr/bin/rpmbuild", - *defines, - *options, + *arguments, *mode, "--nocheck", "--noclean", - str(chroot_spec), + str(staged.spec), ], cwd=cwd, env=env, @@ -170,7 +98,7 @@ def _build_rpm(spec: Spec, topdir: Path, out: Path, subpackages: Path) -> int: version=r"-[^-]+-[^-]+\.[^.]+\.rpm", # rpmbuild adds debuginfo and debugsource packages on its own. generated=r"-debug(info|source)-", - strict=spec["packaging"] is None, + strict=spec["strict_subpackages"], ) return 0 diff --git a/package_system/rpm/build_test.py b/package_system/rpm/build_test.py index 32c9f74a..8c6ca276 100644 --- a/package_system/rpm/build_test.py +++ b/package_system/rpm/build_test.py @@ -41,24 +41,26 @@ def specification( build_dir: Path | None = None, spec_file: Path | None = None, subpackages: list[str] | None = None, - packaging: Path | None = None, + sources: list[Path] | None = None, + strict_subpackages: bool = True, ) -> build.Spec: """Return a complete driver spec for one test build.""" return build.Spec( build_dir=str(build_dir) if build_dir is not None else None, lower=["buildroot"], spec_file=str(spec_file or self.spec_file), - sources=[str(self.auxiliary)], + sources=[str(source) for source in sources] if sources is not None else [str(self.auxiliary)], dist=".test", source_date_epoch=1234567890, release="7", out=str(self.project / "out"), subpackages=subpackages or [], subpackages_out=str(self.project / "subpackages-out"), - in_place_rpmbuild_options=["--define", "local_option yes"], + # The rule passes the in-place options only to a build with a source tree. + in_place_rpmbuild_options=["--define", "local_option yes"] if source_tree is not None else [], rpmbuild_options=["--define", "common_option yes"], source_tree=str(source_tree) if source_tree is not None else None, - packaging=str(packaging) if packaging is not None else None, + strict_subpackages=strict_subpackages, ) def invoke( @@ -190,7 +192,9 @@ def pinned_packaging(self) -> tuple[Path, Path]: def test_pinned_packaging_replaces_the_declared_spec_and_sources(self) -> None: source, packaging = self.pinned_packaging() - specification = self.specification(source, spec_file=packaging / "example.spec", packaging=packaging) + specification = self.specification( + source, spec_file=packaging / "example.spec", sources=[packaging], strict_subpackages=False + ) command, _cwd, _environment, topdir, _binds = self.invoke(specification) @@ -202,6 +206,57 @@ def test_pinned_packaging_replaces_the_declared_spec_and_sources(self) -> None: ) self.assertEqual((topdir / "SOURCES" / "fix.patch").read_text(), "pinned patch\n") + def test_a_symlink_at_the_top_of_a_source_directory_fails(self) -> None: + _source, packaging = self.pinned_packaging() + (packaging / "secret").symlink_to(self.auxiliary) + with self.assertRaisesRegex(SystemExit, "contains symlinks at its top level: secret"): + self.invoke(self.specification(spec_file=packaging / "example.spec", sources=[packaging])) + + def test_a_symlinked_source_directory_fails(self) -> None: + _source, packaging = self.pinned_packaging() + linked = self.scratch / "linked" + linked.symlink_to(packaging) + with self.assertRaisesRegex(SystemExit, "is a symlink"): + self.invoke(self.specification(spec_file=packaging / "example.spec", sources=[linked])) + + def test_a_source_directory_beneath_a_symlink_fails(self) -> None: + tree = self.project / "tree" + tree.mkdir() + external = self.scratch / "external" + (external / "nested").mkdir(parents=True) + (external / "nested" / "helper").write_text("external input\n") + (tree / "rpm").symlink_to(external, target_is_directory=True) + with self.assertRaisesRegex(SystemExit, "the source is a symlink"): + self.invoke(self.specification(sources=[tree / "rpm" / "nested"])) + + def test_a_spec_beneath_a_symlink_fails(self) -> None: + linked = self.project / "linked" + linked.symlink_to(self.scratch, target_is_directory=True) + with self.assertRaisesRegex(SystemExit, "RPM spec is a symlink"): + self.invoke(self.specification(spec_file=linked / self.spec_file.name)) + + def test_a_symlinked_spec_fails(self) -> None: + linked = self.scratch / "linked.spec" + linked.symlink_to(self.spec_file) + with self.assertRaisesRegex(SystemExit, "RPM spec is a symlink"): + self.invoke(self.specification(spec_file=linked)) + + def test_identical_sources_with_one_name_are_copied_once(self) -> None: + other = self.scratch / "other" + other.mkdir() + (other / "auxiliary").write_text("packaging input\n") + _command, _cwd, _environment, topdir, _binds = self.invoke( + self.specification(sources=[self.auxiliary, other]) + ) + self.assertEqual([path.name for path in (topdir / "SOURCES").iterdir()], ["auxiliary"]) + + def test_different_sources_with_one_name_fail(self) -> None: + other = self.scratch / "other" + other.mkdir() + (other / "auxiliary").write_text("something else\n") + with self.assertRaisesRegex(SystemExit, "are different sources with the name auxiliary"): + self.invoke(self.specification(sources=[self.auxiliary, other])) + def produce(self, *names: str) -> Callable[[Path], None]: def produce(topdir: Path) -> None: built = topdir / "RPMS/x86_64" @@ -216,7 +271,8 @@ def test_pinned_packaging_reports_a_subpackage_mismatch_without_failing(self) -> specification = self.specification( source, spec_file=packaging / "example.spec", - packaging=packaging, + sources=[packaging], + strict_subpackages=False, subpackages=["example", "example-imds"], ) diff --git a/package_system/rpm/rpmbuild.py b/package_system/rpm/rpmbuild.py new file mode 100644 index 00000000..dd1cc48b --- /dev/null +++ b/package_system/rpm/rpmbuild.py @@ -0,0 +1,157 @@ +# SPDX-FileCopyrightText: Amutable GmbH +# SPDX-License-Identifier: MPL-2.0 + +"""Stage an RPM spec in an rpmbuild tree, and assemble the arguments that rpmbuild parses it with.""" + +import filecmp +from pathlib import Path +from typing import NamedTuple + +import util + +import builder + +# A persistent build directory means a local iteration build. Such a build skips optimization, debug +# packages and payload compression, which only a release needs. +INCREMENTAL_OPTIONS = [ + "--without", "lto", + "--undefine", "_lto_cflags", + "--undefine", "_annotated_build", + "--define", "debug_package %{nil}", + "--define", "_binary_payload w.ufdio", + "--define", "_source_payload w.ufdio", +] # fmt: skip + + +class Staged(NamedTuple): + # `spec` is the path of the frozen spec in the chroot. + spec: Path + # `sourcedir` is the %_sourcedir of a spec in the source tree, or None for the default SOURCES. + sourcedir: Path | None + # `cwd` is the directory that rpmbuild runs in, or None for the root of the chroot. + cwd: Path | None + + +def reject_symlinks(path: Path, what: str) -> None: + for component in (path, *path.parents): + if component.is_symlink(): + util.fail(f"rpmbuild: {what} is a symlink: {component}") + + +def source_files(paths: list[str]) -> list[Path]: + """Return the Source and Patch files of `paths`, with each directory replaced by its top-level files. + + A dist-git repository contains the sources at its top level. Its subdirectories, such as tests/, + contain no sources. + """ + files: list[Path] = [] + for path in (Path(path) for path in paths): + # The copy into SOURCES follows symlinks, and it runs outside the chroot where the project is + # readable. A symlink in a fetched or unpacked tree, or a sub-target of one, would put a file of + # the project into the package. + reject_symlinks(path, "the source") + if not path.is_dir(): + files.append(path) + continue + if links := sorted(file.name for file in path.iterdir() if file.is_symlink()): + util.fail(f"rpmbuild: {path} contains symlinks at its top level: {', '.join(links)}") + files += sorted(file for file in path.iterdir() if file.is_file()) + chosen: dict[str, Path] = {} + for file in files: + other = chosen.setdefault(file.name, file) + # Overlapping directories and files in srcs can name the same source. rpmbuild finds a source + # by its file name, so only two different files with one name are an error. + if other != file and not filecmp.cmp(other, file, shallow=False): + util.fail(f"rpmbuild: {other} and {file} are different sources with the name {file.name}") + return list(chosen.values()) + + +def stage( + topdir: Path, spec_file: Path, sources: list[str], source_tree: str | None, release: str +) -> Staged: + """Stage `spec_file` and `sources` in the rpmbuild tree `topdir`. + + A source tree has to be mounted at `topdir/CHECKOUT` already. + """ + for d in ("SOURCES", "SPECS", "BUILDROOT", "RPMS", "SRPMS"): + (topdir / d).mkdir(parents=True) + # readonly_project() already created BUILD if it mounted the persistent build directory there. + (topdir / builder.BUILD).mkdir(exist_ok=True) + checkout = topdir / builder.CHECKOUT + if source_tree is not None and spec_file.is_relative_to(source_tree): + relative_spec = spec_file.relative_to(source_tree) + if ".." in relative_spec.parts: + util.fail(f"rpmbuild: RPM spec must stay within the source tree: {relative_spec}") + staged_spec = checkout / relative_spec + # The spec is frozen before the chroot. A spec symlink that escapes the checkout would otherwise + # make the freeze write to a file of the outer sandbox. + if not staged_spec.resolve().is_relative_to(checkout.resolve()): + util.fail(f"rpmbuild: RPM spec symlink escapes the source tree: {relative_spec}") + source_spec = staged_spec + chroot_spec = builder.CHROOT_TOPDIR / builder.CHECKOUT / relative_spec + sourcedir = chroot_spec.parent + else: + # The spec is read outside the chroot. A spec symlink in a fetched tree would read a file of the + # project. + reject_symlinks(spec_file, "RPM spec") + source_spec = spec_file + staged_spec = topdir / "SPECS" / spec_file.name + chroot_spec = builder.CHROOT_TOPDIR / "SPECS" / spec_file.name + sourcedir = None + for s in source_files(sources): + # A spec may modify SOURCES, so it must not share the source artifact's inode. + util.clone_file(s, topdir / "SOURCES" / s.name) + + if not source_spec.is_file(): + util.fail(f"rpmbuild: RPM spec does not exist: {spec_file}") + + # The frozen rpmautospec macros let the spec parse without Git or rpmautospec. An in-place spec stays + # beside its auxiliary files in the disposable tree, so that its relative includes still resolve. + staged_spec.write_text( + f"%global autorelease {release}%{{?dist}}\n%global autochangelog %{{nil}}\n{source_spec.read_text()}" + ) + cwd = builder.CHROOT_TOPDIR / builder.CHECKOUT if source_tree is not None else None + return Staged(chroot_spec, sourcedir, cwd) + + +def environment(source_date_epoch: int) -> dict[str, str]: + return {"HOME": str(builder.CHROOT_TOPDIR), "SOURCE_DATE_EPOCH": str(source_date_epoch)} + + +def arguments( + staged: Staged, + *, + dist: str, + incremental: bool, + rpmbuild_options: list[str], + in_place_rpmbuild_options: list[str], +) -> list[str]: + """Return the macro definitions and options that rpmbuild parses the spec `staged` with. + + The options of a dev-mode build follow `rpmbuild_options`, and `in_place_rpmbuild_options` come last. + """ + arguments: list[str] = [ + "--define", f"_topdir {builder.CHROOT_TOPDIR}", + "--define", f"dist {dist}", + "--define", "_buildhost reproducible", + # rpm otherwise ignores SOURCE_DATE_EPOCH for the BUILDTIME header. + "--define", "use_source_date_epoch_as_buildtime 1", + ] # fmt: skip + if staged.sourcedir is not None: + arguments += ["--define", f"_sourcedir {staged.sourcedir}"] + if incremental: + arguments += ["--define", f"_vpath_builddir {builder.CHROOT_TOPDIR / builder.BUILD}"] + if staged.cwd is not None: + # rpm runs an in-place build in %{builddir}/%{NAME}-%{VERSION}, a symlink to the checkout. + # find-debuginfo collects the sources under the path of the symlink only. A compiler that ninja + # or make -C starts in a subdirectory records the real path of the checkout instead. The + # debugsource package then has no files, and rpmbuild fails. rpm expands the flags when it parses + # the spec, before it defines %{buildsubdir}. The map therefore names the symlink with the macros + # that rpm uses to create it. Fedora appends the _distro_extra_* macros to %{build_cflags}, + # %{build_cxxflags} and %{build_fflags} only. A compiler that a spec runs with %{optflags} or + # $RPM_OPT_FLAGS still records the real path. rustc records the real path as well. + prefix_map = f"-fdebug-prefix-map={staged.cwd}=%{{builddir}}/%{{NAME}}-%{{VERSION}}" + for lang in ("cflags", "cxxflags", "fflags"): + arguments += ["--define", f"_distro_extra_{lang} {prefix_map}"] + arguments += rpmbuild_options + (INCREMENTAL_OPTIONS if incremental else []) + return arguments + in_place_rpmbuild_options diff --git a/package_system/rpm/rules.bzl b/package_system/rpm/rules.bzl index 32fd8402..60b6a374 100644 --- a/package_system/rpm/rules.bzl +++ b/package_system/rpm/rules.bzl @@ -47,9 +47,9 @@ def _rpm_package_impl(ctx: AnalysisContext) -> list[Provider]: spec_file = ctx.attrs.spec if source_tree != None and in_place_spec != None: spec_file = source_tree.project(in_place_spec) - packaging = None + pinned = None if source_tree != None and ctx.attrs.in_place_packaging != None: - packaging = pinned_packaging( + pinned = pinned_packaging( ctx, base_buildroot, "git-core", @@ -57,9 +57,15 @@ def _rpm_package_impl(ctx: AnalysisContext) -> list[Provider]: source_tree.project(ctx.attrs.in_place_packaging), ctx.attrs.in_place_packaging_distro, ) - spec_file = packaging.project(ctx.attrs.package + ".spec") + spec_file = pinned.project(ctx.attrs.package + ".spec") if spec_file == None: fail("rpm_package: a regular build requires a spec file") + sources = ctx.attrs.srcs + if pinned != None: + sources = [pinned] + elif in_place_spec != None: + # The build takes the Source and Patch files from the directory of the spec in the source tree. + sources = [] return package_build( ctx, @@ -76,13 +82,13 @@ def _rpm_package_impl(ctx: AnalysisContext) -> list[Provider]: spec = { "dist": ctx.attrs.dist, "in_place_rpmbuild_options": ctx.attrs.in_place_rpmbuild_options if source_tree != None else [], - "packaging": packaging, "release": ctx.attrs.release, "rpmbuild_options": ctx.attrs.rpmbuild_options, "source_date_epoch": ctx.attrs.source_date_epoch, "source_tree": source_tree, - "sources": ctx.attrs.srcs if in_place_spec == None and packaging == None else [], + "sources": sources, "spec_file": spec_file, + "strict_subpackages": pinned == None, }, ) @@ -111,6 +117,9 @@ _rpm_package = rule( doc = "extra rpmbuild CLI options (--with=..., --without=..., --define=...)", ), "spec": attrs.option(attrs.source(), default = None, doc = "the rpm spec file (derived from `package` by the macro)"), + "srcs": attrs.list( + attrs.source(allow_directory = True), default = [], doc = "the Source and Patch files of the spec, or directories whose top-level files are such files" + ), "_package_system": attrs.dep(providers = [PackageSystemInfo], default = PACKAGE_SYSTEM), }, supports_incoming_transition = True, From 004ec34d47b489e99c6adcfafa8027472c36279f Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 16:20:51 +0100 Subject: [PATCH 2/8] rpm: Stage an in-place spec outside the checkout We write the frozen copy of an in-place spec over the spec in the checkout, so staging needs a writable checkout. rpm resolves %include from its working directory, not from the spec, so let's write the copy to SPECS like any other spec. Signed-off-by: Daan De Meyer --- docs/user/importer.md | 4 ++-- package_system/rpm/build_test.py | 10 +++++++--- package_system/rpm/rpmbuild.py | 21 ++++++++++----------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/docs/user/importer.md b/docs/user/importer.md index 9fba0f31..0a9fc8fc 100644 --- a/docs/user/importer.md +++ b/docs/user/importer.md @@ -202,8 +202,8 @@ Buck ignores, and `%install` can copy them into the RPMs. See Three per-package maps in `_properties.json` can adapt the build to that checkout. `in_place_specs` selects an RPM spec by a path relative to the checkout root. The spec's directory also supplies its Source/Patch -files and relative includes. `in_place_packaging` takes the spec and its Source/Patch files from a dist-git -commit that the checkout pins; see [Taking the packaging from a pin in the +files. rpm opens a relative `%include` from the checkout root. `in_place_packaging` takes the spec and its +Source/Patch files from a dist-git commit that the checkout pins; see [Taking the packaging from a pin in the checkout](#taking-the-packaging-from-a-pin-in-the-checkout). A package cannot set both `in_place_specs` and `in_place_packaging`. `in_place_rpmbuild_options` adds CLI options after the common `rpmbuild_options` and any dev-mode defaults. diff --git a/package_system/rpm/build_test.py b/package_system/rpm/build_test.py index 8c6ca276..fada1052 100644 --- a/package_system/rpm/build_test.py +++ b/package_system/rpm/build_test.py @@ -160,11 +160,15 @@ def test_in_place_spec_and_extras_come_from_the_source_tree(self) -> None: specification = self.specification(source, spec_file=local_spec) def inspect(topdir: Path) -> None: - frozen = (topdir / "CHECKOUT" / "packaging" / "fedora" / "local.spec").read_text() + frozen = (topdir / "SPECS" / "local.spec").read_text() self.assertTrue( frozen.startswith("%global autorelease 7%{?dist}\n%global autochangelog %{nil}\n") ) self.assertIn("Name: from-checkout\n", frozen) + self.assertEqual( + (topdir / "CHECKOUT" / "packaging" / "fedora" / "local.spec").read_text(), + "Name: from-checkout\nSource1: helper\n", + ) self.assertEqual( (topdir / "CHECKOUT" / "packaging" / "fedora" / "helper").read_text(), "checkout helper\n", @@ -172,7 +176,7 @@ def inspect(topdir: Path) -> None: command, cwd, _environment, topdir, _binds = self.invoke(specification, inspect) - self.assertEqual(command[-1], "/build/CHECKOUT/packaging/fedora/local.spec") + self.assertEqual(command[-1], "/build/SPECS/local.spec") self.assertIn("_sourcedir /build/CHECKOUT/packaging/fedora", command) self.assertEqual(cwd, Path("/build/CHECKOUT")) self.assertEqual(list((topdir / "CHECKOUT").iterdir()), []) @@ -424,7 +428,7 @@ def run( else: self.assertEqual(list((topdir / "CHECKOUT").iterdir()), []) - def test_spec_symlink_cannot_write_outside_the_staged_checkout(self) -> None: + def test_spec_symlink_cannot_escape_the_staged_checkout(self) -> None: source = self.scratch / "checkout" source.mkdir() (source / "escape.spec").symlink_to(self.spec_file) diff --git a/package_system/rpm/rpmbuild.py b/package_system/rpm/rpmbuild.py index dd1cc48b..46c951a5 100644 --- a/package_system/rpm/rpmbuild.py +++ b/package_system/rpm/rpmbuild.py @@ -78,25 +78,23 @@ def stage( # readonly_project() already created BUILD if it mounted the persistent build directory there. (topdir / builder.BUILD).mkdir(exist_ok=True) checkout = topdir / builder.CHECKOUT + staged_spec = topdir / "SPECS" / spec_file.name + chroot_spec = builder.CHROOT_TOPDIR / "SPECS" / spec_file.name if source_tree is not None and spec_file.is_relative_to(source_tree): relative_spec = spec_file.relative_to(source_tree) if ".." in relative_spec.parts: util.fail(f"rpmbuild: RPM spec must stay within the source tree: {relative_spec}") - staged_spec = checkout / relative_spec - # The spec is frozen before the chroot. A spec symlink that escapes the checkout would otherwise - # make the freeze write to a file of the outer sandbox. - if not staged_spec.resolve().is_relative_to(checkout.resolve()): + source_spec = checkout / relative_spec + # stage() reads the spec outside the chroot. Without the check, a spec symlink that points out of + # the checkout would copy a file of the action sandbox into the build. + if not source_spec.resolve().is_relative_to(checkout.resolve()): util.fail(f"rpmbuild: RPM spec symlink escapes the source tree: {relative_spec}") - source_spec = staged_spec - chroot_spec = builder.CHROOT_TOPDIR / builder.CHECKOUT / relative_spec - sourcedir = chroot_spec.parent + sourcedir = builder.CHROOT_TOPDIR / builder.CHECKOUT / relative_spec.parent else: # The spec is read outside the chroot. A spec symlink in a fetched tree would read a file of the # project. reject_symlinks(spec_file, "RPM spec") source_spec = spec_file - staged_spec = topdir / "SPECS" / spec_file.name - chroot_spec = builder.CHROOT_TOPDIR / "SPECS" / spec_file.name sourcedir = None for s in source_files(sources): # A spec may modify SOURCES, so it must not share the source artifact's inode. @@ -105,8 +103,9 @@ def stage( if not source_spec.is_file(): util.fail(f"rpmbuild: RPM spec does not exist: {spec_file}") - # The frozen rpmautospec macros let the spec parse without Git or rpmautospec. An in-place spec stays - # beside its auxiliary files in the disposable tree, so that its relative includes still resolve. + # The frozen rpmautospec macros let the spec parse without Git or rpmautospec. rpm opens an %include + # relative to its working directory, not to the spec. rpm therefore still finds the included files of + # an in-place spec when the frozen copy is in SPECS. staged_spec.write_text( f"%global autorelease {release}%{{?dist}}\n%global autochangelog %{{nil}}\n{source_spec.read_text()}" ) From 684e7060818d00b28127bf153cdfc69e1bf028f7 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 14:45:03 +0100 Subject: [PATCH 3/8] rpm: Add a driver that queries the BuildRequires of a spec Nothing reads the BuildRequires of the spec that a build uses. The buildroot only gets what the BUCK file lists. Let's add a driver that stages the spec like build.py and runs rpmbuild -br in the buildroot with the same arguments as the build. That also picks up what %generate_buildrequires reports. Signed-off-by: Daan De Meyer --- docs/design/architecture.md | 2 +- package/builder.py | 6 +- package_system/rpm/BUCK | 15 ++ package_system/rpm/build.py | 2 +- package_system/rpm/buildrequires.py | 110 ++++++++ package_system/rpm/buildrequires_test.py | 314 +++++++++++++++++++++++ 6 files changed, 444 insertions(+), 5 deletions(-) create mode 100755 package_system/rpm/buildrequires.py create mode 100644 package_system/rpm/buildrequires_test.py diff --git a/docs/design/architecture.md b/docs/design/architecture.md index 912d8951..a72b5e7a 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -748,7 +748,7 @@ names none, and gets no per-release aliases either. Entry points: `package_system/rpm/{rules,catalog,generated}.bzl`, `package_system/rpm/{snapshot,plan,install,pkgdb,createrepo,build,extract,rpmfile}.py` and -`package_system/rpm/rpmbuild.py`. +`package_system/rpm/{rpmbuild,buildrequires}.py`. ### The alpm package system diff --git a/package/builder.py b/package/builder.py index 3dd5d10b..b16320b9 100644 --- a/package/builder.py +++ b/package/builder.py @@ -76,8 +76,8 @@ def run(spec: Spec, build: Callable[[Path, Path, Path], int]) -> int: return rc -def buildroot(spec: Spec, topdir: Path) -> AbstractContextManager[Path]: - """Chroot into the buildroot of `spec`, with topdir at `CHROOT_TOPDIR`.""" +def buildroot(lower: list[str], topdir: Path) -> AbstractContextManager[Path]: + """Chroot into the buildroot with the layers `lower`, with topdir at `CHROOT_TOPDIR`.""" return rootfs.rootfs( - "/buildroot", lowers=spec["lower"], binds=[(topdir, CHROOT_TOPDIR)], apivfs=True, chroot=True + "/buildroot", lowers=lower, binds=[(topdir, CHROOT_TOPDIR)], apivfs=True, chroot=True ) diff --git a/package_system/rpm/BUCK b/package_system/rpm/BUCK index a4ccce6f..c2b5a87b 100644 --- a/package_system/rpm/BUCK +++ b/package_system/rpm/BUCK @@ -49,6 +49,21 @@ python.test( deps = [":rpmbuild", "//:specs", "//:util", "//package:builder", "//package:subpackage"], ) +python.binary( + name = "buildrequires", + main = "buildrequires.py", + deps = [":rpmbuild", "//:specs", "//:util", "//package:builder", "//rootfs:rootfs"], + visibility = ["PUBLIC"], +) + +# The test queries specs with the rpmbuild of the dev box. +python.test( + name = "buildrequires-test", + box = _BOX, + srcs = ["buildrequires.py", "buildrequires_test.py"], + deps = [":rpmbuild", "//:specs", "//:util", "//package:builder", "//rootfs:rootfs"], +) + python.binary( name = "snapshot", deps = ["//:util", "//package:href", "//package:snapshotter"], diff --git a/package_system/rpm/build.py b/package_system/rpm/build.py index 1f82a80d..8d8e5e0f 100755 --- a/package_system/rpm/build.py +++ b/package_system/rpm/build.py @@ -54,7 +54,7 @@ def _build_rpm(spec: Spec, topdir: Path, out: Path, subpackages: Path) -> int: incremental = spec["build_dir"] is not None env = os.environ | rpmbuild.environment(spec["source_date_epoch"]) - with builder.buildroot(spec, topdir): + with builder.buildroot(spec["lower"], topdir): arguments = rpmbuild.arguments( staged, dist=spec["dist"], diff --git a/package_system/rpm/buildrequires.py b/package_system/rpm/buildrequires.py new file mode 100755 index 00000000..18603c52 --- /dev/null +++ b/package_system/rpm/buildrequires.py @@ -0,0 +1,110 @@ +#!/usr/bin/python3 + +# SPDX-FileCopyrightText: Amutable GmbH +# SPDX-License-Identifier: MPL-2.0 + +"""Write the BuildRequires of an RPM spec as a sorted JSON list. + +The driver stages the spec as build.py does and runs `rpmbuild -br` in the buildroot with the arguments of +the build. The distribution's macros and the spec's conditionals then select the same BuildRequires as +the build does, and `%generate_buildrequires` adds its BuildRequires as well. +""" + +import json +import os +import subprocess +import sys +import tempfile +from contextlib import ExitStack +from pathlib import Path +from typing import TypedDict + +import specs +import util + +import builder +import rootfs +import rpmbuild + + +class Spec(TypedDict): + # `lower` lists the layers of the buildroot with declared BuildRequires installed, bottom first. + lower: list[str] + spec_file: str + # `sources` lists the Source and Patch files, and directories whose top-level files are Source and Patch + # files. + sources: list[str] + # `source_tree` is the source tree of an in-place build, or None. + source_tree: str | None + dist: str + source_date_epoch: int + release: str + rpmbuild_options: list[str] + in_place_rpmbuild_options: list[str] + # `incremental` is True for a dev-mode build, which adds the incremental rpmbuild options. + incremental: bool + out: str + + +def query(spec: Spec) -> list[str]: + """Return the BuildRequires of the spec that `spec` describes.""" + env = os.environ | rpmbuild.environment(spec["source_date_epoch"]) + source_tree = spec["source_tree"] + # The sandbox points TMPDIR at /var/tmp, which it backs with the action's scratch space. + topdir = Path(tempfile.gettempdir()) / "topdir" + topdir.mkdir() + with ExitStack() as stack: + if source_tree is not None: + stack.enter_context(rootfs.source_overlay(Path(source_tree), topdir / builder.CHECKOUT)) + staged = rpmbuild.stage( + topdir, Path(spec["spec_file"]), spec["sources"], source_tree, spec["release"] + ) + arguments = rpmbuild.arguments( + staged, + dist=spec["dist"], + incremental=spec["incremental"], + rpmbuild_options=spec["rpmbuild_options"], + in_place_rpmbuild_options=spec["in_place_rpmbuild_options"], + ) + # --nodeps writes the BuildRequires into the source rpm instead of failing on the missing ones. + mode = ["-br", "--nodeps"] + # rpmbuild skips %prep for an in-place build. --noprep would skip it as well, but rpmbuild then + # doesn't parse the spec again for its BuildArch, which build.py's rpmbuild -bb does. + if source_tree is not None: + mode.append("--build-in-place") + stack.enter_context(builder.buildroot(spec["lower"], topdir)) + rc = subprocess.run( + ["/usr/bin/rpmbuild", *arguments, *mode, str(staged.spec)], + cwd=staged.cwd, + env=env, + stdout=sys.stderr, + ).returncode + # rpmbuild exits with 11 when %generate_buildrequires reports BuildRequires. It then writes a + # .buildreqs.nosrc.rpm instead of the source rpm. + if rc not in (0, 11): + util.fail(f"buildrequires: rpmbuild -br failed for {spec['spec_file']} with exit status {rc}") + [srpm] = (builder.CHROOT_TOPDIR / "SRPMS").iterdir() + requires = subprocess.run( + ["/usr/bin/rpm", "--query", "--package", "--queryformat", "[%{REQUIRENEVRS}\n]", str(srpm)], + env=env, + stdout=subprocess.PIPE, + text=True, + check=True, + ).stdout.splitlines() + # rpmlib() entries name features of rpm itself, and no package provides them. + return sorted({name for name in requires if not name.startswith("rpmlib(")}) + + +def main(argv: list[str] | None = None) -> None: + """Parse an invocation and write the BuildRequires of its spec.""" + spec = specs.parse(Spec, "buildrequires", argv) + # The query runs %prep and every %(...) of the spec. The project is read-only meanwhile, as during the + # build. + with rootfs.readonly_project(Path.cwd(), {}): + found = query(spec) + Path(spec["out"]).write_text(json.dumps(found, indent=2) + "\n") + print(f"{spec['spec_file']}: {len(found)} BuildRequires", file=sys.stderr) + + +if __name__ == "__main__": + main() diff --git a/package_system/rpm/buildrequires_test.py b/package_system/rpm/buildrequires_test.py new file mode 100644 index 00000000..3aecdabd --- /dev/null +++ b/package_system/rpm/buildrequires_test.py @@ -0,0 +1,314 @@ +# SPDX-FileCopyrightText: Amutable GmbH +# SPDX-License-Identifier: MPL-2.0 + +"""Tests for the BuildRequires query of an RPM spec. + +The tests query specs with the dev box's rpmbuild outside a buildroot. They replace the chroot with a mock +and point `builder.CHROOT_TOPDIR` at the staged rpmbuild tree. + +buck test tine//package_system/rpm:buildrequires-test +""" + +import contextlib +import json +import os +import shutil +import tempfile +import unittest +from pathlib import Path +from typing import override +from unittest import mock + +import builder +import buildrequires + + +def setUpModule() -> None: + # The dev box contains rpmbuild only when it builds from a Fedora release. + if shutil.which("rpmbuild") is None: + raise unittest.SkipTest("the box contains no rpmbuild") + + +SPEC = """\ +Name: example +Version: 1 +Release: %autorelease +Summary: BuildRequires fixture +License: MPL-2.0 +%bcond_without docs +%bcond_without lto +BuildRequires: gcc +BuildRequires: meson >= 0.60 +BuildRequires: (python3-foo or python3-bar) +BuildRequires: %(cat %{_sourcedir}/extra-buildrequires) +%if %{with docs} +BuildRequires: docbook-style-xsl +%endif +%if %{with lto} +BuildRequires: lto-tool +%endif +%if 0%{?common} +BuildRequires: common-devel +%endif +%if "%{?dist}" == ".test" +BuildRequires: dist-devel +%endif +%if "%{release}" == "7.test" +BuildRequires: release-devel +%endif + +%description +A fixture. +""" + +IN_PLACE_SPEC = SPEC.replace( + "%description", "%if %{defined _build_in_place}\nBuildRequires: in-place-devel\n%endif\n\n%description" +) + + +class Query(unittest.TestCase): + @override + def setUp(self) -> None: + self.scratch = Path( + self.enterContext(tempfile.TemporaryDirectory(prefix="rpm-br-test.", dir="/var/tmp")) + ) + self.spec_file = self.scratch / "example.spec" + self.spec_file.write_text(SPEC) + self.extra = self.scratch / "extra-buildrequires" + self.extra.write_text("source-devel\n") + self.enterContext(mock.patch.object(tempfile, "tempdir", str(self.scratch))) + self.mounted = self.enterContext( + mock.patch.object(builder, "buildroot", return_value=contextlib.nullcontext()) + ) + self.enterContext(mock.patch.object(builder, "CHROOT_TOPDIR", self.scratch / "topdir")) + self.enterContext(mock.patch.dict(os.environ)) + + def specification( + self, + *, + spec_file: Path | None = None, + sources: list[Path] | None = None, + source_tree: Path | None = None, + rpmbuild_options: list[str] | None = None, + in_place_rpmbuild_options: list[str] | None = None, + incremental: bool = False, + ) -> buildrequires.Spec: + return buildrequires.Spec( + lower=["buildroot"], + spec_file=str(spec_file or self.spec_file), + sources=[str(source) for source in (sources if sources is not None else [self.extra])], + source_tree=str(source_tree) if source_tree is not None else None, + dist=".test", + source_date_epoch=1754006400, + release="7", + rpmbuild_options=["--define=common 1"] if rpmbuild_options is None else rpmbuild_options, + in_place_rpmbuild_options=in_place_rpmbuild_options or [], + incremental=incremental, + out=str(self.scratch / "buildrequires.json"), + ) + + def test_query_uses_the_options_of_the_build(self) -> None: + self.assertEqual( + buildrequires.query(self.specification()), + [ + "(python3-foo or python3-bar)", + "common-devel", + "dist-devel", + "docbook-style-xsl", + "gcc", + "lto-tool", + "meson >= 0.60", + "release-devel", + "source-devel", + ], + ) + self.mounted.assert_called_once_with(["buildroot"], self.scratch / "topdir") + + def test_dev_mode_options_apply(self) -> None: + self.assertNotIn("lto-tool", buildrequires.query(self.specification(incremental=True))) + + def test_query_uses_the_environment_of_the_build(self) -> None: + self.spec_file.write_text( + SPEC.replace( + "%description", + f'%if "%{{getenv:HOME}}" == "{builder.CHROOT_TOPDIR}"\n' + "BuildRequires: home-devel\n%endif\n" + '%if "%{getenv:SOURCE_DATE_EPOCH}" == "1754006400"\n' + "BuildRequires: epoch-devel\n%endif\n%description", + ) + ) + with mock.patch.dict(os.environ, {"HOME": "/root", "SOURCE_DATE_EPOCH": "1739577600"}): + found = buildrequires.query(self.specification()) + self.assertIn("home-devel", found) + self.assertIn("epoch-devel", found) + + def test_in_place_queries_define_the_debug_prefix_map(self) -> None: + checkout = self.scratch / "checkout" + checkout.mkdir() + (checkout / "extra-buildrequires").write_text("source-devel\n") + (checkout / "example.spec").write_text( + SPEC.replace( + "%description", + "%if %{defined _distro_extra_cflags}\nBuildRequires: flags-devel\n%endif\n%description", + ) + ) + found = buildrequires.query( + self.specification(spec_file=checkout / "example.spec", sources=[], source_tree=checkout) + ) + self.assertIn("flags-devel", found) + + def test_options_select_conditional_buildrequires(self) -> None: + found = buildrequires.query(self.specification(rpmbuild_options=[])) + self.assertIn("docbook-style-xsl", found) + self.assertNotIn("common-devel", found) + + def test_hyphenated_options_select_conditional_buildrequires(self) -> None: + self.spec_file.write_text( + SPEC.replace( + "%description", + "%if %{defined _with_foo_bar}\nBuildRequires: jq\n%endif\n" + "%if %{defined _without_baz_qux}\nBuildRequires: gettext\n%endif\n%description", + ) + ) + found = buildrequires.query( + self.specification(rpmbuild_options=["--with=foo-bar", "--without=baz-qux"]) + ) + self.assertIn("jq", found) + self.assertIn("gettext", found) + + def test_in_place_options_apply_after_the_others(self) -> None: + found = buildrequires.query( + self.specification( + rpmbuild_options=["--with=docs"], in_place_rpmbuild_options=["--without=docs"] + ) + ) + self.assertNotIn("docbook-style-xsl", found) + + def test_only_an_in_place_build_defines_build_in_place(self) -> None: + checkout = self.scratch / "checkout" + checkout.mkdir() + (checkout / "extra-buildrequires").write_text("source-devel\n") + (checkout / "example.spec").write_text(IN_PLACE_SPEC) + self.assertIn( + "in-place-devel", + buildrequires.query( + self.specification(spec_file=checkout / "example.spec", sources=[], source_tree=checkout) + ), + ) + + def test_an_archive_build_does_not_define_build_in_place(self) -> None: + self.spec_file.write_text(IN_PLACE_SPEC) + self.assertNotIn("in-place-devel", buildrequires.query(self.specification())) + + def test_a_noarch_spec_parses_with_the_macro_files_of_noarch(self) -> None: + # rpmbuild expands %{_lib} to lib for a noarch spec on a 64-bit host, not to lib64. + self.spec_file.write_text( + SPEC.replace( + "License: MPL-2.0\n", + "License: MPL-2.0\nBuildArch: noarch\nBuildRequires: /usr/%{_lib}/libexample.so\n", + ) + ) + found = buildrequires.query(self.specification()) + self.assertIn("/usr/lib/libexample.so", found) + # The reload keeps the definitions of the command line. + self.assertIn("common-devel", found) + + def test_a_directory_among_the_sources_contributes_its_top_level_files(self) -> None: + directory = self.scratch / "directory" + directory.mkdir() + (directory / "extra-buildrequires").write_text("directory-devel\n") + found = buildrequires.query(self.specification(sources=[directory])) + self.assertIn("directory-devel", found) + + def test_an_in_place_spec_parses_in_the_checkout_like_the_build(self) -> None: + checkout = self.scratch / "checkout" + (checkout / "rpm").mkdir(parents=True) + (checkout / "VERSION").write_text("2.5\n") + (checkout / "rpm" / "extra.inc").write_text("BuildRequires: include-devel\n") + (checkout / "rpm" / "extra-buildrequires").write_text("source-devel\n") + # rpm opens %include relative to the working directory, which is the root of the checkout. + spec = SPEC.replace("Version: 1\n", "Version: %(cat VERSION)\n%include rpm/extra.inc\n") + (checkout / "rpm" / "example.spec").write_text(spec) + + found = buildrequires.query( + self.specification(spec_file=checkout / "rpm" / "example.spec", sources=[], source_tree=checkout) + ) + + self.assertIn("include-devel", found) + self.assertIn("source-devel", found) + self.assertEqual((checkout / "rpm" / "example.spec").read_text(), spec) + + def test_parse_time_writes_do_not_modify_the_checkout(self) -> None: + checkout = self.scratch / "checkout" + checkout.mkdir() + (checkout / "extra-buildrequires").write_text("source-devel\n") + spec = SPEC.replace( + "%description", + '%if "%(echo yes > written && cat written)" == "yes"\n' + "BuildRequires: generated-devel\n%endif\n%description", + ) + (checkout / "example.spec").write_text(spec) + + found = buildrequires.query( + self.specification(spec_file=checkout / "example.spec", sources=[], source_tree=checkout) + ) + self.assertIn("generated-devel", found) + + self.assertEqual( + sorted(path.name for path in checkout.iterdir()), ["example.spec", "extra-buildrequires"] + ) + + def test_query_leaves_the_spec_unchanged(self) -> None: + buildrequires.query(self.specification()) + self.assertEqual(self.spec_file.read_text(), SPEC) + + def test_main_writes_a_json_list(self) -> None: + spec = self.specification() + path = self.scratch / "spec.json" + path.write_text(json.dumps(spec)) + readonly = self.enterContext( + mock.patch.object( + buildrequires.rootfs, "readonly_project", return_value=contextlib.nullcontext() + ) + ) + buildrequires.main(["--spec", str(path)]) + readonly.assert_called_once() + self.assertIn("gcc", json.loads(Path(spec["out"]).read_text())) + + def test_rpmlib_entries_are_dropped(self) -> None: + # The rich dependency of SPEC makes librpm add rpmlib(RichDependencies). + found = buildrequires.query(self.specification()) + self.assertFalse([name for name in found if name.startswith("rpmlib(")], found) + + def test_missing_spec_fails(self) -> None: + with self.assertRaisesRegex(SystemExit, "does not exist"): + buildrequires.query(self.specification(spec_file=self.scratch / "missing.spec")) + + def test_spec_symlink_cannot_escape_the_source_tree(self) -> None: + checkout = self.scratch / "checkout" + checkout.mkdir() + (checkout / "linked.spec").symlink_to(self.spec_file) + with self.assertRaisesRegex(SystemExit, "escapes"): + buildrequires.query( + self.specification(spec_file=checkout / "linked.spec", sources=[], source_tree=checkout) + ) + self.assertEqual(self.spec_file.read_text(), SPEC) + + def test_unparsable_spec_fails(self) -> None: + self.spec_file.write_text("Name: example\n%if\n") + with self.assertRaisesRegex(SystemExit, "rpmbuild -br failed"): + buildrequires.query(self.specification()) + + def test_generated_buildrequires_are_listed(self) -> None: + # rpmbuild -br runs %generate_buildrequires and exits with 11 when it reports BuildRequires. The + # query lists them next to the static ones. + self.spec_file.write_text( + SPEC.replace("%description", "%generate_buildrequires\necho generated-devel\n\n%description") + ) + found = buildrequires.query(self.specification()) + self.assertIn("generated-devel", found) + self.assertIn("gcc", found) + + +if __name__ == "__main__": + unittest.main() From 9f449141c3c8807751f3b683ef5a4948c54e7c2d Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 3 Nov 2026 21:08:13 +0100 Subject: [PATCH 4/8] buildroot: Skip empty dependency solves rpm.branch() takes buildroot_packages from the BuildRequires of all architectures, but build_requires only has those of the current one. A package can then end up with buildroot_packages and nothing to install, and the solve fails with "a solve needs at least one install spec". Let's skip the solve when there's nothing to install. Signed-off-by: Daan De Meyer --- package/buildroot.bzl | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/package/buildroot.bzl b/package/buildroot.bzl index 08676a77..4624194e 100644 --- a/package/buildroot.bzl +++ b/package/buildroot.bzl @@ -75,9 +75,6 @@ def package_buildroot( delta prefers the packages of `buildroot_packages` over the repositories of the package manager. `owner` starts a failure message. """ - stack = [base.root] - if not build_requires and not buildroot_packages: - return stack package_system = base.package_manager[PackageManagerInfo].package_system extra_packages = [] for dependency in buildroot_packages: @@ -92,6 +89,13 @@ def package_buildroot( ), ) extra_packages.append(info.packages) + stack = [base.root] + + # The packages of buildroot_packages are only candidates for the solve, so they install nothing on + # their own. rpm.branch() derives them from the BuildRequires of every architecture, so a package can + # have them without any BuildRequires on this architecture. + if not build_requires: + return stack return stack + [ install_packages( ctx, From cc5e539da0292210d9922813a91b01c2a762d46f Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 3 Nov 2026 21:08:27 +0100 Subject: [PATCH 5/8] rpm: Don't check rich BuildRequires for delivery undelivered() checks that each install spec ended up in the transaction. For a rich dependency like (A if B), resolve_pkg_spec() assumes B holds, so the check fails when B isn't installed. Let's skip rich dependencies in that check. Signed-off-by: Daan De Meyer --- package_system/rpm/plan.py | 4 ++++ package_system/rpm/plan_test.py | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/package_system/rpm/plan.py b/package_system/rpm/plan.py index be6d2ca9..eb42b552 100755 --- a/package_system/rpm/plan.py +++ b/package_system/rpm/plan.py @@ -133,6 +133,10 @@ def undelivered( if spec.removeprefix("@") not in groups: missing.append(spec) continue + # A rich dependency such as (A if B) holds without installing anything while B is absent. + # resolve_pkg_spec() assumes that the condition holds, so it would report A as missing. + if spec.startswith("("): + continue wanted = libdnf5.rpm.PackageQuery(base) wanted.resolve_pkg_spec(spec, settings, False) delivered = libdnf5.rpm.PackageQuery(wanted) diff --git a/package_system/rpm/plan_test.py b/package_system/rpm/plan_test.py index d751d987..1346bca6 100644 --- a/package_system/rpm/plan_test.py +++ b/package_system/rpm/plan_test.py @@ -94,6 +94,16 @@ def test_resolves_a_spec_naming_a_file(self) -> None: repo = self.repository("file", self.build("gadget", "1")) self.assertEqual(self.solve(["/usr/share/gadget/marker"], repo), ["gadget-1-1.noarch"]) + def test_a_conditional_spec_whose_condition_is_false_installs_nothing(self) -> None: + repo = self.repository("conditional", self.build("gadget", "1"), self.build("trigger", "1")) + self.assertEqual(self.solve(["(gadget if trigger)"], repo), []) + + def test_a_conditional_spec_whose_condition_is_true_installs_its_package(self) -> None: + repo = self.repository("conditional-true", self.build("gadget", "1"), self.build("trigger", "1")) + self.assertEqual( + self.solve(["trigger", "(gadget if trigger)"], repo), ["gadget-1-1.noarch", "trigger-1-1.noarch"] + ) + def test_refuses_a_spec_an_obsoleting_package_displaced(self) -> None: """split-out subpackage obsoleting its parent of older version is refused.""" From 053d937e40cc632d527dd9d43c9e1c7c55bb9250 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 14:46:32 +0100 Subject: [PATCH 6/8] rpm: Resolve in-place BuildRequires at build time A build with in_place_spec or in_place_packaging only gets the BuildRequires that the BUCK file lists. systemd lists 66 of them by hand, and the list goes stale whenever the pin moves. Let's query the spec and install what it needs as an extra layer on top of the declared ones. Imported packages keep their recorded BuildRequires. rpm.branch() derives buildroot_packages from them, so anything new would come from the distribution instead of the branch. Signed-off-by: Daan De Meyer --- docs/design/architecture.md | 14 +++- examples/package/rpm/BUCK | 4 - examples/package/rpm/hello-packaging.bundle | Bin 777 -> 857 bytes .../package/rpm/hello.source/rpm/hello.spec | 6 ++ .../rpm/hello.source/rpm/packaging.json | 2 +- examples/package/rpm/hello_dev_check.py | 2 + examples/package/rpm/hello_pinned_check.py | 2 + examples/package/rpm/hello_tree_check.py | 2 + package/build.bzl | 9 +- package/buildroot.bzl | 54 +++++++++--- package/install.bzl | 26 ++++-- package/installer.py | 7 ++ package/installer_test.py | 26 ++++++ package/solver.bzl | 5 +- package/transaction.py | 13 +++ package/transaction_test.py | 20 +++++ package_system/deb/plan.py | 6 +- package_system/deb/plan_test.py | 23 ++++++ package_system/pacman/plan.py | 9 +- package_system/pacman/plan_test.py | 15 +++- package_system/rpm/generated.bzl | 1 + package_system/rpm/plan.py | 9 +- package_system/rpm/plan_test.py | 21 +++++ package_system/rpm/rules.bzl | 78 ++++++++++++++---- 24 files changed, 296 insertions(+), 58 deletions(-) diff --git a/docs/design/architecture.md b/docs/design/architecture.md index a72b5e7a..fb877717 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -713,7 +713,12 @@ and runtime package closures are still delegated to libdnf5 at buildroot-plan ti An `rpm_package` action: 1. obtains the shared base root from its `BuildrootInfo`; -2. resolves and installs its BuildRequires delta, preferring RPMs from `buildroot_packages` over upstream; +2. resolves and installs its BuildRequires delta, preferring RPMs from `buildroot_packages` over upstream. A + source-tree build of a hand-written `rpm.package()` with `in_place_spec` or `in_place_packaging` also + installs the BuildRequires of its spec. Analysis cannot read the contents of a spec, so an action stages + the spec as the build does and runs `rpmbuild -br` with the build's options in the base root and the + delta. A second delta installs the BuildRequires that it writes. That delta is empty when the spec needs + nothing more; 3. overlays the base and delta, stages its spec/sources in Buck action scratch, and runs `rpmbuild -ba`, or `rpmbuild -bb --build-in-place` on a volatile overlay of a populated source tree. overlayfs shows the directory under a mount rather than the mounted tree, so each directory mount below the source tree, @@ -739,7 +744,12 @@ names none, and gets no per-release aliases either. #### Limitations - The imported self-host dependency graph is inferred from stored BuildRequires/Provides/file metadata; it - does not run RPM's dynamic BuildRequires protocol. + does not run RPM's dynamic BuildRequires protocol. The BuildRequires query runs `%generate_buildrequires` + once. It misses what the script only reports after its own BuildRequires are installed. +- An imported package installs only its recorded BuildRequires, also for a spec from a checkout or a pin. + `rpm.branch()` derives `buildroot_packages` from them, and Buck cannot add a target dependency that an + action output discovers. A queried BuildRequires on a package of the branch would install it from + upstream. - Build cycles fall back to upstream RPMs for ordinary intra-SCC edges, so the package set is not a fully self-hosted fixed point. - Builds use `--nocheck`; package test policy is not implemented. diff --git a/examples/package/rpm/BUCK b/examples/package/rpm/BUCK index ae0d2964..546526ad 100644 --- a/examples/package/rpm/BUCK +++ b/examples/package/rpm/BUCK @@ -48,7 +48,6 @@ rpm.package( distro = _DISTRO, package = "hello", buildroot = _BUILDROOT, - build_requires = ["gcc"], dev = True, in_place_spec = "rpm/hello.spec", release = "1", @@ -77,7 +76,6 @@ rpm.package( distro = _DISTRO, package = "hello", buildroot = _BUILDROOT, - build_requires = ["gcc"], in_place_spec = "rpm/hello.spec", release = "1", source_date_epoch = 1754006400, @@ -105,7 +103,6 @@ rpm.package( distro = _DISTRO, package = "hello", buildroot = _BUILDROOT, - build_requires = ["gcc", "meson"], in_place_spec = "rpm/hello-meson.spec", release = "1", source_date_epoch = 1754006400, @@ -131,7 +128,6 @@ rpm.package( distro = _DISTRO, package = "hello", buildroot = _BUILDROOT, - build_requires = ["gcc"], in_place_packaging = "rpm/packaging.json", in_place_packaging_distro = "fedora", release = "1", diff --git a/examples/package/rpm/hello-packaging.bundle b/examples/package/rpm/hello-packaging.bundle index 9e65979092a3b21b7856d806a6fc091b9620fbf0..9b4e18f7c266983028161ad8211b052694849375 100644 GIT binary patch delta 778 zcmV+l1NHog2H6HUBOrD%AZKZGAYyfHWNc*$W@a%qV`DfuF=RPnVlpx^G%z=1Gi79B zH#0RhHeoVlW@a!jVKp}}kvJ%S4#FT11<*aGn7xVlSfpJTqemIwwH-+gU|^t@4SKOg*P-3!mL)w|g7>HzNm zIRSu)t=8N-|0w2}lB_JH$*%FGOWqdq1;AM{v9APpoHH~qFf%bx$Vkn9$;sC%E=Wyg z2sm5b7PbH8#;>djcG|^O8^2mNKQ@G@D#*;sOHI+uO)SbzEn=`YnR;`L-rL6u&U9w{ z)7qdv?Zk#30JFOx;;=n+kYC8JxQ6$x~kNJ#ILL%d%#*GzcHc zYYGVS*&QrvGHHOH#06A;Nrezg`0S9^_s6xiaD@8V>mI**(K+fOeh IBk@o}0a{*Z>;M1& delta 690 zcmV;j0!{td28jkZBOrD%AZKZGAYyfHWNc*$W@BbHFfe8`FgGzWGh}3CHDzNrIc739 zWjACvH8M71Ibk$nVP!BgkvJ%S3WG2Z1<-v?G5bRKqcM$8q&>9 zcCVeP#4#atv3CKp^&`W(Iq=v8kq{jB%ow~2YUJJ4dQ!`~(*5T{KT?iOud$Yi_%>L; zS%5A8U}7yLx2As-^Gu0W-L^`H#Fs32U(6RxZ87$*1bCb?G%zqTF;U2o-3ouGm8j0& zd!_4P>!Jf+0whhoUjH`jrXfUCL1tcFYKm@dVo`Q#5re(S)SGMc-acM%rZeN8)&~7) zCpP>501b5`(zq;moQ+Z2YTG~%eV?xw2uq*Vic8XhD1|ghFOamxZcE?9dNq;;@2$J5 z)^DIcGR$&dh(D;bwL7MZRW3*FHQ%ZoB-E_PN9>+h?^kgm3KjExzb- z4yh~(HPZ|2ZSfM$j`};+&lOC@$>jEBz@6MI+U64qo$ zB`Q&fyLFt`p*|pe2Z&80?VCSgDnp2i7 zXVl(W&M;>>1$i}^tm$jdky44R*0Aw?)U+&T4{250Q3Mb*B^^0r_71Kbvdh#)H9~B7 zpZiGI_=uSuJxPxVPRWAG-2y;Nxxwygjwl0)q53wKs)?^T9zYCLrS$>DFmaV`jB3*f zn|das|AV}kwu;lc&;V6}UCPQBn17Tv!Ja?Ay!^X%ZWO)G2QgGzc2jYjOdba!dA!{( zp3g2WPz;+7pVrs6%b#%{U22upZ#G2=q~&-t8Q1}ch7F~01OI6HpJqP*b`$$KlM4en Y6f+@fEs@x$!|bK|FegFUf diff --git a/examples/package/rpm/hello.source/rpm/hello.spec b/examples/package/rpm/hello.source/rpm/hello.spec index e071dca1..e354773a 100644 --- a/examples/package/rpm/hello.source/rpm/hello.spec +++ b/examples/package/rpm/hello.source/rpm/hello.spec @@ -7,6 +7,10 @@ Release: %autorelease Summary: Exercise dev-mode source-tree RPM package builds License: MPL-2.0 BuildRequires: gcc +# The base buildroot doesn't contain jq, and the BUCK file doesn't list it. +%if "%(echo jq > generated-buildrequires && cat generated-buildrequires)" == "jq" +BuildRequires: jq +%endif Source1: packaging-marker %description @@ -25,10 +29,12 @@ annobin=enabled printf '%s\n' "$lto" > %{_vpath_builddir}/lto printf '%s\n' "$annobin" > %{_vpath_builddir}/annobin %{__cc} %{build_cflags} %{build_ldflags} -o %{_vpath_builddir}/hello hello.c +jq -n '"checkout"' > %{_vpath_builddir}/jq %install install -Dpm0755 %{_vpath_builddir}/hello %{buildroot}%{_bindir}/tine-package-hello install -Dpm0644 %{SOURCE1} %{buildroot}%{_datadir}/tine-package/spec-from-checkout +install -Dpm0644 %{_vpath_builddir}/jq %{buildroot}%{_datadir}/tine-package/built-with-jq install -d %{buildroot}%{_datadir}/tine-package lto="$(cat %{_vpath_builddir}/lto)" annobin="$(cat %{_vpath_builddir}/annobin)" diff --git a/examples/package/rpm/hello.source/rpm/packaging.json b/examples/package/rpm/hello.source/rpm/packaging.json index 98a7ff29..5b8469fd 100644 --- a/examples/package/rpm/hello.source/rpm/packaging.json +++ b/examples/package/rpm/hello.source/rpm/packaging.json @@ -2,6 +2,6 @@ "fedora": { "url": "examples/package/rpm/hello-packaging.bundle", "branch": "rawhide", - "commit": "fcf600f407123dde5ec79f27e7d8526c9a4bae03" + "commit": "ff16cc891d9bb222407e3edc73566a2eff00a570" } } diff --git a/examples/package/rpm/hello_dev_check.py b/examples/package/rpm/hello_dev_check.py index fd7b5621..d479d4ac 100644 --- a/examples/package/rpm/hello_dev_check.py +++ b/examples/package/rpm/hello_dev_check.py @@ -26,6 +26,8 @@ def query(package: Path, *args: str) -> str: files = set(query(package, "-l").splitlines()) markers = "/usr/share/tine-package" assert f"{markers}/spec-from-checkout" in files, files +# The build installs jq only because the spec lists it in BuildRequires, and %build runs jq. +assert f"{markers}/built-with-jq" in files, files assert f"{markers}/profile-lto-disabled" in files, files assert f"{markers}/profile-annobin-disabled" in files, files assert not list(rpms.glob("hello-debuginfo-*.rpm")), sorted(rpms.iterdir()) diff --git a/examples/package/rpm/hello_pinned_check.py b/examples/package/rpm/hello_pinned_check.py index c8c3c572..ad56118a 100644 --- a/examples/package/rpm/hello_pinned_check.py +++ b/examples/package/rpm/hello_pinned_check.py @@ -25,5 +25,7 @@ def query(package: Path, *args: str) -> str: files = set(query(package, "-l").splitlines()) assert "/usr/share/tine-package/spec-from-pin" in files, files assert "/usr/share/tine-package/spec-from-checkout" not in files, files +# The build installs jq only because the pinned spec lists it in BuildRequires, and %build runs jq. +assert "/usr/share/tine-package/built-with-jq" in files, files assert query(subpackage, "--queryformat", "%{NAME}") == "hello" print(f"hello: pinned packaging, {len(files)} files") diff --git a/examples/package/rpm/hello_tree_check.py b/examples/package/rpm/hello_tree_check.py index 3c2c3587..8e2ce506 100644 --- a/examples/package/rpm/hello_tree_check.py +++ b/examples/package/rpm/hello_tree_check.py @@ -26,6 +26,8 @@ def query(package: Path, *args: str) -> str: markers = "/usr/share/tine-package" # The spec comes out of the checkout as in a dev build, but none of the dev-only savings apply. assert f"{markers}/spec-from-checkout" in files, files +# The build installs jq only because the spec lists it in BuildRequires, and %build runs jq. +assert f"{markers}/built-with-jq" in files, files assert f"{markers}/profile-lto-enabled" in files, files assert f"{markers}/profile-annobin-enabled" in files, files assert list(rpms.glob("hello-debuginfo-*.rpm")), sorted(rpms.iterdir()) diff --git a/package/build.bzl b/package/build.bzl index 3f6c9995..ffe70c08 100644 --- a/package/build.bzl +++ b/package/build.bzl @@ -7,7 +7,7 @@ load("//:specs.bzl", "specs") load("//box:defs.bzl", "box") load("//git:defs.bzl", "git") load("//project:defs.bzl", "project") -load(":buildroot.bzl", "BuildrootInfo", "package_buildroot") +load(":buildroot.bzl", "BuildrootInfo") load(":manager.bzl", "PackageManagerInfo") load(":repository.bzl", "LocalPackageInfo") load(":system.bzl", "PackageSystemInfo") @@ -66,8 +66,7 @@ def package_build( owner: str, package_system: Dependency, base: BuildrootInfo, - build_requires: list[str], - buildroot_packages: list[Dependency], + stack: list[Artifact], configured_dev: bool, keep_build_dir: bool, packages: str, @@ -75,7 +74,7 @@ def package_build( category: str, spec: dict[str, typing.Any], ) -> list[Provider]: - """Install `build_requires` over `base`, and run the `build` driver of `package_system` on the result. + """Run the `build` driver of `package_system` on the layer stack `stack` that `base` starts. `spec` contains the fields of the driver spec that the calling rule owns. `packages` names the output directory of the packages. A dev build keeps its build directory between runs if `keep_build_dir` is @@ -95,7 +94,6 @@ def package_build( reserved = [name for name in ("build", "buildroot") if name in subpackages] if reserved: fail("{}: subpackages {} have the names of built-in sub-targets".format(owner, reserved)) - stack = package_buildroot(ctx, owner, base, build_requires, buildroot_packages) out = project.kept_dir(ctx.actions, packages) build_dir = project.kept_dir(ctx.actions, _PRIVATE + "/build") if configured_dev and keep_build_dir else None @@ -118,7 +116,6 @@ def package_build( spec | { "build_dir": build_dir.as_output() if build_dir != None else None, - # The base root is the lowest layer, and the layer of build_requires is above it. "lower": stack, "out": out.as_output(), "subpackages": subpackages, diff --git a/package/buildroot.bzl b/package/buildroot.bzl index 4624194e..c7132e6a 100644 --- a/package/buildroot.bzl +++ b/package/buildroot.bzl @@ -62,19 +62,7 @@ _buildroot = rule( }, ) -def package_buildroot( - ctx: AnalysisContext, - owner: str, - base: BuildrootInfo, - build_requires: list[str], - buildroot_packages: list[Dependency], -) -> list[Artifact]: - """Return the layer stack, bottom to top, that one package builds on. - - The stack is the shared base root, plus a delta that installs `build_requires`. The solve of the - delta prefers the packages of `buildroot_packages` over the repositories of the package manager. - `owner` starts a failure message. - """ +def _extra_packages(owner: str, base: BuildrootInfo, buildroot_packages: list[Dependency]) -> list[Artifact]: package_system = base.package_manager[PackageManagerInfo].package_system extra_packages = [] for dependency in buildroot_packages: @@ -89,6 +77,22 @@ def package_buildroot( ), ) extra_packages.append(info.packages) + return extra_packages + +def package_buildroot( + ctx: AnalysisContext, + owner: str, + base: BuildrootInfo, + build_requires: list[str], + buildroot_packages: list[Dependency], +) -> list[Artifact]: + """Return the layer stack, bottom to top, that one package builds on. + + The stack is the shared base root, plus a delta that installs `build_requires`. The solve of the + delta prefers the packages of `buildroot_packages` over the repositories of the package manager. + `owner` starts a failure message. + """ + extra_packages = _extra_packages(owner, base, buildroot_packages) stack = [base.root] # The packages of buildroot_packages are only candidates for the solve, so they install nothing on @@ -107,6 +111,30 @@ def package_buildroot( ), ] +def recipe_delta( + ctx: AnalysisContext, + owner: str, + base: BuildrootInfo, + stack: list[Artifact], + buildroot_packages: list[Dependency], + requires: Artifact, +) -> Artifact: + """Return a delta over `stack` that installs the JSON list `requires` of build dependencies. + + The solve of the delta prefers the packages of `buildroot_packages` over the repositories of the + package manager. `owner` starts a failure message. + """ + return install_packages( + ctx, + base.package_manager, + base.box, + [], + stack = stack, + extra_packages = _extra_packages(owner, base, buildroot_packages), + install_files = [requires], + identifier = "recipe", + ) + def buildroot(name: str, **kwargs) -> None: if not name.endswith(".buildroot"): fail("buildroot name must end with '.buildroot': {}".format(name)) diff --git a/package/install.bzl b/package/install.bzl index 9322fd7c..1dd655f4 100644 --- a/package/install.bzl +++ b/package/install.bzl @@ -34,11 +34,13 @@ def resolve_packages( extra_packages: list[Artifact] = [], local_seed: list[str] | None = None, identifier: str | None = None, + install_files: list[Artifact] = [], ) -> Artifact: """Plan an install in `box_dep` and select its exact package artifacts. A manager with local packages offers the seed's runtime closure of locally built packages to - the solver; `local_seed` widens that seed beyond this request, e.g. to a whole layer stack.""" + the solver; `local_seed` widens that seed beyond this request, e.g. to a whole layer stack. + `install_files` are JSON lists of install specs that the solve adds to `install`.""" package_manager = package_manager_dep[PackageManagerInfo] configured_repositories, solver_caches = configure_repositories(ctx, package_manager, box_dep) system = package_manager.package_system[PackageSystemInfo] @@ -46,6 +48,10 @@ def resolve_packages( if package_manager.local_packages != None: if extra_packages: fail("resolve_packages: explicit extra packages cannot combine with manager local packages") + + # The seed of local packages has to be known at analysis, and analysis can't read install files. + if install_files: + fail("resolve_packages: install files cannot combine with manager local packages") selected = select_local_packages( package_manager.local_packages[LocalPackageUniverseInfo], local_seed if local_seed != None else install, @@ -79,6 +85,7 @@ def resolve_packages( system = system, repositories = plan_repositories, install = install, + install_files = install_files, arch = package_manager.arch, output = tx.as_output(), solver_caches = solver_caches, @@ -103,17 +110,20 @@ def _install_actions( install: list[str], stack: list[Artifact], extra_packages: list[Artifact], + install_files: list[Artifact] = [], + identifier: str | None = None, ) -> Artifact: package_manager = package_manager_dep[PackageManagerInfo] system = package_manager.package_system[PackageSystemInfo] - closure = resolve_packages(ctx, package_manager_dep, box_dep, install, stack, extra_packages) - out = ctx.actions.declare_output("install.delta" if stack else "root", dir = True) - work = ctx.actions.declare_output("install.work", dir = True) if stack else None + closure = resolve_packages(ctx, package_manager_dep, box_dep, install, stack, extra_packages, identifier = identifier, install_files = install_files) + prefix = identifier + "/" if identifier != None else "" + out = ctx.actions.declare_output(prefix + ("install.delta" if stack else "root"), dir = True) + work = ctx.actions.declare_output(prefix + "install.work", dir = True) if stack else None cmd = cmd_args( box.run(box = box_dep[box.Info], exe = system.install), specs.args( ctx.actions, - "install.spec.json", + prefix + "install.spec.json", { "arch": architecture.spelling(package_manager.arch, system.arch_schema), "box_config": False, @@ -127,7 +137,7 @@ def _install_actions( }, ), ) - ctx.actions.run(cmd, category = "install") + ctx.actions.run(cmd, category = "install", identifier = identifier or "install") return out def _install_packages_impl(ctx: AnalysisContext) -> list[Provider]: @@ -154,6 +164,8 @@ def install_packages( install: list[str], stack: list[Artifact] = [], extra_packages: list[Artifact] = [], + install_files: list[Artifact] = [], + identifier: str | None = None, ) -> Artifact: if not stack: root = ctx.actions.anon_target( @@ -168,4 +180,4 @@ def install_packages( ).artifact("root") # A promise is resolved before the anon target is analysed, so its path kind is asserted here. return ctx.actions.assert_short_path(ctx.actions.assert_has_content_based_path(root), short_path = "root") - return _install_actions(ctx, package_manager, box_dep, install, stack, extra_packages) + return _install_actions(ctx, package_manager, box_dep, install, stack, extra_packages, install_files, identifier) diff --git a/package/installer.py b/package/installer.py index 9fad4eff..7e02df5a 100644 --- a/package/installer.py +++ b/package/installer.py @@ -182,6 +182,13 @@ def run( if layered: if spec["work"] is None: fail(f"{prog}: lower needs a work overlay directory") + # The closure is empty when the lower layers contain every package of the layer, for example in + # the buildroot of a package whose spec needs nothing beyond the base root, or in a box layer + # that adds nothing to its parent. The layer then stays empty, and the package manager doesn't + # run. Buck still expects the work directory as an output. + if not any(packages_dir.iterdir()): + Path(spec["work"]).mkdir(parents=True, exist_ok=True) + return root = rootfs.rootfs( BUILDROOT, lowers=spec["lower"], diff --git a/package/installer_test.py b/package/installer_test.py index 01a48b1f..b241befc 100644 --- a/package/installer_test.py +++ b/package/installer_test.py @@ -53,6 +53,8 @@ def test_package_scripts_tolerate_unmapped_acl_groups(self) -> None: target, lower = root / "target", root / "lower" target.mkdir() lower.mkdir() + (root / "packages").mkdir() + (root / "packages" / "systemd.rpm").write_text("") spec = installer.InstallSpec( arch="x86_64", packages_dir=str(root / "packages"), @@ -95,6 +97,8 @@ def test_a_box_layer_may_add_but_not_delete(self) -> None: lower = root / "lower" (lower / "etc").mkdir(parents=True) (lower / "doomed").write_text("") + (root / "packages").mkdir() + (root / "packages" / "systemd.rpm").write_text("") spec = installer.InstallSpec( arch="x86_64", packages_dir=str(root / "packages"), @@ -125,6 +129,28 @@ def install( else: installer.run("install", install, []) + def test_an_empty_closure_leaves_the_layer_empty(self) -> None: + with tempfile.TemporaryDirectory(dir="/var/tmp") as scratch: + root = Path(scratch) + (root / "lower").mkdir() + (root / "packages").mkdir() + spec = installer.InstallSpec( + arch="x86_64", + packages_dir=str(root / "packages"), + target=str(root / "target"), + installroot=None, + lower=[str(root / "lower")], + work=str(root / "work"), + box_config=False, + langs=[], + docs=True, + ) + install = mock.Mock() + with mock.patch.object(installer.specs, "parse", return_value=spec): + installer.run("install", install, []) + install.assert_not_called() + self.assertEqual(list((root / "target").iterdir()), []) + def test_fresh_root_mount_owns_capture_after_an_install_failure(self) -> None: with tempfile.TemporaryDirectory(dir="/var/tmp") as scratch: root = Path(scratch) diff --git a/package/solver.bzl b/package/solver.bzl index db1ee39d..903fab83 100644 --- a/package/solver.bzl +++ b/package/solver.bzl @@ -17,6 +17,7 @@ def solve_command( install: list[str], arch: str, output: OutputArtifact | None = None, + install_files: list[Artifact] = [], solver_caches: list[Artifact] = [], lowers: list[Artifact] = [], spec_name: str = "solve.spec.json", @@ -24,7 +25,8 @@ def solve_command( """Construct a package solve command for one configured solver context. The transaction destination stays on the command line: the same command is published as a - run target, where the caller names the transaction it wants written. + run target, where the caller names the transaction it wants written. `install_files` are JSON lists + of install specs that the solve adds to `install`. """ command = cmd_args( box_run(box = box, exe = system.plan), @@ -36,6 +38,7 @@ def solve_command( "arch": architecture.spelling(arch, system.arch_schema), "cache": solver_caches, "install": install, + "install_files": install_files, "lower": lowers, "repositories": encode_repositories(repositories), }, diff --git a/package/transaction.py b/package/transaction.py index c3ed92ec..ee6632a1 100644 --- a/package/transaction.py +++ b/package/transaction.py @@ -142,6 +142,19 @@ def resolved_against(repositories: list[Repository]) -> list[TransactionMetadata return entries +def install_specs(install: list[str], install_files: list[str]) -> list[str]: + """Return `install` followed by the install specs of the JSON lists `install_files`, without duplicates. + + An empty result is a solve with nothing to install. A solve that names neither is an error. + """ + if not install and not install_files: + fail("plan: a solve needs at least one install spec") + found = dict.fromkeys(install) + for path in install_files: + found |= dict.fromkeys(json.loads(Path(path).read_text())) + return list(found) + + def write(path: Path, packages: list[TransactionPackage], repositories: list[Repository]) -> None: """Order a transaction before writing it, so identical solves give identical bytes.""" packages.sort( diff --git a/package/transaction_test.py b/package/transaction_test.py index 4c22ff0b..29419ba8 100644 --- a/package/transaction_test.py +++ b/package/transaction_test.py @@ -14,6 +14,26 @@ FILES = [{"out": "InRelease", "sha256": "ab" * 32, "size": 3, "url": "https://mirror.invalid/InRelease"}] +class TestInstallSpecs(unittest.TestCase): + def test_files_extend_the_listed_specs_without_duplicates(self) -> None: + with tempfile.TemporaryDirectory(dir="/var/tmp") as scratch: + path = Path(scratch) / "requires.json" + path.write_text(json.dumps(["meson", "gcc", "jq"])) + self.assertEqual( + transaction.install_specs(["gcc", "make"], [str(path)]), ["gcc", "make", "meson", "jq"] + ) + + def test_an_empty_file_is_a_solve_with_nothing_to_install(self) -> None: + with tempfile.TemporaryDirectory(dir="/var/tmp") as scratch: + path = Path(scratch) / "requires.json" + path.write_text("[]") + self.assertEqual(transaction.install_specs([], [str(path)]), []) + + def test_a_solve_without_specs_or_files_fails(self) -> None: + with self.assertRaisesRegex(SystemExit, "at least one install spec"): + transaction.install_specs([], []) + + class TestWrite(unittest.TestCase): def test_records_the_metadata_a_vouching_repository_was_resolved_against(self) -> None: with tempfile.TemporaryDirectory() as scratch: diff --git a/package_system/deb/plan.py b/package_system/deb/plan.py index 09f68fce..3f692fee 100755 --- a/package_system/deb/plan.py +++ b/package_system/deb/plan.py @@ -35,6 +35,7 @@ class SolveSpec(transaction.Spec): install: list[str] + install_files: list[str] lower: list[str] # The solve spec that all package systems share has `cache`. This driver ignores it, because # APT needs no prebuilt solver cache. @@ -250,13 +251,12 @@ def main(argv: list[str] | None = None) -> None: args = parser.parse_args(argv) spec = specs.load(SolveSpec, args.spec, prog="plan") - if not spec["install"]: - util.fail("plan: a solve needs at least one install spec") + install = transaction.install_specs(spec["install"], spec["install_files"]) repositories = sorted( transaction.load_repositories(spec, absolute=True), key=lambda repository: repository.priority, ) - resolved = solve(repositories, spec["install"], spec["lower"], spec["arch"]) + resolved = solve(repositories, install, spec["lower"], spec["arch"]) if install else [] transaction.write(Path(args.out), resolved, repositories) diff --git a/package_system/deb/plan_test.py b/package_system/deb/plan_test.py index e0d27255..80cecd50 100644 --- a/package_system/deb/plan_test.py +++ b/package_system/deb/plan_test.py @@ -59,6 +59,7 @@ def test_accepts_the_shared_solve_spec_without_prebuilt_caches(self) -> None: "arch": "amd64", "cache": [], "install": ["bash"], + "install_files": [], "lower": ["lower"], "repositories": [], }, @@ -70,6 +71,28 @@ def test_accepts_the_shared_solve_spec_without_prebuilt_caches(self) -> None: solve.assert_called_once_with([], ["bash"], ["lower"], "amd64") self.assertEqual(json.loads(out.read_text(encoding="utf-8")), []) + def test_an_empty_install_file_resolves_nothing(self) -> None: + with tempfile.TemporaryDirectory(dir="/var/tmp") as scratch: + root = Path(scratch) + (root / "requires.json").write_text("[]") + spec = specs.write( + root / "solve.spec.json", + { + "arch": "amd64", + "cache": [], + "install": [], + "install_files": [str(root / "requires.json")], + "lower": ["lower"], + "repositories": [], + }, + ) + out = root / "transaction.json" + with mock.patch.object(plan, "solve") as solve: + plan.main(["solve", "--spec", str(spec), "--out", str(out)]) + + solve.assert_not_called() + self.assertEqual(json.loads(out.read_text(encoding="utf-8")), []) + class TestResolvedPackages(unittest.TestCase): def test_maps_a_fetch_to_the_repository_apt_read_it_from(self) -> None: diff --git a/package_system/pacman/plan.py b/package_system/pacman/plan.py index 096ae493..a2f9d271 100755 --- a/package_system/pacman/plan.py +++ b/package_system/pacman/plan.py @@ -34,6 +34,7 @@ class SolveSpec(transaction.Spec): install: list[str] + install_files: list[str] lower: list[str] # Written by the neutral solve command, unread here: alpm prebuilds no repository metadata. cache: list[str] @@ -100,9 +101,11 @@ def main(argv: list[str] | None = None) -> None: args = parser.parse_args(argv) spec = specs.load(SolveSpec, args.spec, prog="plan") - if not spec["install"]: - fail("plan: a solve needs at least one install spec") + install = transaction.install_specs(spec["install"], spec["install_files"]) repositories = load_repositories(spec) + if not install: + transaction.write(Path(args.out), [], repositories) + return with ExitStack() as stack: scratch = Path(stack.enter_context(TemporaryDirectory(prefix="plan."))) @@ -118,7 +121,7 @@ def main(argv: list[str] | None = None) -> None: solver = stack.enter_context(alpm.Alpm(root, dbpath, spec["arch"])) for repository in repositories: solver.register(repository.id) - resolved = solver.resolve(spec["install"]) + resolved = solver.resolve(install) transaction.write(Path(args.out), resolved_packages(repositories, resolved), repositories) diff --git a/package_system/pacman/plan_test.py b/package_system/pacman/plan_test.py index dbea11a4..2454d0ab 100644 --- a/package_system/pacman/plan_test.py +++ b/package_system/pacman/plan_test.py @@ -44,16 +44,21 @@ def _entry(name: str, **fields: list[str]) -> dict[str, list[str]]: class TestSolve(unittest.TestCase): def solve( - self, install: list[str], *entries: dict[str, list[str]] + self, install: list[str], *entries: dict[str, list[str]], install_file: list[str] | None = None ) -> list[transaction.TransactionPackage]: """Run the planner's solve verb against one repository serving `entries`.""" scratch = Path(tempfile.mkdtemp(prefix="plan-test.")) (scratch / "repo").mkdir() + install_files = [] + if install_file is not None: + (scratch / "install.json").write_text(json.dumps(install_file)) + install_files.append(str(scratch / "install.json")) alpm.write_db([(f"{e['NAME'][0]}-1-1", e) for e in entries], scratch / "repo" / "test.db", EPOCH) spec = { "arch": "x86_64", "cache": [], "install": install, + "install_files": install_files, "lower": [], "repositories": [ { @@ -76,6 +81,14 @@ def test_resolves_a_closure(self) -> None: transaction = self.solve(["a"], _entry("a", DEPENDS=["b"]), _entry("b")) self.assertEqual(sorted(entry["package_id"] for entry in transaction), ["a-1-1", "b-1-1"]) + def test_resolves_the_specs_of_an_install_file(self) -> None: + transaction = self.solve([], _entry("a", DEPENDS=["b"]), _entry("b"), install_file=["a"]) + self.assertEqual(sorted(entry["package_id"] for entry in transaction), ["a-1-1", "b-1-1"]) + + def test_an_empty_install_file_resolves_nothing(self) -> None: + transaction = self.solve([], _entry("a"), install_file=[]) + self.assertEqual(transaction, []) + def test_names_both_sides_of_a_conflict(self) -> None: # What libalpm hands back for this error is a list of conflicts, not of missing # dependencies; read as the latter, the planner used to crash instead of explaining. diff --git a/package_system/rpm/generated.bzl b/package_system/rpm/generated.bzl index 8fafb0e6..9a00cafd 100644 --- a/package_system/rpm/generated.bzl +++ b/package_system/rpm/generated.bzl @@ -82,6 +82,7 @@ def _declare_rpm_package( source_date_epoch = meta.source_date_epoch, subpackages = _by_architecture(meta, lambda rpm_arch: sorted(meta.binaries[rpm_arch])), build_requires = _by_architecture(meta, lambda rpm_arch: _build_requires(meta, rpm_arch)), + imported = True, buildroot_packages = buildroot_packages, in_place_packaging = in_place_packaging, in_place_packaging_distro = in_place_packaging_distro, diff --git a/package_system/rpm/plan.py b/package_system/rpm/plan.py index eb42b552..6220686c 100755 --- a/package_system/rpm/plan.py +++ b/package_system/rpm/plan.py @@ -34,6 +34,7 @@ class SolveSpec(transaction.Spec): install: list[str] + install_files: list[str] lower: list[str] # Prebuilt repository caches (make-cache outputs) seeding this solve. cache: list[str] @@ -237,10 +238,12 @@ def main(argv: list[str] | None = None) -> None: return solve_spec = specs.load(SolveSpec, args.spec, prog="plan") - if not solve_spec["install"]: - fail("plan: a solve needs at least one install spec") + install = transaction.install_specs(solve_spec["install"], solve_spec["install_files"]) repos = load_repositories(solve_spec) seeds = [Path(cache_dir).absolute() for cache_dir in solve_spec["cache"]] + if not install: + transaction.write(Path(args.out), [], repos) + return with ExitStack() as stack: installroot = None @@ -249,7 +252,7 @@ def main(argv: list[str] | None = None) -> None: installroot = stack.enter_context(rootfs.rootfs("/installroot", lowers=solve_spec["lower"])) tx = plan( repos, - solve_spec["install"], + install, CACHEDIR, installroot, solve_spec["arch"], diff --git a/package_system/rpm/plan_test.py b/package_system/rpm/plan_test.py index 1346bca6..3bb3efca 100644 --- a/package_system/rpm/plan_test.py +++ b/package_system/rpm/plan_test.py @@ -7,6 +7,7 @@ libdnf5's actual resolution. """ +import json import subprocess import tempfile import unittest @@ -115,5 +116,25 @@ def test_refuses_a_spec_an_obsoleting_package_displaced(self) -> None: self.assertIn("widget, obsoleted by widget-keys-2-1.noarch", str(failure.exception)) +class TestMain(unittest.TestCase): + def test_an_empty_install_file_writes_an_empty_transaction(self) -> None: + with tempfile.TemporaryDirectory(dir="/var/tmp") as scratch: + root = Path(scratch) + (root / "requires.json").write_text("[]") + spec = { + "arch": "x86_64", + "cache": [], + "install": [], + "install_files": [str(root / "requires.json")], + "lower": [], + "repositories": [], + } + (root / "solve.spec.json").write_text(json.dumps(spec)) + plan.main( + ["solve", "--spec", str(root / "solve.spec.json"), "--out", str(root / "transaction.json")] + ) + self.assertEqual(json.loads((root / "transaction.json").read_text()), []) + + if __name__ == "__main__": unittest.main() diff --git a/package_system/rpm/rules.bzl b/package_system/rpm/rules.bzl index 60b6a374..f3372155 100644 --- a/package_system/rpm/rules.bzl +++ b/package_system/rpm/rules.bzl @@ -3,9 +3,11 @@ """RPM repositories and package-build rules.""" +load("//:specs.bzl", "specs") +load("//box:defs.bzl", "box") load("//distribution:defs.bzl", "distribution") load("//package:build.bzl", "PACKAGE_BUILD_ATTRS", "package_build", "package_build_source") -load("//package:buildroot.bzl", "BuildrootInfo") +load("//package:buildroot.bzl", "BuildrootInfo", "package_buildroot", "recipe_delta") load("//package:packaging.bzl", "pinned_packaging") load("//package:repository.bzl", "RepositoryPin", "declare_remote_repository") load("//package:system.bzl", "PackageSystemInfo") @@ -40,6 +42,32 @@ def rpm_remote_repository( **kwargs, ) +# The fields of the build's driver spec that the BuildRequires query parses the spec with. +_QUERY_FIELDS = ["dist", "in_place_rpmbuild_options", "release", "rpmbuild_options", "source_date_epoch", "source_tree", "sources", "spec_file"] + +def _query_buildrequires( + actions: AnalysisActions, + query: RunInfo, + spec: dict[str, typing.Any], + lower: list[Artifact], + incremental: bool, +) -> Artifact: + """Return a JSON list of the BuildRequires of the spec that the build driver spec `spec` builds.""" + buildrequires = actions.declare_output("__tine/buildrequires.json") + actions.run( + cmd_args( + query, + specs.args( + actions, + "buildrequires.spec.json", + {field: spec[field] for field in _QUERY_FIELDS} | {"incremental": incremental, "lower": lower, "out": buildrequires.as_output()}, + ), + ), + allow_cache_upload = True, + category = "rpm_buildrequires", + ) + return buildrequires + def _rpm_package_impl(ctx: AnalysisContext) -> list[Provider]: base_buildroot = ctx.attrs.buildroot[BuildrootInfo] source_tree = ctx.attrs.source_tree @@ -66,30 +94,47 @@ def _rpm_package_impl(ctx: AnalysisContext) -> list[Provider]: elif in_place_spec != None: # The build takes the Source and Patch files from the directory of the spec in the source tree. sources = [] + spec = { + "dist": ctx.attrs.dist, + "in_place_rpmbuild_options": ctx.attrs.in_place_rpmbuild_options if source_tree != None else [], + "release": ctx.attrs.release, + "rpmbuild_options": ctx.attrs.rpmbuild_options, + "source_date_epoch": ctx.attrs.source_date_epoch, + "source_tree": source_tree, + "sources": sources, + "spec_file": spec_file, + "strict_subpackages": pinned == None, + } + + stack = package_buildroot(ctx, "rpm_package", base_buildroot, ctx.attrs.build_requires, ctx.attrs.buildroot_packages) + + # rpm.branch() derives the buildroot_packages of an imported package from its recorded BuildRequires. A + # BuildRequires that only the spec has would install a package of the branch from the repositories, so + # an imported package always installs the recorded BuildRequires instead. + if (pinned != None or in_place_spec != None) and not ctx.attrs.imported: + buildrequires = _query_buildrequires( + ctx.actions, + box.run(box = base_buildroot.box[box.Info], exe = ctx.attrs._buildrequires), + spec, + stack, + # rpm_package keeps the build directory of a dev build, and build.py adds the incremental + # options to a build with a kept build directory. + ctx.attrs.configured_dev, + ) + stack = stack + [recipe_delta(ctx, "rpm_package", base_buildroot, stack, ctx.attrs.buildroot_packages, buildrequires)] return package_build( ctx, owner = "rpm_package", package_system = ctx.attrs._package_system, base = base_buildroot, - build_requires = ctx.attrs.build_requires, - buildroot_packages = ctx.attrs.buildroot_packages, + stack = stack, configured_dev = ctx.attrs.configured_dev, keep_build_dir = True, packages = "rpms", subpackages = ctx.attrs.subpackages, category = "rpmbuild", - spec = { - "dist": ctx.attrs.dist, - "in_place_rpmbuild_options": ctx.attrs.in_place_rpmbuild_options if source_tree != None else [], - "release": ctx.attrs.release, - "rpmbuild_options": ctx.attrs.rpmbuild_options, - "source_date_epoch": ctx.attrs.source_date_epoch, - "source_tree": source_tree, - "sources": sources, - "spec_file": spec_file, - "strict_subpackages": pinned == None, - }, + spec = spec, ) _rpm_package = rule( @@ -97,6 +142,10 @@ _rpm_package = rule( attrs = PACKAGE_BUILD_ATTRS | { "dist": attrs.string(default = ".aos"), + "imported": attrs.bool( + default = False, + doc = "whether build_requires are the BuildRequires that the importer recorded; the build then installs them without reading the spec", + ), "in_place_rpmbuild_options": attrs.list( attrs.string(), default = [], @@ -120,6 +169,7 @@ _rpm_package = rule( "srcs": attrs.list( attrs.source(allow_directory = True), default = [], doc = "the Source and Patch files of the spec, or directories whose top-level files are such files" ), + "_buildrequires": attrs.exec_dep(providers = [RunInfo], default = "tine//package_system/rpm:buildrequires"), "_package_system": attrs.dep(providers = [PackageSystemInfo], default = PACKAGE_SYSTEM), }, supports_incoming_transition = True, From 634dbe31eb81f86bcbecac1c5b08af23e45bd656 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 3 Nov 2026 21:09:56 +0100 Subject: [PATCH 7/8] importer: Pass every branch property to rpm.branch() The generated BUCK passes each property of _properties.json on its own line, so a new property does nothing until the BUCK is regenerated. Let's pass the whole dict instead. Signed-off-by: Daan De Meyer --- docs/user/importer.md | 9 +++++---- tools/importer.py | 12 +++++------- tools/importer_test.py | 5 ++--- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/docs/user/importer.md b/docs/user/importer.md index 0a9fc8fc..c7060032 100644 --- a/docs/user/importer.md +++ b/docs/user/importer.md @@ -140,7 +140,8 @@ introduce a proper tool verb for that cleanup, but that can wait until the need A `packages/`_distro_`/`_branch_`/` directory may carry a `_properties.json` alongside the per-package `srcpkg.json` files. It holds hand-authored, branch-level build curation. The generated branch `BUCK` -loads it and passes its properties to `rpm.branch()`. Never edit the `@generated` `BUCK` by hand. +loads it and passes every property to `rpm.branch()`, so loading the branch fails on a property that +`rpm.branch()` doesn't know. Never edit the `@generated` `BUCK` by hand. ### Buildroot-only packages @@ -278,6 +279,6 @@ The buildroot still contains the imported BuildRequires. If the pinned spec requ imported spec does not, rpmbuild fails with `Failed build dependencies:` until the package is imported again at a commit with that BuildRequires. -The branch's generated `BUCK` passes `in_place_packaging` to the build only if its `rpm.branch()` call has -an `in_place_packaging` line and a `distro` line. `import`, and an `update` that takes new commits, -regenerate the `BUCK` with both lines. +The branch's generated `BUCK` must pass the branch's `distro` and the properties from `_properties.json` +to `rpm.branch()`. If your `BUCK` doesn't forward `in_place_packaging`, run `import`, or an `update` that +takes new commits, to regenerate it. diff --git a/tools/importer.py b/tools/importer.py index b798cf0c..34f5079e 100755 --- a/tools/importer.py +++ b/tools/importer.py @@ -1424,6 +1424,10 @@ def regenerate_buck(branchdir: Path) -> None: if not has_properties: loads.append("_properties = {}") + # The BUCK passes every property of _properties.json to rpm.branch(), over the defaults, and + # rpm.branch() fails on a property that it doesn't know. A new property then needs no new line in the + # BUCK of an existing branch. + defaults = {"buildroot": f"//buildroots/{distro}:{branch}"} lines = ["# @generated -- do not edit (regenerated on import/update).", *loads, "", "PACKAGES = {"] lines += [f' "{p}": {_load_symbol(p)},' for p in pkgs] lines += [ @@ -1431,14 +1435,8 @@ def regenerate_buck(branchdir: Path) -> None: "", "rpm.branch(", " packages = PACKAGES,", - f' buildroot = _properties.get("buildroot", "//buildroots/{distro}:{branch}"),', - ' buildroot_only_packages = _properties.get("buildroot_only_packages", []),', f' distro = "{distro}",', - ' in_place_packaging = _properties.get("in_place_packaging", {}),', - ' in_place_rpmbuild_options = _properties.get("in_place_rpmbuild_options", {}),', - ' in_place_specs = _properties.get("in_place_specs", {}),', - ' seed_only_packages = _properties.get("seed_only_packages", []),', - ' rpmbuild_options = _properties.get("rpmbuild_options", {}),', + f" **({json.dumps(defaults, sort_keys=True)} | _properties)", ")", ] (branchdir / "BUCK").write_text("\n".join(lines) + "\n") diff --git a/tools/importer_test.py b/tools/importer_test.py index f613e9b1..b431a7fb 100644 --- a/tools/importer_test.py +++ b/tools/importer_test.py @@ -1730,8 +1730,7 @@ def test_curation_is_loaded_not_embedded(self) -> None: self.curate() buck = self.buck() self.assertIn('load(":_properties.json", _properties = "value")', buck) - for key in self.CURATION: - self.assertIn(f'_properties.get("{key}"', buck) + self.assertIn("| _properties)", buck) for value in ( "//buildroots/myos:base", "glibc32", @@ -1768,7 +1767,7 @@ def test_default_buildroot(self) -> None: # Absent an override, the buildroot label is derived from the branch's / # path -- the one default that must be generated, since buck cannot know the branch's # distro. It stays the fallback even when the branch overrides it. - default = '_properties.get("buildroot", "//buildroots/fedora:rawhide")' + default = '**({"buildroot": "//buildroots/fedora:rawhide"} | _properties)' self.assertIn(default, self.buck()) self.curate() self.assertIn(default, self.buck()) From 70f2928d749e9d257e275e14789ee57ba8e2f710 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 14:52:08 +0100 Subject: [PATCH 8/8] rpm: Query the BuildRequires of a hand-written package A hand-written rpm.package only installs the build_requires from its BUCK file, which goes out of sync with the spec. Let's query the spec for every build of a package that wasn't imported. Signed-off-by: Daan De Meyer --- README.md | 1 + docs/design/architecture.md | 10 ++--- docs/user/rpm.md | 42 +++++++++++++++++++++ examples/package/rpm/BUCK | 22 +++++++++++ examples/package/rpm/hello/hello.spec | 9 ++++- examples/package/rpm/hello_archive_check.py | 5 +++ package_system/rpm/rules.bzl | 2 +- 7 files changed, 84 insertions(+), 7 deletions(-) create mode 100644 docs/user/rpm.md diff --git a/README.md b/README.md index 610b7134..ec0e9a12 100644 --- a/README.md +++ b/README.md @@ -207,6 +207,7 @@ User guides: itself embeds in the executables - [Fetching project sources](docs/user/git.md): pinning an external project, or use a local checkout for development +- [Building RPM packages](docs/user/rpm.md): building a package from a spec, its build dependencies - [Maintaining packages](docs/user/importer.md): importing and updating packages from upstream distributions, local modifications, branch curation - [Development boxes](docs/user/box.md): pinned interactive development environments diff --git a/docs/design/architecture.md b/docs/design/architecture.md index fb877717..93e51de1 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -714,11 +714,11 @@ An `rpm_package` action: 1. obtains the shared base root from its `BuildrootInfo`; 2. resolves and installs its BuildRequires delta, preferring RPMs from `buildroot_packages` over upstream. A - source-tree build of a hand-written `rpm.package()` with `in_place_spec` or `in_place_packaging` also - installs the BuildRequires of its spec. Analysis cannot read the contents of a spec, so an action stages - the spec as the build does and runs `rpmbuild -br` with the build's options in the base root and the - delta. A second delta installs the BuildRequires that it writes. That delta is empty when the spec needs - nothing more; +2. resolves and installs its BuildRequires delta, preferring RPMs from `buildroot_packages` over upstream. A + hand-written `rpm.package()` also installs the BuildRequires of the spec that it builds. Analysis cannot + read the contents of a spec, so an action stages the spec as the build does and runs `rpmbuild -br` with + the build's options in the base root and the delta. A second delta installs the BuildRequires that it + writes. That delta is empty when the spec needs nothing more; 3. overlays the base and delta, stages its spec/sources in Buck action scratch, and runs `rpmbuild -ba`, or `rpmbuild -bb --build-in-place` on a volatile overlay of a populated source tree. overlayfs shows the directory under a mount rather than the mounted tree, so each directory mount below the source tree, diff --git a/docs/user/rpm.md b/docs/user/rpm.md new file mode 100644 index 00000000..6bd178ac --- /dev/null +++ b/docs/user/rpm.md @@ -0,0 +1,42 @@ + + +# Building RPM packages + +`rpm.package()` builds RPM packages from a spec and its Source and Patch files. The importer declares the +packages that it brings in from a distribution; see [Maintaining packages](importer.md). + +## Building a package from a spec + +Put the spec and the files that it names in the directory of the `BUCK` file or below it, and declare the +package: + +```starlark +load("@tine//package_system/rpm:defs.bzl", "rpm") + +rpm.package( + name = "hello", + package = "hello", + spec = "hello/hello.spec", + srcs = ["hello/hello-2.12.3.tar.gz"], + buildroot = "catalog//:fedora.rawhide.buildroot", + release = "1", + source_date_epoch = 1754006400, + subpackages = ["hello"], +) +``` + +`package` is the `Name:` of the spec, and `subpackages` lists the binary packages that the build produces. +Each subpackage is a sub-target, such as `:hello[hello]`. + +## Build dependencies + +The build installs the BuildRequires of the spec over the shared base buildroot. `build_requires` lists +packages that the build installs before it reads the spec, such as tools or RPM macros that the spec uses +while parsing or in `%prep`. Before the build, `rpmbuild -br` reads the spec in that buildroot with the +build's rpmbuild options, so the spec's conditionals select the same BuildRequires as in the build. + +`rpmbuild -br` runs `%generate_buildrequires` once. If the script needs packages of its own before it +reports everything, list them in `build_requires`. diff --git a/examples/package/rpm/BUCK b/examples/package/rpm/BUCK index 546526ad..bbb0e5b2 100644 --- a/examples/package/rpm/BUCK +++ b/examples/package/rpm/BUCK @@ -41,6 +41,28 @@ box.sh_test( args = ["$(source hello_archive_check.py)", "$(location :hello-archive)"], ) +rpm.package( + name = "hello-parse-helper", + distro = _DISTRO, + package = "hello", + spec = "hello/hello.spec", + build_requires = ["jq"], + buildroot = _BUILDROOT, + release = "1", + rpmbuild_options = ["--define=version_override %(jq -nr '\"1\"')"], + source_date_epoch = 1754006400, + subpackages = ["hello"], +) + +box.sh_test( + name = "hello-parse-helper-test", + distro = _DISTRO, + labels = ["image"], + box = _CHECK_BOX, + script = 'python3 "$@"', + args = ["$(source hello_archive_check.py)", "$(location :hello-parse-helper)"], +) + # The explicit override keeps this source-tree fixture in dev mode without depending on the # user's root config. rpm.package( diff --git a/examples/package/rpm/hello/hello.spec b/examples/package/rpm/hello/hello.spec index a5355b43..afd6729c 100644 --- a/examples/package/rpm/hello/hello.spec +++ b/examples/package/rpm/hello/hello.spec @@ -2,11 +2,15 @@ # SPDX-License-Identifier: MPL-2.0 Name: hello -Version: 1 +Version: %{?version_override}%{!?version_override:1} Release: %autorelease Summary: Exercise regular RPM package builds License: MPL-2.0 BuildArch: noarch +# The base buildroot doesn't contain jq. +%if "%{getenv:HOME}" == "/build" && "%{getenv:SOURCE_DATE_EPOCH}" == "1754006400" +BuildRequires: jq +%endif %description An integration fixture for tine's regular RPM package build path. @@ -14,10 +18,13 @@ An integration fixture for tine's regular RPM package build path. %prep %build +jq -n '"archive"' > jq %install install -d %{buildroot}%{_datadir}/tine-package touch %{buildroot}%{_datadir}/tine-package/archive +install -pm0644 jq %{buildroot}%{_datadir}/tine-package/built-with-jq %files %{_datadir}/tine-package/archive +%{_datadir}/tine-package/built-with-jq diff --git a/examples/package/rpm/hello_archive_check.py b/examples/package/rpm/hello_archive_check.py index 28558977..4b22e705 100644 --- a/examples/package/rpm/hello_archive_check.py +++ b/examples/package/rpm/hello_archive_check.py @@ -16,4 +16,9 @@ query = ["rpm", "-qp", "--queryformat", "%{PAYLOADCOMPRESSOR}", rpm] compressor = subprocess.run(query, check=True, stdout=subprocess.PIPE, text=True).stdout assert compressor in {"gzip", "bzip2", "xz", "lzma", "zstd"}, (rpm, compressor) +[binary] = [rpm for rpm in rpms if rpm not in sources] +files = subprocess.run( + ["rpm", "-qpl", binary], check=True, stdout=subprocess.PIPE, text=True +).stdout.splitlines() +assert "/usr/share/tine-package/built-with-jq" in files, files print(f"archive: {len(rpms)} packages with compressed payloads") diff --git a/package_system/rpm/rules.bzl b/package_system/rpm/rules.bzl index f3372155..363fc4b9 100644 --- a/package_system/rpm/rules.bzl +++ b/package_system/rpm/rules.bzl @@ -111,7 +111,7 @@ def _rpm_package_impl(ctx: AnalysisContext) -> list[Provider]: # rpm.branch() derives the buildroot_packages of an imported package from its recorded BuildRequires. A # BuildRequires that only the spec has would install a package of the branch from the repositories, so # an imported package always installs the recorded BuildRequires instead. - if (pinned != None or in_place_spec != None) and not ctx.attrs.imported: + if not ctx.attrs.imported: buildrequires = _query_buildrequires( ctx.actions, box.run(box = base_buildroot.box[box.Info], exe = ctx.attrs._buildrequires),