From e3065c2e10af46719ebfec6891b214ee0fc17708 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 14:42:44 +0100 Subject: [PATCH 1/7] 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 with the same spec fields, so let's move both into rpmbuild.py. Pinned packaging comes in as a directory in sources, and a strict_subpackages flag replaces the packaging field. The copy into SOURCES runs outside the chroot and follows symlinks, so we now refuse a symlink in the path of the spec or of a source, and at the top of a source directory. Signed-off-by: Daan De Meyer --- docs/design/architecture.md | 5 +- package_system/rpm/BUCK | 12 ++- package_system/rpm/build.py | 115 +++----------------- package_system/rpm/build_test.py | 19 ++-- package_system/rpm/rpmbuild.py | 156 ++++++++++++++++++++++++++++ package_system/rpm/rpmbuild_test.py | 97 +++++++++++++++++ package_system/rpm/rules.bzl | 18 +++- 7 files changed, 304 insertions(+), 118 deletions(-) create mode 100644 package_system/rpm/rpmbuild.py create mode 100644 package_system/rpm/rpmbuild_test.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..c6bb7d2c 100644 --- a/package_system/rpm/BUCK +++ b/package_system/rpm/BUCK @@ -38,13 +38,21 @@ 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.test( + name = "rpmbuild-test", + srcs = ["rpmbuild.py", "rpmbuild_test.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..6feafb54 100755 --- a/package_system/rpm/build.py +++ b/package_system/rpm/build.py @@ -20,34 +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): +class Spec(builder.Spec, rpmbuild.Parse): """Buck's generated build invocation, distinct from the package's RPM spec.""" - spec_file: str - sources: list[str] - dist: str - source_date_epoch: int - release: str - # 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 +38,23 @@ 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) - - 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"]), - } + staged = rpmbuild.stage(topdir, spec, source_tree) + 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, spec) 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 +79,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..5fc0904d 100644 --- a/package_system/rpm/build_test.py +++ b/package_system/rpm/build_test.py @@ -41,24 +41,28 @@ 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", + # rpm_package keeps the build directory of every dev build, and only of a dev build. + incremental=build_dir is not None, 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 +194,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) @@ -216,7 +222,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..3ec9f883 --- /dev/null +++ b/package_system/rpm/rpmbuild.py @@ -0,0 +1,156 @@ +# 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.""" + +from pathlib import Path +from typing import NamedTuple, TypedDict + +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 Parse(TypedDict): + """The fields of a driver spec that select how rpm parses the 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 + release: str + rpmbuild_options: list[str] + # Extra switches for a build against a source tree, after the common ones. + in_place_rpmbuild_options: list[str] + # `incremental` is True for a dev build, which keeps its build directory and adds INCREMENTAL_OPTIONS. + incremental: bool + + +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()) + return files + + +def stage(topdir: Path, parse: Parse, source_tree: str | None) -> Staged: + """Stage the spec and the sources of `parse` 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 + spec_file = Path(parse["spec_file"]) + 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(parse["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 {parse['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, parse: Parse) -> list[str]: + """Return the macro definitions and options that rpmbuild parses the spec `staged` with.""" + arguments: list[str] = [ + "--define", f"_topdir {builder.CHROOT_TOPDIR}", + "--define", f"dist {parse['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 parse["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 += parse["rpmbuild_options"] + (INCREMENTAL_OPTIONS if parse["incremental"] else []) + return arguments + parse["in_place_rpmbuild_options"] diff --git a/package_system/rpm/rpmbuild_test.py b/package_system/rpm/rpmbuild_test.py new file mode 100644 index 00000000..ec459486 --- /dev/null +++ b/package_system/rpm/rpmbuild_test.py @@ -0,0 +1,97 @@ +# SPDX-FileCopyrightText: Amutable GmbH +# SPDX-License-Identifier: MPL-2.0 + +"""Tests for the staging of an RPM spec and its sources in an rpmbuild tree. + +buck test tine//package_system/rpm:rpmbuild-test +""" + +import tempfile +import unittest +from pathlib import Path +from typing import override + +import rpmbuild + + +class Stage(unittest.TestCase): + @override + def setUp(self) -> None: + self.scratch = Path( + self.enterContext(tempfile.TemporaryDirectory(prefix="rpmbuild-test.", dir="/var/tmp")) + ) + self.spec_file = self.scratch / "example.spec" + self.spec_file.write_text("Name: example\n") + self.source = self.scratch / "source" + self.source.write_text("source input\n") + self.topdir = self.scratch / "topdir" + + def stage(self, *, spec_file: Path | None = None, sources: list[Path] | None = None) -> rpmbuild.Staged: + parse = rpmbuild.Parse( + spec_file=str(spec_file or self.spec_file), + sources=[str(source) for source in (sources if sources is not None else [self.source])], + dist=".test", + source_date_epoch=1754006400, + release="7", + rpmbuild_options=[], + in_place_rpmbuild_options=[], + incremental=False, + ) + return rpmbuild.stage(self.topdir, parse, None) + + def directory(self) -> Path: + directory = self.scratch / "directory" + (directory / "tests").mkdir(parents=True) + (directory / "fix.patch").write_text("patch\n") + (directory / "tests" / "test.sh").write_text("test\n") + return directory + + def test_a_directory_contributes_its_top_level_files(self) -> None: + # A dist-git repository contains its Source and Patch files at the top level. Stage a directory with + # a file at the top and one in a subdirectory, and check that SOURCES contains only the top one. + self.stage(sources=[self.directory()]) + self.assertEqual(sorted(path.name for path in (self.topdir / "SOURCES").iterdir()), ["fix.patch"]) + + def test_a_symlink_at_the_top_of_a_source_directory_fails(self) -> None: + # The copy into SOURCES follows symlinks. A symlink at the top of a fetched tree must not copy the + # file that it points to. + directory = self.directory() + (directory / "secret").symlink_to(self.source) + with self.assertRaisesRegex(SystemExit, "contains symlinks at its top level: secret"): + self.stage(sources=[directory]) + + def test_a_symlinked_source_directory_fails(self) -> None: + # A source directory that is itself a symlink fails to stage. + linked = self.scratch / "linked" + linked.symlink_to(self.directory()) + with self.assertRaisesRegex(SystemExit, "the source is a symlink"): + self.stage(sources=[linked]) + + def test_a_source_directory_beneath_a_symlink_fails(self) -> None: + # A source directory below a symlinked directory fails to stage, like a symlink of its own. + external = self.scratch / "external" + (external / "nested").mkdir(parents=True) + (external / "nested" / "helper").write_text("external input\n") + tree = self.scratch / "tree" + tree.mkdir() + (tree / "rpm").symlink_to(external, target_is_directory=True) + with self.assertRaisesRegex(SystemExit, "the source is a symlink"): + self.stage(sources=[tree / "rpm" / "nested"]) + + def test_a_spec_beneath_a_symlink_fails(self) -> None: + # stage() reads the spec outside the chroot. A spec below a symlinked directory fails to stage. + linked = self.scratch / "linked" + linked.symlink_to(self.scratch, target_is_directory=True) + with self.assertRaisesRegex(SystemExit, "RPM spec is a symlink"): + self.stage(spec_file=linked / self.spec_file.name) + + def test_a_symlinked_spec_fails(self) -> None: + # A spec that is a symlink fails to stage. + linked = self.scratch / "linked.spec" + linked.symlink_to(self.spec_file) + with self.assertRaisesRegex(SystemExit, "RPM spec is a symlink"): + self.stage(spec_file=linked) + + +if __name__ == "__main__": + unittest.main() diff --git a/package_system/rpm/rules.bzl b/package_system/rpm/rules.bzl index 32fd8402..f897720c 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,15 @@ 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, + # package_build() keeps the build directory of a dev build, because rpm_package passes keep_build_dir. + "incremental": ctx.attrs.configured_dev, "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, }, ) From 29f95c8733e4ac488a5734069fbe31b8b2254a6f Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 16:20:51 +0100 Subject: [PATCH 2/7] 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 that rpm finds the files that the spec includes. rpm resolves %include from its working directory and not from the spec, so the copy doesn't need to be there. Let's write the copy to SPECS like any other spec. Staging then never writes into the checkout. 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 5fc0904d..beb2bb5f 100644 --- a/package_system/rpm/build_test.py +++ b/package_system/rpm/build_test.py @@ -162,11 +162,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", @@ -174,7 +178,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()), []) @@ -375,7 +379,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 3ec9f883..8a150d2b 100644 --- a/package_system/rpm/rpmbuild.py +++ b/package_system/rpm/rpmbuild.py @@ -86,25 +86,23 @@ def stage(topdir: Path, parse: Parse, source_tree: str | None) -> Staged: (topdir / builder.BUILD).mkdir(exist_ok=True) checkout = topdir / builder.CHECKOUT spec_file = Path(parse["spec_file"]) + 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(parse["sources"]): # A spec may modify SOURCES, so it must not share the source artifact's inode. @@ -113,8 +111,9 @@ def stage(topdir: Path, parse: Parse, source_tree: str | None) -> Staged: 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 {parse['release']}%{{?dist}}\n%global autochangelog %{{nil}}\n" + source_spec.read_text() From 05711e9ee69c208e356a707fa121e9a736db6fe0 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 14:45:03 +0100 Subject: [PATCH 3/7] rpm: Add a driver that queries the BuildRequires of a spec Nothing reads the BuildRequires of the spec that a build uses, so the buildroot only gets what the BUCK file lists. Let's add a driver that stages the spec like build.py, runs rpmbuild -bd in the buildroot with the same arguments as the build, and writes the BuildRequires that the buildroot lacks. Unlike -br, -bd doesn't check that the Source and Patch files exist, and an in-place build doesn't need the Source archive. 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 | 119 ++++++++ package_system/rpm/buildrequires_test.py | 357 +++++++++++++++++++++++ 6 files changed, 496 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 c6bb7d2c..2b9c8ab2 100644 --- a/package_system/rpm/BUCK +++ b/package_system/rpm/BUCK @@ -55,6 +55,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 6feafb54..b80d0bf6 100755 --- a/package_system/rpm/build.py +++ b/package_system/rpm/build.py @@ -41,7 +41,7 @@ def _build_rpm(spec: Spec, topdir: Path, out: Path, subpackages: Path) -> int: source_tree = spec["source_tree"] staged = rpmbuild.stage(topdir, spec, source_tree) env = os.environ | rpmbuild.environment(spec["source_date_epoch"]) - with builder.buildroot(spec, topdir): + with builder.buildroot(spec["lower"], topdir): arguments = rpmbuild.arguments(staged, spec) mode = ["-ba"] cwd = staged.cwd diff --git a/package_system/rpm/buildrequires.py b/package_system/rpm/buildrequires.py new file mode 100755 index 00000000..45f67bd1 --- /dev/null +++ b/package_system/rpm/buildrequires.py @@ -0,0 +1,119 @@ +#!/usr/bin/python3 + +# SPDX-FileCopyrightText: Amutable GmbH +# SPDX-License-Identifier: MPL-2.0 + +"""Write the BuildRequires of an RPM spec that the buildroot lacks as a JSON list. + +The driver stages the spec as build.py does and runs `rpmbuild -bd` 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. +""" + +import itertools +import json +import os +import subprocess +import sys +import tempfile +from contextlib import ExitStack +from pathlib import Path + +import specs +import util + +import builder +import rootfs +import rpmbuild + + +class Spec(rpmbuild.Parse): + # `lower` lists the layers of the buildroot with declared BuildRequires installed, bottom first. + lower: list[str] + # `source_tree` is the source tree of an in-place build, or None. + source_tree: str | None + out: str + + +_HEADER = "error: Failed build dependencies:" + + +def query(spec: Spec) -> list[str]: + """Return the BuildRequires of the spec of `spec` that the buildroot lacks.""" + # The driver parses the messages of rpmbuild, which rpm translates otherwise. + env = os.environ | rpmbuild.environment(spec["source_date_epoch"]) | {"LC_ALL": "C.UTF-8"} + 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: + # %prep can leave read-only directories in topdir, and Buck must be able to remove them. The stack + # exits in reverse order. capture_on_exit() then runs after source_overlay() unmounts the checkout. + stack.enter_context(rootfs.capture_on_exit(topdir)) + if source_tree is not None: + stack.enter_context(rootfs.source_overlay(Path(source_tree), topdir / builder.CHECKOUT)) + staged = rpmbuild.stage(topdir, spec, source_tree) + arguments = rpmbuild.arguments(staged, spec) + # rpmbuild -bd checks the static BuildRequires against the rpm database of the buildroot first. It + # runs %prep and %generate_buildrequires only when the buildroot has all of them, and then checks + # what the script reports. -bd doesn't package the sources, so it doesn't check that the Source and + # Patch files exist. An in-place spec can then name a Source archive that the checkout replaces. + mode = ["-bd"] + # 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)) + result = subprocess.run( + ["/usr/bin/rpmbuild", *arguments, *mode, str(staged.spec)], + cwd=staged.cwd, + env=env, + stdout=sys.stderr, + stderr=subprocess.PIPE, + text=True, + # %prep and %generate_buildrequires write to the same stderr as rpmbuild, in any encoding. + errors="replace", + ) + sys.stderr.write(result.stderr) + if result.returncode == 0: + return [] + # rpmbuild exits with 11 when BuildRequires are missing or BuildConflicts are installed. It lists each + # problem on a tab-indented line after a header: + # + # error: Failed build dependencies: + # meson >= 0.60 is needed by example-1-1.x86_64 + # + # rpmbuild checks the dependencies after the scripts run, so only the lines after the last header are + # its own. + lines = result.stderr.splitlines() + problems: list[str] = [] + if result.returncode == 11 and _HEADER in lines: + start = len(lines) - lines[::-1].index(_HEADER) + problems = [ + line.strip() for line in itertools.takewhile(lambda line: line.startswith("\t"), lines[start:]) + ] + if conflicts := [problem for problem in problems if " conflicts with " in problem]: + util.fail( + f"buildrequires: the buildroot contains BuildConflicts of {spec['spec_file']}: {conflicts}" + ) + missing = {problem.rsplit(" is needed by ", 1)[0] for problem in problems if " is needed by " in problem} + if not missing: + util.fail( + f"buildrequires: rpmbuild -bd failed with exit status {result.returncode}: {spec['spec_file']}" + ) + return sorted(missing) + + +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 every %(...) of the spec. For a spec with %generate_buildrequires, it also runs the + # script, and %prep in an archive build. The project is read-only meanwhile, as during the build. + with rootfs.readonly_project(Path.cwd(), {}): + missing = query(spec) + Path(spec["out"]).write_text(json.dumps(missing, indent=2) + "\n") + print(f"{spec['spec_file']}: {len(missing)} missing 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..f8108953 --- /dev/null +++ b/package_system/rpm/buildrequires_test.py @@ -0,0 +1,357 @@ +# 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: example-compiler +BuildRequires: example-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. +""" + +GENERATOR_SPEC = """\ +Name: example +Version: 1 +Release: 1 +Summary: %generate_buildrequires fixture +License: MPL-2.0 +BuildRequires: /usr/bin/rpmbuild + +%description +A fixture. + +%generate_buildrequires +echo generated-devel +""" + +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", + "example-compiler", + "example-meson >= 0.60", + "lto-tool", + "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: foo-bar-devel\n%endif\n" + "%if %{defined _without_baz_qux}\nBuildRequires: baz-qux-devel\n%endif\n%description", + ) + ) + found = buildrequires.query( + self.specification(rpmbuild_options=["--with=foo-bar", "--without=baz-qux"]) + ) + self.assertIn("foo-bar-devel", found) + self.assertIn("baz-qux-devel", 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_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("example-compiler", json.loads(Path(spec["out"]).read_text())) + + def test_unparsable_spec_fails(self) -> None: + self.spec_file.write_text("Name: example\n%if\n") + with self.assertRaisesRegex(SystemExit, "rpmbuild -bd failed"): + buildrequires.query(self.specification()) + + def test_missing_static_buildrequires_are_listed_before_generated_ones(self) -> None: + # rpmbuild -bd checks the static BuildRequires before it runs %generate_buildrequires. The script can + # need a static BuildRequires, such as the package of its macros. With a static BuildRequires + # missing, the query lists only 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("example-compiler", found) + self.assertNotIn("generated-devel", found) + + def test_generated_buildrequires_are_listed_once_the_static_ones_are_installed(self) -> None: + # The box contains /usr/bin/rpmbuild, so the static BuildRequires is installed. rpmbuild -bd then + # runs %generate_buildrequires, and the query lists what the script reports. + self.spec_file.write_text(GENERATOR_SPEC) + self.assertEqual(buildrequires.query(self.specification()), ["generated-devel"]) + + def test_an_installed_spec_lists_nothing(self) -> None: + # The query lists nothing when the buildroot contains every BuildRequires. + self.spec_file.write_text( + GENERATOR_SPEC.replace("%generate_buildrequires\necho generated-devel\n", "") + ) + self.assertEqual(buildrequires.query(self.specification()), []) + + def test_an_in_place_spec_needs_no_source_archive(self) -> None: + # The source tree of an in-place build replaces the Source archive of the spec, and the build never + # reads it. Create an in-place spec whose Source0 and Patch0 aren't in the checkout, and check that + # the query lists its BuildRequires. + checkout = self.scratch / "checkout" + checkout.mkdir() + (checkout / "extra-buildrequires").write_text("source-devel\n") + (checkout / "example.spec").write_text( + SPEC.replace( + "License: MPL-2.0\n", "License: MPL-2.0\nSource0: example-1.tar.gz\nPatch0: fix.patch\n" + ) + ) + found = buildrequires.query( + self.specification(spec_file=checkout / "example.spec", sources=[], source_tree=checkout) + ) + self.assertIn("example-compiler", found) + + def test_script_output_does_not_add_buildrequires(self) -> None: + # %generate_buildrequires writes to the same stderr as rpmbuild. A line of the script that looks like + # a missing BuildRequires, or that isn't UTF-8, must not change the result. Only the lines after + # rpmbuild's last header count. + self.spec_file.write_text( + GENERATOR_SPEC.replace( + "echo generated-devel\n", + "printf 'error: Failed build dependencies:\\n' >&2\n" + "printf '\\tbogus-devel is needed by example\\n\\377\\n' >&2\n" + "echo generated-devel\n", + ) + ) + self.assertEqual(buildrequires.query(self.specification()), ["generated-devel"]) + + def test_an_installed_buildconflicts_fails(self) -> None: + # rpmbuild exits with 11 for an installed BuildConflicts as well. The box contains rpm-build, so a + # spec that conflicts with it fails and names the conflict. + self.spec_file.write_text( + GENERATOR_SPEC.replace("BuildRequires: /usr/bin/rpmbuild\n", "BuildConflicts: rpm-build\n") + ) + with self.assertRaisesRegex(SystemExit, "BuildConflicts.*rpm-build conflicts with"): + buildrequires.query(self.specification()) + + +if __name__ == "__main__": + unittest.main() From 427aede7cbc9bfd26fef21db924778a2adb47947 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 3 Nov 2026 21:08:13 +0100 Subject: [PATCH 4/7] 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 --- examples/package/rpm/BUCK | 23 +++++++++++++++++++++++ package/buildroot.bzl | 10 +++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/examples/package/rpm/BUCK b/examples/package/rpm/BUCK index ae0d2964..fc354c65 100644 --- a/examples/package/rpm/BUCK +++ b/examples/package/rpm/BUCK @@ -19,6 +19,29 @@ _CHECK_BOX = "catalog//:fedora.rawhide.box" git.checkout(name = "hello.source") +# rpm.branch() derives buildroot_packages from the BuildRequires of every architecture. A package can then +# have buildroot_packages and no build_requires on the architecture that it builds for. +rpm.package( + name = "hello-buildroot-packages", + distro = _DISTRO, + package = "hello", + spec = "hello/hello.spec", + buildroot = _BUILDROOT, + buildroot_packages = [":hello-archive"], + release = "1", + source_date_epoch = 1754006400, + subpackages = ["hello"], +) + +box.sh_test( + name = "hello-buildroot-packages-test", + distro = _DISTRO, + labels = ["image"], + box = _CHECK_BOX, + script = 'python3 "$@"', + args = ["$(source hello_archive_check.py)", "$(location :hello-buildroot-packages)"], +) + # The archive path runs %prep and emits both binary and source RPMs. It exists separately from the # source-tree fixture to check release and dev compression through real RPM headers. rpm.package( 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 f73e245f20899c3fa92bedcc0ad853e94a4b0f82 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 3 Nov 2026 21:08:27 +0100 Subject: [PATCH 5/7] rpm: Don't check rich BuildRequires for delivery undelivered() checks that each install spec ended up in the transaction. resolve_pkg_spec() doesn't evaluate rich dependencies, so for (A if B) it reports A as missing when B isn't installed. Let's skip rich dependencies in that check. The solve itself still fails when it can't satisfy one. Signed-off-by: Daan De Meyer --- package_system/rpm/plan.py | 5 +++++ package_system/rpm/plan_test.py | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/package_system/rpm/plan.py b/package_system/rpm/plan.py index be6d2ca9..5d633e95 100755 --- a/package_system/rpm/plan.py +++ b/package_system/rpm/plan.py @@ -133,6 +133,11 @@ def undelivered( if spec.removeprefix("@") not in groups: missing.append(spec) continue + # resolve_pkg_spec() doesn't evaluate rich dependencies. For (A if B), it would report A as missing + # while B is absent, although the dependency holds. The solve still requests every rich dependency, + # and it fails when no package satisfies one, such as (A or B) with neither A nor B available. + 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..6a36a995 100644 --- a/package_system/rpm/plan_test.py +++ b/package_system/rpm/plan_test.py @@ -94,6 +94,28 @@ 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_an_alternative_spec_installs_the_available_package(self) -> None: + # The solve satisfies (A or B) with the package that the repository contains. + repo = self.repository("alternative", self.build("gadget", "1")) + self.assertEqual(self.solve(["(widget or gadget)"], repo), ["gadget-1-1.noarch"]) + + def test_an_alternative_spec_without_any_package_fails(self) -> None: + # undelivered() skips rich dependencies. The solve itself must fail on (A or B) when the + # repository contains neither A nor B. + repo = self.repository("no-alternative", self.build("gadget", "1")) + with self.assertRaises(SystemExit): + self.solve(["(widget or gizmo)"], repo) + def test_refuses_a_spec_an_obsoleting_package_displaced(self) -> None: """split-out subpackage obsoleting its parent of older version is refused.""" From 724d7b8773ae8f1fb32da4ed1cc5f281a3a0d911 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 14:46:32 +0100 Subject: [PATCH 6/7] 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 the missing ones as an extra layer. The solve reads them from the query's JSON file and installs nothing when the list is empty. Imported packages keep their recorded BuildRequires, because rpm.branch() derives buildroot_packages from them. Signed-off-by: Daan De Meyer --- docs/design/architecture.md | 20 ++++- 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 | 11 +-- package/buildroot.bzl | 54 +++++++++--- package/install.bzl | 26 ++++-- package/installer.py | 7 ++ package/installer_test.py | 30 +++++++ 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 | 77 ++++++++++++++---- 24 files changed, 302 insertions(+), 63 deletions(-) diff --git a/docs/design/architecture.md b/docs/design/architecture.md index a72b5e7a..0c17a65c 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -604,7 +604,9 @@ Native package installation has three phases shared by buildroots and images: 1. **Plan.** Run `PackageSystemInfo.plan` against each configured repository's materialized directory, effective priority, and solver cache. Weak dependencies are disabled. Existing lower layers are mounted - read-only so installed packages can satisfy an incremental request. + read-only so installed packages can satisfy an incremental request. A request lists its packages in the + analysis, or in JSON files that an earlier action writes, such as the BuildRequires query of an rpm + package. A request with no packages installs nothing and leaves its layer empty. 2. **Select.** Use the resulting transaction to select package files from repository pools, verified after download if the repository declares signing keys, named with the package system's declared suffix so its installer finds them. A local repository is materialized by @@ -713,8 +715,13 @@ 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; -3. overlays the base and delta, stages its spec/sources in Buck action scratch, and runs `rpmbuild -ba`, +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 -bd` with the build's options in the base root and the + delta. It writes the BuildRequires that are missing, and a second delta installs them. That delta is + empty when the spec needs nothing more; +3. overlays the base and the deltas, 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, such as a `tine mount`, gets a volatile overlay of its own. The driver discovers directory mounts with @@ -739,7 +746,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` + only when the base root and the declared delta contain every static BuildRequires, and runs it once. +- 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 fc354c65..9c925de0 100644 --- a/examples/package/rpm/BUCK +++ b/examples/package/rpm/BUCK @@ -71,7 +71,6 @@ rpm.package( distro = _DISTRO, package = "hello", buildroot = _BUILDROOT, - build_requires = ["gcc"], dev = True, in_place_spec = "rpm/hello.spec", release = "1", @@ -100,7 +99,6 @@ rpm.package( distro = _DISTRO, package = "hello", buildroot = _BUILDROOT, - build_requires = ["gcc"], in_place_spec = "rpm/hello.spec", release = "1", source_date_epoch = 1754006400, @@ -128,7 +126,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, @@ -154,7 +151,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..8408778c 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") @@ -27,7 +27,7 @@ PACKAGE_BUILD_ATTRS = { "buildroot_packages": attrs.list( attrs.dep(providers = [LocalPackageInfo]), default = [], - doc = "our packages that the solve of build_requires prefers over upstream ones", + doc = "our packages that the solves of the BuildRequires prefer over upstream ones", ), "configured_dev": attrs.bool( doc = "whether project configuration selects this package for dev mode", @@ -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..0d9556f6 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,32 @@ def install( else: installer.run("install", install, []) + def test_an_empty_closure_leaves_the_layer_empty(self) -> None: + # A layered install with no packages to install must not run the package manager. Buck still + # expects the work directory as an output. Install an empty package directory over one lower + # layer, and check that the layer is empty and that the work directory exists. + 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()), []) + self.assertTrue((root / "work").is_dir()) + 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 5d633e95..9045461d 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] @@ -238,10 +239,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 @@ -250,7 +253,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 6a36a995..7adcbf90 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 @@ -127,5 +128,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 f897720c..13e6d263 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,30 @@ def rpm_remote_repository( **kwargs, ) +def _query_buildrequires( + actions: AnalysisActions, + query: RunInfo, + parse: dict[str, typing.Any], + source_tree: Artifact | None, + lower: list[Artifact], +) -> Artifact: + """Return a JSON list of the BuildRequires of the spec that `parse` describes and the layers `lower` lack.""" + buildrequires = actions.declare_output("__tine/buildrequires.json") + actions.run( + cmd_args( + query, + specs.args( + actions, + "buildrequires.spec.json", + parse | {"lower": lower, "out": buildrequires.as_output(), "source_tree": source_tree}, + ), + ), + # A dev build queries a source tree that changes with every edit, so no other build reuses the result. + allow_cache_upload = not parse["incremental"], + 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,32 +92,46 @@ 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 = [] + # The build and the BuildRequires query parse the spec with the same fields. + parse = { + "dist": ctx.attrs.dist, + "in_place_rpmbuild_options": ctx.attrs.in_place_rpmbuild_options if source_tree != None else [], + # package_build() keeps the build directory of a dev build, because rpm_package passes keep_build_dir. + "incremental": ctx.attrs.configured_dev, + "release": ctx.attrs.release, + "rpmbuild_options": ctx.attrs.rpmbuild_options, + "source_date_epoch": ctx.attrs.source_date_epoch, + "sources": sources, + "spec_file": spec_file, + } + + 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), + parse, + source_tree, + stack, + ) + 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 [], - # package_build() keeps the build directory of a dev build, because rpm_package passes keep_build_dir. - "incremental": ctx.attrs.configured_dev, - "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 = parse | {"source_tree": source_tree, "strict_subpackages": pinned == None}, ) _rpm_package = rule( @@ -99,6 +139,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 querying the spec", + ), "in_place_rpmbuild_options": attrs.list( attrs.string(), default = [], @@ -119,6 +163,7 @@ _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)"), + "_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 0a5a0778a117ce1b4493df4479ba98e78c31a90a Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 2 Nov 2026 14:52:08 +0100 Subject: [PATCH 7/7] rpm: Query the BuildRequires of a hand-written package Only an in-place build queries its spec. A hand-written rpm.package that builds from an archive still 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 | 15 +++--- docs/user/rpm.md | 51 +++++++++++++++++++++ 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, 96 insertions(+), 9 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 0c17a65c..fb6680e6 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -716,11 +716,10 @@ 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 -bd` with the build's options in the base root and the - delta. It writes the BuildRequires that are missing, and a second delta installs them. That delta is - empty when the spec needs nothing more; + 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 -bd` with + the build's options in the base root and the delta. It writes the BuildRequires that are missing, and a + second delta installs them. That delta is empty when the spec needs nothing more; 3. overlays the base and the deltas, 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, @@ -1596,8 +1595,10 @@ future remote workers. Bazel's broader language-rule ecosystem mattered less tha for a package-heavy repository. Buck cannot add ordinary target dependencies discovered from an action output. Dynamic actions may select -among declared inputs but cannot turn newly discovered BuildRequires into a new static graph. Therefore -dependency discovery/import must produce committed or analysis-time lock data before normal builds. +among declared inputs but cannot turn newly discovered BuildRequires into a new static graph. An action can +still install BuildRequires that an earlier action discovers, as the BuildRequires query of a hand-written +`rpm.package()` does. A BuildRequires on a package that the project builds needs committed or analysis-time +lock data before normal builds. ### Commit repository snapshots and selectively freeze boxes diff --git a/docs/user/rpm.md b/docs/user/rpm.md new file mode 100644 index 00000000..a211fa19 --- /dev/null +++ b/docs/user/rpm.md @@ -0,0 +1,51 @@ + + +# 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 rpm needs to +parse the spec. Before the build, `rpmbuild -bd` checks the spec in that buildroot with the build's rpmbuild +options, so the spec's conditionals select the same BuildRequires as in the build. The build then installs +the BuildRequires that the buildroot lacks. + +`rpmbuild -bd` runs `%generate_buildrequires` only when the buildroot contains every static BuildRequires +of the spec, and runs it once. For a spec with the script, list its static BuildRequires in +`build_requires`. If the script reports more packages once its first ones are installed, list those as +well. Otherwise the build fails with rpm's `Failed build dependencies:`. + +Neither the check nor the build looks for the Source and Patch files of an in-place spec. The build only +fails if the spec reads a missing file, for example a Source that `%install` copies. + +A package of an imported branch skips the check. It installs the BuildRequires that the importer recorded +in `build_requires`. diff --git a/examples/package/rpm/BUCK b/examples/package/rpm/BUCK index 9c925de0..f133aa31 100644 --- a/examples/package/rpm/BUCK +++ b/examples/package/rpm/BUCK @@ -64,6 +64,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 13e6d263..eda6eb99 100644 --- a/package_system/rpm/rules.bzl +++ b/package_system/rpm/rules.bzl @@ -110,7 +110,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),