Repository navigation
fix(rpm): support rpmbuild 4.20+ per-package build directory - #1084
Conversation
037cdb9 to
f28b2d1
Compare
aiuto
left a comment
There was a problem hiding this comment.
Did you reproduce using the docker sample from the bug?
@aiuto Yes:
and succeeds with this PR. |
RPM 4.20 introduced a per-package build directory and redefined
`%{_builddir}` accordingly (rpm-software-management/rpm#2885).
As a result, the paths generated by `rules_pkg` for `%install`, which
were relative to the previous working directory, no longer resolve:
```text
cp: cannot stat 'tests/testdata/a.ar': No such file or directory
```
This change makes the generated paths independent of `rpmbuild`'s
working directory:
- `%install` copies inputs from `%{_topdir}/BUILD`, where `make_rpm.py`
stages them,
- `%files -f` receives an absolute path,
- the same applies to regular files and tree artifact inputs.
This also removes the debuginfo-specific `../` path variant introduced
in bazelbuild#1006, since the same paths now work across RPM versions.
The legacy RPM test spec relied on the working directory as well and
gets the same anchor.
RPM 4.20 also makes `buildsubdir .` usable again, while the `BUILD_SUB`
path used for RPM 4.18 and 4.19 is no longer reachable from the new
per-package build directory.
Debuginfo autodetection therefore selects:
- "centos" for RPM versions before 4.18 and from 4.20 onward,
- "fedora" for RPM 4.18 and 4.19.
Validated with `//tests/rpm/...`, `//toolchains/...`, and
`//pkg/legacy/tests/rpm/...` against RPM 4.18.2 and 6.0.1, CI covering
RPM 4.17.
Fixes bazelbuild#938.
f28b2d1 to
d4999c7
Compare
### What does this PR do? Bump `rules_pkg` to 1.3.0 and repin its `archive_override` to upstream main at bazelbuild/rules_pkg#1076, which upstreams the local Windows junction patch. Apply the pending bazelbuild/rules_pkg#1084 on top as the only patch. ### Motivation RPM 4.20 redefined `%{_builddir}` as a per-package build directory (rpm-software-management/rpm#2885), which breaks the relative paths `rules_pkg` generates for `%install` and `%files`. With RPM 6.0.1, as shipped by Ubuntu 26.04 LTS, `bazel build //packages/dogstatsd:rpm` fails with `Could not open %files file` and `//packages/installer/linux:rpm` with `cp: cannot stat ...README_rpm.md`. CI images ship RPM 4.18.2 and older, so they are not affected yet, but running them against the patch proves it keeps working with older RPM versions, which upstream cannot test. ### Describe how you validated your changes Both targets above build with RPM 6.0.1, whereas they fail without this change once `MakeRpm` actions bypass caches (`--modify_execution_info=MakeRpm=+no-cache`). ### Additional Notes `rpmbuild` is a system toolchain absent from action keys, so cached RPMs mask both the breakage and the fix.
### What does this PR do? Bump `rules_pkg` to 1.3.0 and repin its `archive_override` to upstream main at bazelbuild/rules_pkg#1076, which upstreams the local Windows junction patch. Apply the pending bazelbuild/rules_pkg#1084 on top as the only patch. ### Motivation RPM 4.20 redefined `%{_builddir}` as a per-package build directory (rpm-software-management/rpm#2885), which breaks the relative paths `rules_pkg` generates for `%install` and `%files`. With RPM 6.0.1, as shipped by Ubuntu 26.04 LTS, `bazel build //packages/dogstatsd:rpm` fails with `Could not open %files file` and `//packages/installer/linux:rpm` with `cp: cannot stat ...README_rpm.md`. CI images ship RPM 4.18.2 and older, so they are not affected yet, but running them against the patch proves it keeps working with older RPM versions, which upstream cannot test. ### Describe how you validated your changes Both targets above build with RPM 6.0.1, whereas they fail without this change once `MakeRpm` actions bypass caches (`--modify_execution_info=MakeRpm=+no-cache`). ### Additional Notes The `rpmbuild` version only reaches `MakeRpm` action keys through `--debuginfo_type`, which no target enables, so cached RPMs mask both the breakage and the fix.
…57825) ### What does this PR do? Bump `rules_pkg` to 1.3.0 and repin its `archive_override` to upstream main at bazelbuild/rules_pkg#1076, which upstreams the local Windows junction patch. Conversely, apply the pending bazelbuild/rules_pkg#1084 on top as the only patch. ### Motivation RPM 4.20 redefined `%{_builddir}` as a per-package build directory (rpm-software-management/rpm#2885), which breaks the relative paths `rules_pkg` generates for `%install` and `%files`. With RPM 6.0.1, as shipped by Ubuntu 26.04 LTS (author's laptop), `bazel build //packages/dogstatsd:rpm` fails with `Could not open %files file` and `//packages/installer/linux:rpm` with `cp: cannot stat ...README_rpm.md`. CI images ship RPM 4.18.2 and older, so they are not affected yet, but running them against the patch proves it keeps working with older RPM versions, which upstream doesn't necessarily cover. ### Describe how you validated your changes Both targets above build with RPM 6.0.1, whereas they fail without this change once `MakeRpm` actions bypass caches (`--modify_execution_info=MakeRpm=+no-cache`). ### Additional Notes The `rpmbuild` version only reaches `MakeRpm` action keys through `--debuginfo_type`, which no target enables, so cached RPMs might mask both the breakage and the fix when switching the base build image to Ubuntu 26.04. This dormant issue predates bazelbuild/rules_pkg#1084, and therefore deserves a separate upstream contribution. Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>
|
@kellyma2 FYI |
aiuto
left a comment
There was a problem hiding this comment.
I guess?
We really should strive to keep old behavior even if it is known broken because that preserves the best experience of bug for bug backwards compatibilty. We can never know what are users are doing to work around previous behavior.
OTOH:
- it is mostly changing for deprecated rpm variations which people are less likely to build on.
- And the next release will be a 2.x because of other breaking changes, so friction on corner cases is allowed.
- And if someone complains, they can fork their own until we come up with bug-compatible back fix.
| if major < 4 or (major == 4 and minor < 18): | ||
| debuginfo_type = DEBUGINFO_TYPE_CENTOS | ||
| else: | ||
| if major == 4 and minor in (18, 19): |
There was a problem hiding this comment.
This is the kind of crap which really makes me despise rpmbuild.
Just to confirm, from a careful reading of the release notes, you are saying that they changed the format ONLY for 4.18 and 4.19, then reverted back.
It might be safer to leave the condition as is to preserve backward compatibility with what people expect.
It looks like either way, at version 6 it will be CENTOS format
There was a problem hiding this comment.
Not quite a revert: RPM 4.20 redefined %{_builddir} as a per-package directory created by rpmbuild itself (release notes).
The "fedora" type sets buildsubdir BUILD_SUB, which make_rpm.py pre-creates under the former BUILD/, so rpmbuild now looks for it in the wrong place.
buildsubdir . instead resolves to the per-package directory, which exists, so the "centos" settings work again from 4.20 on.
On backward compatibility, autodetection only changes for RPM 4.20+:
- below 4.18: "centos" before and after => unchanged here,
- 4.18 and 4.19: "fedora" before and after => unchanged here,
- 4.20+: "fedora" before, "centos" now => fixed here.
With the current condition, 4.20+ gets "fedora" (i.e. major < 4 or (major == 4 and minor < 18) evaluates to False), and on 4.20+ every MakeRpm fails anyway (that's #938).
So no working setup changes behavior, and an explicit debuginfo_type is unaffected.
There was a problem hiding this comment.
TBH, I think our saving grace is that the people using old rpm styles are not updating rules_pkg, so we'll get lucky and miss any potential problems.
RPM 4.20 introduced a per-package build directory and redefined
%{_builddir}accordingly (rpm-software-management/rpm#2885).As a result, the paths generated by
rules_pkgfor%install, which were relative to the previous working directory, no longer resolve:This change makes the generated paths independent of
rpmbuild's working directory:%installcopies inputs from%{_topdir}/BUILD, wheremake_rpm.pystages them,%files -freceives an absolute path,This also removes the debuginfo-specific
../path variant introduced in #1006, since the same paths now work across RPM versions.The RPM test spec relied on the working directory as well and gets the same anchor.
RPM 4.20 also makes
buildsubdir .usable again, while theBUILD_SUBpath used for RPM 4.18 and 4.19 is no longer reachable from the new per-package build directory.Debuginfo autodetection therefore selects:
Validated with
//tests/rpm/...,//toolchains/..., and//pkg/legacy/tests/rpm/...against RPM 4.18.2 and 6.0.1, CI covering RPM 4.17.Fixes #938.