As discussed in https://github.com/fedora-ci/rpmdeplint-pipeline/pull/47 , repoclosure module of rpmdeplint is disabled because it's slow (takes an hour per package). This is a very potentially valuable test - dependency breakages are a key thing to detect and a key thing that breaks composes and so on. It should be re-enabled. If we need to make it faster to do that, we should figure that out.
I think there are various possibilities we can look at for making it faster. AIUI, right now it basically tries to solve the deps of everything in Fedora that's not in the set of packages under test against the set of packages under test. I feel like it ought to be possible to do something more targeted than that. Like, first of all find all packages with dependencies against the previous version(s) of the package(s) under test, then restrict the depsolve attempt against the new versions to just those packages? You can do this with dnf repoquery and it doesn't take an hour.
dnf repoquery
We could look into leveraging the fedrq tool - https://fedrq.gtmx.me/ - for this, possibly, or at least reusing its logic. I didn't look into it in detail yet but it just feels like something we could improve.
I guess there are special cases like "this is a new package, there is no previous version" and "this version of the package newly obsoletes some other package", but then I think those affect the current approach too. I don't know if we currently attempt to address them.
So, I poked and prodded and pontificated about this for a while. One interesting thing I found out is that we have something like five versions of more-or-less the same thing:
That seems like a lot! There are differences in focus between them all, but they do kinda overlap too.
Anyway. I considered just trying to 'fix' the rpmdeplint implementation for quite a while. It's slow because it loops over every package in the set, testing installability on each one at a time. I tried unwinding that so it tests installability of all of them at once, but that has a subtle problem: if two packages in the existing 'base' state conflict with each other, they'll be considered to have an existing problem, and if the update we're testing breaks one of them, we might not find out. I assume this is why the loop exists. I think some of the other implementations use a similar loop approach and are fast, but I couldn't totally figure out what the difference is and how to tweak rpmdeplint's version to be fast.
I also considered kinda lift-and-shifting an existing fast implementation into rpmdeplint, but didn't get far with that either; we obviously can't use DNF 5's directly as it's in C++, we'd need to translate it to Python, and that was tricky to figure out with the lacking docs in the area. It'd be 'easier' to steal one of the existing Python implementations, but the ones I knew of at the time (I didn't know about ftbfs-fti) used the old dnf Python interface and ideally we should avoid using that in new stuff, plus it felt weird to add dnf binding usage to rpmdeplint which is otherwise libsolv-based. Possibly now I know about it we could look at the ftbfs-fti implementation? I don't know how fast that runs, though.
Anyhow, in the end, I decided to do something else: I wrote another new thing! But not a new implementation of repoclosure logic. Mine just wraps a dnf repoclosure call, so it uses dnf's own logic, whichever version of dnf is installed. The interesting thing my implementation does is outside of the repoclosure logic itself, it's more in the way it sets up the test configuration.
dnf repoclosure
The idea is to try and reproduce as closely the process that really happens when an update "goes stable", because that's the action we're trying to gate with the check. When an update goes stable, it replaces the previous most-recently-tagged build of the same source package in a given repository: for development releases directly in the "release" repository, for stable releases in the "updates" repository. That means all the binary packages from the previous tagged build are taken out, and all the binary packages from the new build go in.
So, rmdepcheck tries to imitate exactly that process. You pass it the base repository/ies and a repository containing the packages to test. First, it runs dnf repoclosure on the base repository/ies to set a baseline. Then it copies the repository metadata from the base repository/ies to a temporary directory, edits it to remove all binary packages from the source packages in the update, and runs dnf repoclosure again, this time including the modified base repository/ies and the new repository, and checking only the base repository. Then it effectively diffs the output. (As a bonus, it also checks repoclosure of the new repository with the base repository/ies available, which acts as an installability test).
The idea is that this should accurately find all new repoclosure problems that would appear if the packages under test were actually pushed stable, by imitating the process of doing so. It should avoid problems in existing approaches that result from the tested configuration not really matching what would happen when the update is pushed stable, because the binary packages it would still replace are still available to the solver (even if we try to get it to avoid using them with remove instructions, I don't think this is 100% reliable). And since it uses dnf repoclosure for the work, it runs very fast and we don't have another implementation of that logic to maintain.
remove
I've now deployed this test via openQA staging for a trial run. Long term it should run in Fedora CI, but since I control openQA it was easy and fast to do this as a prototype. It will run on most critical path Fedora updates and all ELN updates. I'm going to monitor results for a while and see how accurate it is. It definitely runs fast - it usually takes less than two minutes.
Tricky things to think about with this approach (and just in general) are multilib and obsoletes.
I tried various fancy things to "deal with" multilib, but on the whole, it seems like just sort of ignoring it works best. I'm generally aiming for the test to fail on the side of "false passes" rather than "false failures" as I think that's most appropriate for a gating check, certainly at first. So for now, for x86_64 (which is the only remaining multilib arch), I'm planning to just check the x86_64 packages and ignore i686.
Obsoletes are interesting. Should we consider it "OK" if an update introduces new dependency problems in a package it obsoletes? Tentatively I'd say "for stable releases, no; for development releases, maybe yes". Stable releases are quite easy because packages are not supposed to get obsoleted from stable releases in general, I think. Development is a bit trickier because we kinda should allow for the case, but just obsoleting a package doesn't automatically remove it from the repos. Just because Y obsoletes X, we don't remove X from the repos when Y goes stable, that has to be done separately (either by a spec change to drop a subpackage, or by retiring the source package). So if we allow this, we are assuming the correct corresponding changes will be made at the same time as or soon after the update goes stable.
For now, rmdepcheck doesn't handle obsoletes at all, so it will always flag new dep problems in obsoleted packages as a problem. I'm curious to see how often this really happens. My tentative plan here is to add obsoletes handling as an optional feature, so we can easily decide and change our minds about whether to allow it in various scenarios. (Actually implementing it might be interesting because I'll have to do some parsing and version comparison, I think).
It's notable that rpmdeplint's test suite focuses quite hard on such 'obsoletes' cases, but I think this is mainly intended to handle cases within a single source package - where the subpackages within a single source package change in an update. This isn't a problem for rmdepcheck because its approach "naturally" handles this scenario without special handling; when we edit the metadata of the base repository to remove all binary packages from the tested update's source packages, we "solve" the problem there.
So I think we can say that https://github.com/fedora-ci/rmdepcheck-pipeline/pull/1 kinda replaces this, at least I hope so. rmdepcheck should be a complete replacement for the repoclosure part of rpmdeplint. So far it's been performing pretty well in testing.
Metadata Update from @adamwill: - Issue status updated to: Closed (was: Open)