On 06. 04. 21 11:16, Panu Matilainen wrote:
On 4/1/21 12:45 AM, Miro Hrončok wrote:
On 31. 03. 21 21:52, Ben Cotton wrote:
* Strict checking for unpackaged content in builds
> ...
* Many existing packages will fail to build due to the stricter
buildroot content checking. Fixing this in the packaging is always
backwards compatible. We could temporarily set
`%_unpackaged_files_terminate_build 0` in rawhide to alleviate initial
impact if necessary.
This is my main concern with this update.
tl;dr If you %exclude something and there is no other subpackage to own the
files, the build fails:
This fails:
%install
...
touch %{buildroot}/foo %{buildroot}/bar
%files
/
%exclude /foo
This still succeeds:
%files
/
%exclude /foo
%files foo
/foo
Many packages do the former in Fedora for various different reasons, namely
to, well... exclude files from the package (and not ship them at all).
Sometimes a `rm` in %install can be used instead. Sometimes not, because the
files are needed in the %{buildroot} for %check but not needed to be shipped.
The bottom line is that the buildroot should not contain anything that is not
packaged. This has been the basic premise ever since the check for unpackaged
files was added somewhere around turn of the millenium, but some loopholes were
left around. Yes, %exclude is a loophole.
So 'rm -f' at end of %install for undesired material is the expected fix.
What %check may or may not do has probably never been designed, much less
documented or enforced. It runs after %install yes, so one might assume it's
ok/supposed to use what's in buildroot, but then it runs from the build
directory, not buildroot.
It doesn't matter where does it run *from*, it needs to "see" the files from
%{buildroot} because that is what we want to test: The files we ship.
The way I see it, %check should be able to use/access buildroot contents, but it
certainly should not write there. That might be something to even enforce one day.
The packages (that I know about) don't need to actually write there in %check.
They just need to to see the files that will be later excluded (e.g. because
they belong to a different package that the soon-to-be-built package requires on
runtime).
When this change was introduced upstream in November 2020, I've analyzed the
impact on Fedora packages. Bare in mind that the data is 4+ months old.
https://github.com/rpm-software-management/rpm/pull/1442#issuecomment-731554917
- 1675 packages had %exclude in the spec file
- 261 packages FTBFS for unrelated reason (incl. a very limited timeout)
- 1414 packages actually tested
- 537 packages built successfully, that is ~38%
- 877 packages failed with unpackaged files, that is ~62%, list attached
When I extrapolate the numbers to compensate the unrelated FTBFS, that's
likely more than 1000 affected Fedora packages.
OTOH ~500 packages are generated rubygem-* packages, automatically fixable.
I'd like to know how are the affected packages supposed to migrate to RPM 4.17
behavior, especially if they cannot remove the files in %install prior to
%check. Are they supposed to remove the files at the end of %check instead?
What if the package is build without %check?
In the order of preference:
1. 'rm -f <unwanted>' at end of %install
That one is simple. I agree. If it is possible, let's do this. We could even
automate it if needed. (As a side note I'd say writing such automation is a
right thing to do when a change like this is proposed, but I understand that we
cannot have everything.)
2. change %check not to rely on unpackaged files in buildroot
That one is non-trivial and depends on the reason it is needed.
For example, what is common for Python "namepsace" packages, e.g. pkg_name.foo.
1) We want to test installed files, not what is in $PWD, so we set PYTHONPATH to
%{buildroot}%{python3_sitearch}:%{buildroot}%{python3_sitelib} and we
(try hard to) exclude $PWD from it. This is crucial to ensure the files
we actually ship are working and the installed file set is complete.
Our macros do this for the packagers.
2) The %{python3_site...}/pkg_name/ directory and
%{python3_site...}/pkg_name/__init__.py and
%{python3_site...}/pkg_name/__pycache__/ and
%{python3_site...}/pkg_name/__pycache__/__init__...pyc
files must be present in %{buildroot} to successfully run the tests.
3) The files from (2) must be excluded from the package because *pkg_name* owns
them, not *pkg_name.foo*.
We Require the "toplevel" *pkg_name* package from *pkg_name.foo*.
The files are not bit-by-bit+metadata identical,
so both packages cannot ship them.
Or, let's assume I want to package libfoo-devel for EPEL 9 because RHEL 9 only
ships libfoo. And I want to run %check, but only ship the headers. There are
plenty situations like this.
The case-by-case fix is also impossible to do at scale without a huge heroic
effort.
3. short-term, "%_unpackaged_files_terminate_build 0" in the spec with
explanation (similar to eg unsupported architectures)
I will address below why I don't like this (and why I consider it dangerous).
2. would be case-by-case of course, but an "universal" solution is to create a
private install root inside %check, eg
---
%check
export CHECKROOT=${PWD}/_check
%make_install DESTDIR=${CHECKROOT}
# ...do what you normally do, just replace BUILDROOT with CHECKROOT
---
This way %check will not be messing with the precious to-be-packaged contents.
Yes, this works. However it requires a tedious copy-paste boilerplate :/
E.g. instead of this:
%install
%py3_install
%check
%pytest
%files
%exclude %{python3_sitelib}/...unwanted...
...
I need to do this:
%install
%py3_install
mkdir _check
cp -a %{buildroot} _check
rm -rf %{buildroot}%{python3_sitelib}/...unwanted...
%check
export PYTHONPATH=${PWD}/_check%{python3_sitelib}
%pytest
%files
...
Also suffers from "very hard to do this change at scale" problem.
More light-weight options will exist on case-by-case basis, eg typically a cache
can be diverted to some other location with environment variables or such.
Using `s` in entire rawhide just to compensate this:
- is dangerous for other implications of the setting
- only postpones the problem to a later time (when we will face the same
issue)
It's hardly "dangerous", because the only content that will "leak" because of it
is buildid links, which is what happens now. It doesn't affect content
inclusion, just whether the message is a warning or error.
Nothing will "leak", quite the opposite: files will be missing.
Setting %_unpackaged_files_terminate_build to 0 globally is dangerous, because
packagers will make mistakes and they will forget to include newly added files,
e.g. on updates. Such packages will happily build but will miss needed files,
only to fail in unpredictable situations on runtime.
Consider this spec:
Name: hammer
Version: 1.1
...
%global _unpackaged_files_terminate_build 0
%install
%make_install
%check
%make test
%files
%{_bindir}/hammer
%exclude %{_bindir}/test-hammer
It works and builds fine.
Now the maintainer updates to version 1.2, where %make_install also installs
/usr/share/hammer with some data files. When the package builds, there is a
warning (yet the packagers don't usually read the Koji build logs when the build
is successful) but /usr/share/hammer is missing from the package.
%check has passed, so the maintainer thinks everything works. However in
reality, hammer crashes on runtime. Even if the maintainer does their due
diligence to test the package before they ship it, they might not notice the
problem because hammer crashes only when the user tries to do a very specific
action.
And let's face it: Many packagers will just bump the version in the spec file
and "if it builds, ship it".
Setting %_unpackaged_files_terminate_build to 0 in the affected packages only is
only less dangerous because of the limited impact, but can cause the very same
problem.
OTOH if we had a %_unpackaged_excluded_files_terminate_build 0 setting, I would
argue that adding that to all affected packages until they are fixed is a good
backwards compatibility measure. Adding %_unpackaged_files_terminate_build 0 is not.
And for a more specific problem, around ~100 Python packages were affected
when tested, many of them crucial (e.g. dnf), so this problem will block the
upgrade to Python 3.10 if the change lands in Rawhide before we upgrade Python
(which is the current plan) until we fix all the affected packages (by at
least adding `%global _unpackaged_files_terminate_build 0` to them, which is a
tad big hammer, but it will be our last-resort option).
Which is why suggested the global "%_unpackaged_files_terminate_build 0" in
rawhide: just to move the impact to a less inconvenient time. No kittens will be
killed by doing so.
We will ship broken packages in the transition period. Kittens may die if we
break the packages that take care of kitten-life-support systems.
One could argue that kitten-lives-critical systems should not run Rawhide,
however once we end the transition period, the broken packages will start to
fail to build. And until we fix them, we will ship the broken versions. Hence it
is likely we will ship such packages with missing files unnoticed to the stable
release as well.
--
Miro Hrončok
--
Phone: +420777974800
IRC: mhroncok
_______________________________________________
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
