Introduce an rpm-controlled per-build directory

[pmatilai]

In our ancient wacko setup, %_builddir is shared by all your packages, under which a package may create %buildsubdir - if it uses %setup that is. In other words, there's no safe and sane place for rpm to place per-build material. Introduce a new intermediate directory between the two, created in a newly added (internal) %builddir build step, to give rpm that place. This opens up all manner of opportunities, to be explored in later commits.

A new build-time macro %pkgbuilddir is added for the absolute path of this new directory, but in addition we override %_builddir to the same value for maximum compatibility with existing specs - a LOT of packages reference %{_builddir} for all sorts of (mostly bad) reasons and we don't want to deal with the carnage that would follow from breaking it. A further complication here is that defining %_builddir (along with sources etc) to current working directory is a common configuration (used eg by fedpkg and its variants) that we don't want to break either. So upon entry, we grab the pre-existing %_builddir and define %_top_builddir from it, which we can then base the %pkgbuilddir on, while preserving compatibility with both of the above scenarios.

Fixes: #2078

[pmatilai]

FWIW, the %{NAME}-%{VERSION}-PKG name is not really intended as final, I used that just to have it stand out from the logs. Better ideas welcome.

[voxik]

Do I understand correctly that the BUILDROOT dir was replaced by %{_builddir}/%{_target_cpu}-%{_target_os}-root? The %{_builddir} is the right move IMHO, but what is the advantage of %{_target_cpu}-%{_target_os}-root over BUILDROOT, especially when e.g. SPECPARTS stays the same.

[pmatilai]

Do I understand correctly that the BUILDROOT dir was replaced by %{_builddir}/%{_target_cpu}-%{_target_os}-root? The %{_builddir} is the right move IMHO, but what is the advantage of %{_target_cpu}-%{_target_os}-root over BUILDROOT, especially when e.g. SPECPARTS stays the same.

These details are nowhere near fully thought through, and are certainly debatable and open/subject to change. In my initial patch buildroot was always BUILDROOT, but then experimenting with the vpath build stuff and your own mention about potentially needing multiple build directories made me think perhaps it should match the build directory name, and those are commonly named by the target tuple.

All that is of course irrelevant within a single rpmbuild run, but down that road far in the horizon one could imagine unpacking the source once, and then executing just the build steps from other hosts using a read-only shared mount of the source. Or (cross-) building on the same host for multiple targets at once. Of course in such a scenario, SPECPARTS would have to be similarly per-arch-os too, it's a valid point.

Wild visions aside, think of the (buildroot) rename more like shaking a tree a bit to see what falls out rather than something decided.

[pmatilai]

@kloczek , this is not about SPECPARTS although if you look at the PR, that's one of the issues that gets solved by this. See #2532 for the background.

[kloczek]

@kloczek , this is not about SPECPARTS although if you look at the PR, that's one of the issues that gets solved by this. See #2532 for the background.

On first look it looks like it is setuptools issue. .. like in other python modules cases (which I've mention here).

[voxik]

These details are nowhere near fully thought through, and are certainly debatable and open/subject to change. In my initial patch buildroot was always BUILDROOT, but then experimenting with the vpath build stuff and your own mention about potentially needing multiple build directories made me think perhaps it should match the build directory name, and those are commonly named by the target tuple.

While I think multiple directories for vpath builds are useful, if BUILDROOT is still the image of what will be installed on the resulting system, then my current thinking is that there should be only one destination, where everything should be parallel installable if possible. But maybe you had the curl example with RemovePathPostfixes on your mind which could be legitimate reason.

[pmatilai]

Multiple buildroots could be useful for the the "variant builds" case, but the arch-os naming doesn't help with that at all, the potential benefits I see are more far fetched. But, it's useful to shake things a bit just to see if something more meaningful falls out, it doesn't have to be BUILDROOT just because we've had such a thing in the past.

[dmnks]

Not a fan of the -PKG and -root suffixes. Now that we have the option, why not take full advantage of directories and have a tree like this:

BUILD/
├── bar-1.0
│   ├── ROOT
│   │   └── x86_64-linux
│   │   └── [...]
│   └── SPECPARTS
└── foo-1.0
    ├── ROOT
    │   └── x86_64-linux
    │   └── [...]
    └── SPECPARTS

While I like the singular ROOT better, maybe ROOTS would be more consistent.

[pmatilai]

Not a fan of the -PKG and -root suffixes.

The -PKG is ugly for sure, but some differentation from the traditional %setup created directory seems necessary to drive the point across, and make logs easier to look at. If you just have 'foo-1.0/foo-1.0/' its easy to mix up etc. NEVRA might be more reasonable, and for --rebuild mode it could actually be randomized as a packaging sanity check.

Now that we have the option, why not take full advantage of directories and have a tree like this:
[...]
While I like the singular ROOT better, maybe ROOTS would be more consistent.

For now we only have one buildroot though. I guess I'll just go back to the original BUILDROOT inside the new build directory, at least people will know what that thing is 😅

[voxik]

Not a fan of the -PKG and -root suffixes.

If you just have 'foo-1.0/foo-1.0/' its easy to mix up etc.

I don't mind this path. But if the path actually was 'foo-1.0/SOURCE/' that could mitigate your concern. And yes, there is #2859 always expanding the tarball into name-version directory, so if the expansion was instead always into SRC that would not be so bad IMHO.

But arguably, quite some projects place their real sources into something like src directory, so then there could be patch like 'foo-1.0/SOURCE/src' 🙈

[pmatilai]

The thing is (well, one of the things) is that, foo-1.0/foo-1.0 can mask a bug or packaging error. The ugly -PKG suffix was absolutely necessary for seeing which path a thing is actually trying to use when developing this PR, and will be equally necessary for packagers trying to troubleshoot something more exotic.

So it doesn't need to be -PKG but it needs to be something that is easily distinguishable from something the %{name}-%{version} thing %setup normally does.

[pmatilai]

Merged this into a single commit at least for now as the changing paths clashed in maddening ways in the test-suite on rebases, making simple changes too hard to bother with.

Buildroot is just BUILDROOT now. I would've used name-version-release.arch for the per-build directory, but this turns out to break a buildid no-recompute test which assumes that a buildid remains the same regardless of the release. So it's name-version-arch now, for the better or worse. name-version.arch seemed even weirder somehow.

I'm wondering if these should still be macro overridable. I just recently demanded @ffesti add macros for the SPECPARTS stuff, but here I'm stripping them off, left and right.

[pmatilai]

Other ponderings include the per-build directory macro name, should it be just %builddir without the underscore (instead of %pkgbuilddir)

[dmnks]

One thing to keep in mind here is that we'll be getting rid of a shared BUILDROOT. I've always wondered what the purpose of that (or the shared %_topdir workspace in general) was, but I can think of one use case:

You wish to deploy a common set of packages and/or configuration (a suite) to a number of hosts in your organization. You want that suite to be managed and updated "atomically" as a whole, and ideally distributed as a single binary RPM without additional dependencies (basically an equivalent of a statically compiled binary). This is where the shared RPM workspace comes into play! You simply build those individual packages locally one by one, and finally build the "meta" package that just packages the resulting BUILDROOT. This would of course require some light scripting with RPM and such but not terribly difficult.

Basically, this is what container images are in the modern world. I don't think the above use case is very common (or real at all) but I can imagine that being one of the motivations behind the shared workspace back then.

[dmnks]

Which makes me think (and this already is a bit off-topic) - couldn't the shared BUILDROOT be useful for actually building container images? I'm not sure about the advantages over just grabbing pre-built RPMs to compose the final root filesystem tree, but it does seem like you'd save a number of redundant steps if you were building those packages yourself.

Edit: OK, one show stopper would probably be runtime scriptlets - there's no way around those, at least not when using "off-the-shelf" packages that use them quite a lot.

[dmnks]

To summarize my above comments a bit, from a higher-level perspective:

Currently, it's possible to combine multiple BUILDs, SOURCES or SPECs into a single resulting package. This PR will remove that flexibility. That's probably OK.

[dmnks]

Thinking more about it, the shared BUILDROOT use case might actually be impossible to achieve because of the fact that RPM checks for unpackaged files in there when building a single package (see the recent discussions around excludes).

You'd also have to list all the %files in BUILDROOT in the "meta" package which would make it a huge hassle...

[pmatilai]

Eliminating any ideas of shared build/buildroot content and associated shenanigans (and consequent problems) is one of the goals of this PR. Utopistic-ideally, a build would not have access to any content outside this dedicated build directory.

[pmatilai]

Yet another use-case: we can override build-time %_tmpdir to something inside this build area, at which point a build is pretty thoroughly contained within this one directory where it needs write-permissions.

[pmatilai]

Eh, except that we create our scriptlets, including the one that creates the builddir, in %_tmppath so we there's a tiny 🥚 - 🐔 matter there. Oh well, maybe later.

[pmatilai]

After (way too) much mulling over it: BUILDROOT and SPECPARTS names are now hardcoded in the source, neither of these should be user configurable really but were so for various historical reasons, such as not having the guaranteed per-build directory we're adding in this very PR. Removed the corresponding macros from the main macros file too.

With SPECPARTS now always in the per-build directory, we could probably drop the "rm -rf '%{specpartsdir}'" from %setup handling but it's not harmful so we can ponder about that later.

From my POV this is good to go in at this point, unless we want to bikeshed paths or names more 😅

[dmnks]

Speaking of name bikeshedding,

Other ponderings include the per-build directory macro name, should it be just %builddir without the underscore (instead of %pkgbuilddir)

This has gotten lost in the above chatter, but I think it's worth considering. It's a macro name that we'll be living with from now on and somehow %builddir feels more natural than %pkgbuilddir. To me, that is.

OTOH, the "pkg" prefix perhaps emphasizes that it's a per-package build directory as opposed to some common one (which is %_top_builddir now with the patch). So dunno. Maybe I'm overthinking it...

Update: Just realized that this is a build-time macro. We already have %buildroot there so using %builddir seems more consistent in that regard (?)

Update 2: Is there an actual need for a SPEC to ever use the %pkgbuilddir (or %builddir for that matter) macro? If not, why not make it at least "private" with an underscore?

Update 3: Oh... you actually mentioned that "a LOT of packages reference %_builddir for all sorts of (mostly bad) reasons" in the commit message. In that case, why introduce yet another such macro at all?

[dmnks]

As for the paths, I think they're fine now, no need to mull over those more, indeed 😄

[dmnks]

Lastly, the docs/manual/dynamic_specs.md files needs updating as it mentions the old SPECPARTS path.

[pmatilai]

Update 2: Is there an actual need for a SPEC to ever use the %pkgbuilddir (or %builddir for that matter) macro? If not, why not make it at least "private" with an underscore?

Oh, I actually mulled about %builddir without the underscore as the user-oriented name for this. I too kinda liked that more, I think I ended up with the pkg-version to make minimize confusion with %_builddir. OTOH, like you said we already have %buildroot and %builddir would seem more consistent with that. I could quite easily be convinced either way 😄

Update 3: Oh... you actually mentioned that "a LOT of packages reference %_builddir for all sorts of (mostly bad) reasons" in the commit message. In that case, why introduce yet another such macro at all?

Primarily because %_builddir is ambiguous, you don't really know whether it's the bad old or good new. It still points to the potentially shared %{_topdir}/BUILD by default, whereas %pkgbuilddir (or %builddir) is only defined during the actual build (which is also the only time that directory exists).

Note that the "mostly bad" stops being bad with this change - this is now the legit place for packages to use for their needs during the build.

[pmatilai]

Good point on the docs, that dynamic stuff certainly needs updating, but there could be other things too. Like, explaining the new layout somehow. Not that packagers need to know but it doesn't exactly hurt either.

It's possible this could use a test or two as well...

[pmatilai]

Ended up renaming %pkgbuilddir to %builddir as per your and my gut feelings - we don't have "pkg" anywhere at all in our macros so introducing it here would seem a bit strange somehow.
Updated the docs a bit + added couple of tests to demonstrate the new layout and behavior.

As a side-note, SPECPARTS behaves a little funny here because it's only created from %setup (because there was no other place to do it before), but could be moved to %builddir script now. I'd rather not do so in this PR to avoid scope-creep, but we can do that later.

The above paragraph also made me notice there's an ambiguity in the naming now: the script and the directory that the script creates are both called %builddir. Maybe the script needs to be called %mkbuilddir instead.

[pmatilai]

Renamed the builddir creation script to %mkbuilddir (along with the enum) in the last push to disambiguate the two.

[pmatilai]

So... I would think that this is ready now, for real 😅

[dmnks]

Seems like the test-suite changes have accidentally spilled into the preceding commit Print informative messages to stderr, not stdout on --target build 😄

[pmatilai]

Thirteenth time the charm, eh? 😄
Should be fixed up now, and thanks guys for spotting this stuff.

[Conan-Kudo]

Can we please not include architecture? That can leak into builds and cause problems. That's why I had to make an adjustment in redhat-rpm-config for %_vpath_builddir: https://src.fedoraproject.org/rpms/redhat-rpm-config/c/e0cfcc0fc76a7642faabb25c5e348d6a1314ace2

[pmatilai]

I'm not sure that's the same thing, this is one level below it. It gets a bit hysterical if we can't name our directories the way we want just because Doxygen.

I admit this is tricky stuff, like debuginfo expectations breaking if NVRA is used there. But that's at least actually related to rpm somehow.

[pmatilai]

Oh and FWIW, I would love to get the full build paths somehow abstracted out anyhow. I don't know how much is really doable in that space, but on systems that support this kind of stuff, builds would run temp directory inside their build directory and writes limited to their own build directory, bind-mounted so that the build thinks its running in /build directory instead of /home/whatever/somewhere path etc.