Repository navigation
rpm: Install BuildRequires from the spec - #397
Conversation
728907f to
2d19e78
Compare
daandemeyer
changed the title
2d19e78 to
70f2928
Compare
|
For the first round, bot-to-bot review: pr-in-place-buildrequires-review.md Sorry, in a long meeting, not much human review on that report -- can do in ~ 1 h. |
70f2928 to
bbe21b6
Compare
martinpitt
left a comment
martinpitt
left a comment
There was a problem hiding this comment.
TBH, my gut doesn't like this. It's a lot of brittle complexity that doesn't fit into a human brain any more, all for the relatively corner use case of building a local image with a temporarily mounted systemd tree.
This does not and cannot replace the rpm imports with its srcpkg.json, as regardless of how expensive/powerful we make this machinery, it cannot feed back into the build graph (buck just doesn't work that way, which is why we introduced the "record package metadata in json files" concept in the first place). So that code path has to remain.
AFAIUI, our primary use case/motivation is to build against a local systemd, right? That doesn't use %generate_buildrequires, but supporting that is responsible for a lot of complexity here, mostlythe dynamic action loop from bbe21b6 and the PackageInstaller split from 734bb08. With no generators, the query could also become rpmspec -q --buildrequires that runs in the buildroot.
Claude estimates that about 400 lines. Could we drop that and refuse local packages with %generate_buildrequires? Feels too much of a YAGNI case to me. (As an old Debianite, I openly admit that I hate that concept: it makes reasoning about build graphs unpredictable and hard, and building packages a PITA).
This isn't a veto, just some venting -- you stared at the requirements for this for a long time, and I trust you to know whether it's worth it.
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 <daan@amutable.com>
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 <daan@amutable.com>
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 <daan@amutable.com>
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 <daan@amutable.com>
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 <daan@amutable.com>
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 <daan@amutable.com>
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 <daan@amutable.com>
bbe21b6 to
0a5a077
Compare
|
@martinpitt I dropped the dynamic buildrequires stuff again. Note that this wasn't just for local builds of systemd. I think any project using tine should be able to build one or two rpms locally without having to pull in the entire importer without having to list all buildrequires one by one. I dislike the concept as well but unfortunately it's a thing rpm supports :/. Anyway, dropped for now, can bring this up again when I actually have a use case for it. |
martinpitt
left a comment
martinpitt
left a comment
There was a problem hiding this comment.
ack, thanks! I'll close my eyes on the other minor issues, let's go!
A hand-written rpm.package, or an in-place build, only gets the BuildRequires that its BUCK file lists.
systemd lists 66 of them by hand, and the list goes stale whenever the pin moves.
Let's run rpmbuild -bd on the spec in the buildroot with the same options as the build, and install the
BuildRequires that it reports missing as an extra layer. rpm runs %generate_buildrequires only when the
static BuildRequires are installed, so a spec with the script lists those in build_requires. Imported
packages keep using their recorded BuildRequires, since rpm.branch() derives their buildroot_packages from
them.