When we notice a pre-existing dependency issue, it seems strange
to exit non-zero, since the update being tested didn't break
anything new. Let's just treat it more like a warning. Let's exit
0 in this case as there's no 'warning' exit code; the pipelines
can use the JSON output to report an INFO or WARN result.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
Packaging two Python scripts, one of which is a wrapper for the
other, is pretty awkward. But more importantly, we can make the
EL "wrapper" work better with a refactor, too. It's now a stand
alone script called `rdc-compose`. The original script is renamed
to `rdc-repos`.
The CLI scripts now share a lot of bits via `shared.py`. We put
handy run-it-locally wrappers for each CLI script at the top
level; you *can* install the package properly and you'll get
`rdc-repos` and `rdc-compose` executables but we'll likely rarely
do that.
`rdc-compose` will now run faster because it will reuse the same
metadata between variants (previously it got re-downloaded with
every execution of `rmdepcheck.py`) and it can now nicely collect
up all the repoclosure strings before using the same parse-and-
exit code as rmdepcheck, which solves a lot of awkward issues
around output.
We also add tests covering `rdc-compose`, as part of which it made
sense to factor out the Pungi config URL discovery (which I meant
to do anyway). We fix a bug that was exposed by writing the tests:
we have to check variants that depend on variants we have packages
for, even if we don't have packages for those variants.
This isn't great git hygiene, sorry. Should have made the refactor,
the addition of tests and the bug fix into separate commits. But
it's awkward and time-consuming to unpick now and I don't think
it's a good use of time.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
After much discussion with @yselkowitz, this should be the best
option for ELN testing, I hope.
ELN (like other ELs) has multiple variants with repositories.
There is a mapping in Pungi config of relationships between the
variants. Each variant is expected to be repoclosure-complete,
possibly in itself, possibly with reference to some or all of
the other variants. Each variant contains only a subset of the
packages in the buildroot.
We have been asked to make it so that, given an arbitrary set of
"packages under test" for a given EL, the packages it contains
from each variant should be tested separately against that
variant, so we fail if replacing the packages in the variant with
the matching packages from the set under test would cause new
repoclosure issues in that variant, or the packages under test
for that variant have installability issues when tested against
the variant and any others it "depends" on.
Packages from the set under test that are not in any variant
should be ignored, as repoclosure of and installability within
the buildroot is not currently desired to be tested.
This wrapper script takes as inputs a compose that we should test
against, and a filesystem path containing the packages to be
tested. The compose is usually expected to be the latest compose
of the EL under test. Using the metadata, we discover all variants
in the compose that have repositories (except Buildroot, which is
ignored). We then list out the binary packages by name in each
variant repository.
We then split the set of packages-under-test into several temporary
variant repositories: each package is placed in the repo for all
variants of which the identically-named package in the compose is a
part, if any.
We then get and parse a copy of the variant mapping from the
Pungi config. Then, for each "populated" variant (that is, a
variant for which the packages-under-test set contains at least
one package), we run rmdepcheck, with the compose repos for the
variant plus any variants it "depends" on as the baserepos, the
local temporary variant repository as the newrepo, and the local
temp repos for all "depended-on" variants as addrepos. All
rmdepcheck output is passed through.
We track the return codes of each rmdepcheck run and return the
sum of each unique return code.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
This adds a 'non-checked base repo' concept as part of addressing
the ELN scenario covered in #37. A 'non-checked base repo' is just
like a base repo, but it is not included in the `--check` arg when
generating the modified repoclosure. So it's always available to
the depsolver, and it is modified just like a regular base repo
(unlike a 'non-modified base repo'), but we don't actually check
for repoclosure issues in it.
This is intended to be used in EL scenarios where there are many
interdependent variant repositories. We may be testing a set of
packages that are each destined for a different set of the variant
repositories. We want to test each variant repository separately.
Some repositories 'depend' on others - e.g. packages in Extras can
depend on packages in BaseOS, Appstream and CRB. So when testing
the repository of packages destined for the Extras variant, we
should keep the BaseOS, Appstream and CRB variant repos "in scope"
for both the "base repositories" set and the "new package
repositories" set.
For the "new package repositories" set we can use `--addrepos`
to include the other variant repos but not check them (when doing
the installability phase). But we need the equivalent on the
"base repositories" side: we need to be able to specify repos
that will be modified, but not checked during the repoclosure
phase. This adds that feature.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
Looks like we can get empty lines in the `dnf repoclosure` output
sometimes (not sure why). They break things, so we'll skip them.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
The only way to actually reach this safety check is to force the
dnf repoclosure output to include a line that is exactly the name
of one of the excluded source packages, with no other text. This
is pretty unlikely to ever happen, but oh well, it gets rid of a
pragma.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
This is obviously the 'right' way to do this but I did it the
other way to try and make both mypy and pylint happy. This should
be a better way to make them both happy, though.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
Instead of painfully retrieving, decompressing, parsing, editing
and reconstructing the repodata, let's use dnf's excludepkgs
option to achieve the same thing. This is much simpler, faster,
and more robust.
To 'modify' the modified base repos, we use dnf repoquery to
list the packages they contain and their source RPM names, then
construct a table with the NEVRs to be removed for each repo.
Then when we do the repoclosure command, we pass
--setopt <repo>.excludepkgs=<nevrlist> for each modified repo.
This tells dnf to act as if the specified NEVRs simply do not
exist in the specified repo, which is exactly what we want to
achieve.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
The main goal here is to also download and modify filelists and
modules metadata to fix some problems I saw in EPEL 9 update
tests. We need the filelists metadata to get correct results if
a package has a dependency on a file that is not present in the
primary metadata; if we don't also download filelists, we'll get
an incorrect "new" broken dependency because repoclosure on the
modified repository will not be able to find the package that
contains the file.
Similarly, we need modules metadata (if present) to ensure that
dnf knows module packages are module packages. Without the modules
metadata it treats all module packages as non-module packages,
so repoclosure on the modified repo might incorrectly say a dep
of a non-module package is fixed because it can be satisfied by a
module package.
However, there was a big trap lurking here: the filelists metadata
is even larger than the primary metadata, and both are kinda huge.
Previously we were using ElementTree.parse(), which loads the
entire XML tree into memory; this was already using >6GB of RAM
for a typical primary metadata file. If you tried to load both
primary and filelists into RAM at once (as my initial attempt did)
it uses a huge amount of RAM and probably gets OOM killed.
So, we'll parse the XML as a chunked bytestring. As we go along,
we split out package elements, one by one. Everything that is
not part of a package element gets passed straight through to the
output file.
When we encounter a package element, we parse it with lxml (which
gives us a nice ~2x speedup over ElementTree). We decide whether
to drop it. We do this the same way as before for the primary
metadata, but record the pkgid (which is usually the checksum).
When parsing the filelists metadata, we take the list of pkgids
removed from the primary metadata as input, and remove all
package elements with the same pkgid.
If we decide to drop the package, we move to the end of it in
the current input chunk. Otherwise, we write the chunk through
to the output file, then move ahead and continue.
We use the `open()` methods of various compression libraries to
achieve transparent decompression and recompression, and track
uncompressed checksums and sizes along the way. This also adds
support for various other compression formats; previously we
assumed zstd. Supporting at least gzip is important as there are
still extant RHEL releases with only gzip-compressed metadata.
We switch the other uses of ElementTree to lxml for consistency.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
It's possible, I guess, that the URL to a file we want to download
might redirect us. Let's follow redirects.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
We run curl through `SUBPCHECK` when downloading the repo metadata
(currently the only thing we're using it for), which will raise an
exception if it fails. But withouth `-f`, curl still exits 0 for
e.g. a 404. Let's add `-f` so curl will fail if it doesn't get a
200 status.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
It seems repoclosure is sensitive to the configured arch, which
surprised me a bit. So to test non-host arches we need to allow
specifying an arch, which gets passed through to dnf --forcearch.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
From 5.2.15.0, upstream dnf repoclosure ignores rich dependencies:
https://github.com/rpm-software-management/dnf5/pull/2291
so this is redundant. All current Fedoras have at least that
version of dnf.
From 5.4.0.0 onwards, upstream dnf repoclosure handles rich
dependencies correctly. Which is nice.
I don't think I want to bother making this conditional on a dnf
version check, it's a bit of a pain. I think I'm just going to
say you should always use dnf 5.2.15.0 or later to run
rmdepcheck.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
I noticed in https://bodhi.fedoraproject.org/updates/FEDORA-2026-17b6b2edf4
that rmdepcheck's output when an update contains a package with
a pre-existing dependency issue (and does not fix it) is confusing.
We report a 'fixed' dependency in the base repo and a new broken
dependency in the new repo:
Dependency problems in the tested packages themselves:
package: libreoffice-langpack-lv-1:26.2.2.2-0.1.eln155.x86_64 from file:///mnt/update_repo
mythes-lv
Dependencies of other packages that would be FIXED by the tested packages:
package: libreoffice-langpack-lv-1:26.2.2.1-0.1.eln155.x86_64 from https://kojipkgs.fedoraproject.org/repos/eln-build/latest/x86_64
mythes-lv
It's hard to understand what this is telling you. This addresses
the issue (to some extent) by catching cases where the exact same
dependency problem exists for a package with the same name in
both the 'fixed errors in a base repo' list and the 'new errors
in the tested repo' list, and handling them better (we drop them
from those two lists and add them to a new 'unfixed preexisting
errors' list that is reported separately).
This won't handle a case where the dependency is versioned and
bumped as part of the update, I guess, but that seems more
borderline to 'handle'.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
I made this into a shared reusable workflow a while back, so let's
use that. This avoids carrying the model definition in a zillion
different repos.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
I noticed that https://openqa.fedoraproject.org/tests/4471016
passed when it should not have: it should have caught that the
new gstreamer1-plugins-good package had a dependency issue. I was
able to reproduce this locally. Some debugging indicated that,
in get_new_repoclosure, the base repository that should have been
modified was not behaving as if it was modified.
Looking further, we don't actually use the modified repodata for
get_new_repoclosure(), even though the description says we do.
We use the same repository name and DNF cache location, but we
pass the base URL for the original unmodified repo, not the
base URL where the modified repodata created by
get_modified_repoclosure() is (it's been cleaned up by the time
get_new_repoclosure() runs anyway).
This is only noticeable if the tested packages include a dep on
something in a base repo that is removed by the modification
step. The test doesn't cover this scenario so I'm not sure
whether this ever worked - perhaps via DNF's cache? - or has
just been broken all along.
Let's improve it by creating the temporary directory where we
store the modified repodata in main() and passing it to both
get_modified_repoclosure() and get_new_repoclosure(); this way
get_new_repoclosure() can access and use the actual modified
repodata. In my local testing, this fixes the bug and we now
correctly find the dependency error. Let's also improve the tests
to cover this case.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
So I happened to find another reason to work on #10 - it's useful
for figuring out the consequences of dropping a package:
quality/tickets#880 (comment)
The good news is, this turns out to be easy. We only need a small
change to the repository modification code to handle source repos,
where the package entries don't specifiy a sourcerpm - we just
take the package name. We identify this case by looking at the
package arch; if it's "src" we just take the name, if it's
anything else we use the existing approach.
With this change, you can just include the appropriate source
repository as a base repository, and otherwise use rmdepcheck
just the same. e.g. this command tests what happens if you drop
the python-pytest-xprocess package from F44:
./rmdepcheck.py --removes https://dl.fedoraproject.org/pub/fedora/linux/development/44/Everything/x86_64/os/,https://dl.fedoraproject.org/pub/fedora/linux/development/44/Everything/source/tree/ python-pytest-xprocess
You can see it works because there are results for .src packages
from the source repo; these are build dependencies. If you run the
same command without the source repo you get a smaller set of
results covering only the runtime dependencies.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
This is potentially useful for testing package retirements. See
https://pagure.io/releng/issue/11957 .
Signed-off-by: Adam Williamson <awilliam@redhat.com>
This makes the 'headings' in the human-readable output more
understandable. It adds a missing blank line between two sets of
output. And it adds the `--onlyerrors` arg, which skips printing
dependencies that would be *fixed* by the repo under test.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
All of these only need to be iterables, not sequences, and this
makes mypy happy with us using a set of source package names.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
element.find() can return None, which we don't really handle.
All the elements we search for should always be there - if not,
we're dealing with really unexpected repodata, and we probably
want to crash so we can figure out what's going on. This makes
mypy happy.
Signed-off-by: Adam Williamson <awilliam@redhat.com>
