releng/tooling
10
0
Fork 14
Code Pull requests 4 Projects Releases Packages Wiki Activity Actions

fix(orphans): handle alternative providers to eliminate false positives #13031

Open
jnsamyak wants to merge 1 commit from jnsamyak/tooling:fix#11460 into main
pull from: jnsamyak/tooling:fix#11460
merge into: releng:main
Conversation 4 Commits 1 Files changed 1 +63 -19
jnsamyak commented 2026-04-23 17:05:47 +00:00
Owner
Copy link

The problem was that the dependency checker didn't account for alternative providers and rich dependencies.

This fix adds two helper methods to properly handle these cases:

  1. is_requirement_satisfied_by_others() - checks if a requirement has
    alternative providers that aren't being removed. This prevents
    flagging dependencies when other valid providers exist.

  2. has_all_requirements_satisfied() - verifies that a package's
    complete set of requirements can be satisfied without the orphaned
    packages. This properly handles rich dependencies with OR clauses.

Open to discussing this approach.

Fixes: releng/tickets#11460

Signed-off-by: Samyak Jain samyak.jn11@gmail.com

The problem was that the dependency checker didn't account for alternative providers and rich dependencies. This fix adds two helper methods to properly handle these cases: 1. is_requirement_satisfied_by_others() - checks if a requirement has alternative providers that aren't being removed. This prevents flagging dependencies when other valid providers exist. 2. has_all_requirements_satisfied() - verifies that a package's complete set of requirements can be satisfied without the orphaned packages. This properly handles rich dependencies with OR clauses. Open to discussing this approach. Fixes: https://forge.fedoraproject.org/releng/tickets/issues/11460 Signed-off-by: Samyak Jain <samyak.jn11@gmail.com>
fix(orphans): handle alternative providers to eliminate false positives
All checks were successful
Orphaned CI / Run Tox Checks for Orphaned tools (pull_request) Successful in 1m18s
Details
322f5c2164 
Signed-off-by: Samyak Jain <samyak.jn11@gmail.com>
adamwill commented 2026-04-23 17:20:19 +00:00
Member
Copy link

Could my rmdepcheck tool be useful here?

You can use it like this to determine the consequences of removing a package from an arbitrary (set of) repositor(y/ies):

[adamw@toolbx fedora-toolbox-43 rmdepcheck (excludes-sensible %)]$ ./rmdepcheck.py --removes https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ gedit
Dependencies of other packages that would be BROKEN by the specified source package removals:
package: gedit-plugin-bookmarks-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/
  libgedit-49.so()(64bit)
package: gedit-plugin-drawspaces-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/
  libgedit-49.so()(64bit)
package: gedit-plugin-smartspaces-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/
  libgedit-49.so()(64bit)
package: gedit-plugin-wordcompletion-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/
  libgedit-49.so()(64bit)
package: gedit-plugins-data-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/
  gedit

it does this by diffing repoclosure output, which means it uses the "real" dnf/libsolv dependency resolution path, meaning its results should be consistent and correct and the tool doesn't have to laboriously re-invent dependency resolution logic.

Could my [rmdepcheck](https://forge.fedoraproject.org/quality/rmdepcheck) tool be useful here? You can use it like this to determine the consequences of removing a package from an arbitrary (set of) repositor(y/ies): ``` [adamw@toolbx fedora-toolbox-43 rmdepcheck (excludes-sensible %)]$ ./rmdepcheck.py --removes https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ gedit Dependencies of other packages that would be BROKEN by the specified source package removals: package: gedit-plugin-bookmarks-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ libgedit-49.so()(64bit) package: gedit-plugin-drawspaces-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ libgedit-49.so()(64bit) package: gedit-plugin-smartspaces-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ libgedit-49.so()(64bit) package: gedit-plugin-wordcompletion-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ libgedit-49.so()(64bit) package: gedit-plugins-data-49.0-2.fc44.x86_64 from https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ gedit ``` it does this by diffing repoclosure output, which means it uses the "real" dnf/libsolv dependency resolution path, meaning its results should be consistent and correct and the tool doesn't have to laboriously re-invent dependency resolution logic.
gotmax23 commented 2026-04-23 22:21:15 +00:00
Member
Copy link

@adamwill, yeah, I actually did want to talk to you about replacing this script's dep solving code with rmdepcheck. Currently, I don't think it'd work here because this script needs a structured mapping of which other packages would be broken by which orphaned packages so it can get a list of affected package maintainers. rmdepcheck just prints a textual summary of the resolution failures AFAICT.

@adamwill, yeah, I actually did want to talk to you about replacing this script's dep solving code with rmdepcheck. Currently, I don't think it'd work here because this script needs a structured mapping of which other packages would be broken by which orphaned packages so it can get a list of affected package maintainers. rmdepcheck just prints a textual summary of the resolution failures AFAICT.
gotmax23 requested changes 2026-04-23 23:12:10 +00:00
gotmax23 left a comment
Copy link

Thanks for looking into to this! I was aware of the issue but never got around to looking into it because I was worried about performance impacts of additional queries and wanted to prioritize a complete rewrite to simplify the code or at least a libdnf5 port of the existing code. But hopefully, we can find a workable solution in the meantime.

I am concerned that this PR is not the correct solution and will create false negatives, though. I left comments explaining the issue in more detail.

In any case, it would be helpful if you ran the old and new versions of the script and compared execution time to see what kind of performance impact the changes would have.

Thanks for looking into to this! I was aware of the issue but never got around to looking into it because I was worried about performance impacts of additional queries and wanted to prioritize a complete rewrite to simplify the code or at least a libdnf5 port of the existing code. But hopefully, we can find a workable solution in the meantime. I am concerned that this PR is not the correct solution and will create false negatives, though. I left comments explaining the issue in more detail. In any case, it would be helpful if you ran the old and new versions of the script and compared execution time to see what kind of performance impact the changes would have.
packages/orphaned/find_unblocked_orphans.py
@ -401,0 +423,4 @@
"""
# Check each requirement of the package
for req in package.requires:
req_str = str(req).split()[0] # Get base requirement name
gotmax23 commented 2026-04-23 22:45:36 +00:00
Member
Copy link

This logic does not seem quite right to me, and I believe it'll create false negatives. You can't just discard the version constraints and use the "base requirement name" and then assume that another package could satisfy the requirement. Let's say python3-foo and python3-foo2 both Provide python3dist(foo), but the former is python3dist(foo) = 3 and the latter is python3dist(foo) = 2. Let's say that python3-foo2 is an orphan. Let's also say that package bar Requires python3dist(foo) < 3. That means that only python3-foo2 (the provider of python3dist(foo) = 2) would be able to satisfy bar's dependency. But this code just sees that a package other than the orphaned python3-foo2 can satisfy the base requirement name and removes bar from the affected packages list even though the version constraints in the full requirement string makes it so the other package cannot satisfy it.

This logic does not seem quite right to me, and I believe it'll create false negatives. You can't just discard the version constraints and use the "base requirement name" and then assume that another package could satisfy the requirement. Let's say `python3-foo` and `python3-foo2` both Provide `python3dist(foo)`, but the former is `python3dist(foo) = 3` and the latter is `python3dist(foo) = 2`. Let's say that `python3-foo2` is an orphan. Let's also say that package `bar` Requires `python3dist(foo) < 3`. That means that only `python3-foo2` (the provider of `python3dist(foo) = 2`) would be able to satisfy `bar`'s dependency. But this code just sees that a package other than the orphaned `python3-foo2` can satisfy the base requirement name and removes `bar` from the affected packages list even though the version constraints in the full requirement string makes it so the other package can**not** satisfy it.
gotmax23 commented 2026-04-23 22:47:31 +00:00
Member
Copy link

Also, the splitting on spaces like this will break rich dependencies with parentheses and create an invalid requirement string.

Also, the splitting on spaces like this will break rich dependencies with parentheses and create an invalid requirement string.
gotmax23 commented 2026-04-23 23:02:31 +00:00
Member
Copy link

If this is changed to operate on the full requirement string and the is_requirement_satisfied_by_others() check is completely removed, I think this would actually solve the issue.

If this is changed to operate on the full requirement string and the `is_requirement_satisfied_by_others()` check is completely removed, I think this would actually solve the issue.
packages/orphaned/find_unblocked_orphans.py
@ -468,0 +493,4 @@
# Check if this provide is satisfied by other packages
# (not the orphaned package or other packages being removed)
if self.is_requirement_satisfied_by_others(base_provide, rpms):
# This requirement has alternative providers, skip it
gotmax23 commented 2026-04-23 23:09:04 +00:00
Member
Copy link

Even if the base_provide does have alternative providers, we can't skip it here yet because different packages can have version constraints that disqualify the other providers. See the example case in my comment on def has_all_requirements_satisfied().

Even if the `base_provide` does have alternative providers, we can't skip it here yet because different packages can have version constraints that disqualify the other providers. See the example case in my comment on `def has_all_requirements_satisfied()`.
adamwill commented 2026-04-24 00:07:04 +00:00
Member
Copy link

@gotmax23 wrote in #13031 (comment):

@adamwill, yeah, I actually did want to talk to you about replacing this script's dep solving code with rmdepcheck. Currently, I don't think it'd work here because this script needs a structured mapping of which other packages would be broken by which orphaned packages so it can get a list of affected package maintainers. rmdepcheck just prints a textual summary of the resolution failures AFAICT.

rmdepcheck has a JSON output mode:

[adamw@toolbx fedora-toolbox-43 rmdepcheck (excludes-sensible %)]$ ./rmdepcheck.py --json --removes https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ gedit
{
    "newerrors": [
        [
            "gedit-plugin-bookmarks-49.0-2.fc44.x86_64",
            "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/",
            "libgedit-49.so()(64bit)"
        ],
        [
            "gedit-plugin-drawspaces-49.0-2.fc44.x86_64",
            "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/",
            "libgedit-49.so()(64bit)"
        ],
        [
            "gedit-plugin-smartspaces-49.0-2.fc44.x86_64",
            "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/",
            "libgedit-49.so()(64bit)"
        ],
        [
            "gedit-plugin-wordcompletion-49.0-2.fc44.x86_64",
            "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/",
            "libgedit-49.so()(64bit)"
        ],
        [
            "gedit-plugins-data-49.0-2.fc44.x86_64",
            "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/",
            "gedit"
        ]
    ]
}

The first element of each output dep tuple is a package NVR. Currently this is parsed out of dnf repoclosure text output, but I have a ticket to switch to using dnf repoclosure JSON output once it's more widely available. I need to document the output format, but you'd always want the newerrors list for this purpose.

It doesn't currently infer source packages from binary for you, but it could do that, and I've filed myself a ticket to add it as a convenience feature.

If you run rmdepcheck --removes and specify multiple removes it can't really tell you which of them causes a given dep issue without a bunch of extra fancy stuff I don't really want to put in. But it runs very fast (~30 seconds per run, usually) so you can probably just run it on one package at a time. I could actually implement a sort of 'serial mode' where it would run on one package at a time internally so it could give you info per removal, I guess; that would be even faster as it wouldn't have to redo the metadata downloads which it does on each run.

another option would be to just borrow the "repo modification logic" out of rmdepcheck and use it here, I guess; it's very simple these days. I do vaguely have the idea that I could make it exportable from rmdepcheck for other things to use also, I'd have to look at how that should work.

@gotmax23 wrote in https://forge.fedoraproject.org/releng/tooling/pulls/13031#issuecomment-675619: > @adamwill, yeah, I actually did want to talk to you about replacing this script's dep solving code with rmdepcheck. Currently, I don't think it'd work here because this script needs a structured mapping of which other packages would be broken by which orphaned packages so it can get a list of affected package maintainers. rmdepcheck just prints a textual summary of the resolution failures AFAICT. rmdepcheck has a JSON output mode: ``` [adamw@toolbx fedora-toolbox-43 rmdepcheck (excludes-sensible %)]$ ./rmdepcheck.py --json --removes https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/ gedit { "newerrors": [ [ "gedit-plugin-bookmarks-49.0-2.fc44.x86_64", "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/", "libgedit-49.so()(64bit)" ], [ "gedit-plugin-drawspaces-49.0-2.fc44.x86_64", "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/", "libgedit-49.so()(64bit)" ], [ "gedit-plugin-smartspaces-49.0-2.fc44.x86_64", "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/", "libgedit-49.so()(64bit)" ], [ "gedit-plugin-wordcompletion-49.0-2.fc44.x86_64", "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/", "libgedit-49.so()(64bit)" ], [ "gedit-plugins-data-49.0-2.fc44.x86_64", "https://kojipkgs.fedoraproject.org/repos/f45-build/latest/x86_64/", "gedit" ] ] } ``` The first element of each output dep tuple is a package NVR. Currently this is parsed out of `dnf repoclosure` text output, but I have a [ticket](https://forge.fedoraproject.org/quality/rmdepcheck/issues/28) to switch to using `dnf repoclosure` JSON output once it's more widely available. I need to document the output format, but you'd always want the `newerrors` list for this purpose. It doesn't currently infer source packages from binary for you, but it *could* do that, and I've filed myself [a ticket](https://forge.fedoraproject.org/quality/rmdepcheck/issues/31) to add it as a convenience feature. If you run `rmdepcheck --removes` and specify *multiple* removes it can't really tell you which of them causes a given dep issue without a bunch of extra fancy stuff I don't really want to put in. But it runs very fast (~30 seconds per run, usually) so you can probably just run it on one package at a time. I *could* actually implement a sort of 'serial mode' where it would run on one package at a time internally so it could give you info per removal, I guess; that would be even faster as it wouldn't have to redo the metadata downloads which it does on each run. another option would be to just borrow the "repo modification logic" out of rmdepcheck and use it here, I guess; it's very simple these days. I do vaguely have the idea that I could make it exportable from rmdepcheck for other things to use also, I'd have to look at how that should work.
All checks were successful
Orphaned CI / Run Tox Checks for Orphaned tools (pull_request) Successful in 1m18s
Details
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u fix#11460:jnsamyak-fix#11460
git switch jnsamyak-fix#11460

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff jnsamyak-fix#11460
git switch jnsamyak-fix#11460
git rebase main
git switch main
git merge --ff-only jnsamyak-fix#11460
git switch jnsamyak-fix#11460
git rebase main
git switch main
git merge --no-ff jnsamyak-fix#11460
git switch main
git merge --squash jnsamyak-fix#11460
git switch main
git merge --ff-only jnsamyak-fix#11460
git switch main
git merge jnsamyak-fix#11460
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
gotmax23
releng/Owners
Labels
Clear labels   
after freeze

   
automation

   
backlog
Tasks that will take longer then ~4 hours to do

  
blocked

   
change-ack
Changes that have releng work and are accepted

  
change-nak
Changes that have releng work and have been rejected

  
change-noreleng
Changes that do not have any releng work

  
changes
  
Closed As
Can't Fix
 Closed with the reason of Can't Fix

  
Closed As
Duplicate
 Closed with the reason of Duplicate

  
Closed As
Fixed
 Closed with the reason of Fixed

  
Closed As
Fixed with Explanation
 Closed with the reason of Fixed with Explanation

  
Closed As
Get back later
 Closed with the reason of Get back later

  
Closed As
Grooming
 Closed with the reason of Grooming

  
Closed As
Insufficient data
 Closed with the reason of Insufficient data

  
Closed As
Invalid
 Closed with the reason of Invalid

  
Closed As
It's all good
 Closed with the reason of It's all good

  
Closed As
taiga
 Closed with the reason of taiga

  
Closed As
upstream
 Closed with the reason of upstream

  
dev

   
docs

   
easyfix

   
epel

   
f26

   
f27
Fedora 27

  
f28
Fedora 28

  
f29
Fedora 29

  
f30

   
f31

   
f32

   
f33
Fedora 33

  
f34

   
f35
Fedora 35

  
f36

   
f37

   
f38

   
f39
Fedora 39

  
f40

   
f41
Fedora 41

  
f42
Fedora 42

  
f43
Fedora 43

  
f44
Fedora 44

  
fedora

   
groomed

   
high-gain

   
high-trouble

   
in-progress

   
in-review

   
investigation

   
legal

   
low-gain

   
low-trouble

   
mass rebuild

   
medium-gain

   
medium-trouble

   
meeting

   
mini-initiative

   
new_artifact
New artifact for fedora release

  
ops

   
pdc_retirement

   
rawhide

   
RCA
Root cause analysis

  
review

   
script

   
sidetarget

   
sprint-0

   
sprint-1
Releng Sprint 1 Tracker

  
sprint-2
Sprint 2

  
unfrozen
Revisit after freeze

  
waiting on external
  
Backlog Status
Needs Review
 Needs to go through refinement

  
Backlog Status
Ready
 Ready for sprint

  
chore

   
documentation
  
points
01
 A task that is simple, small, and has very little uncertainty. It should be easy enough for any team member to pick up and complete quickly, without a lot of discussion or research.

  
points
02
 This is a task that is slightly more complex or larger than a "1." It might involve two or three simple steps, but the outcome is still very clear and there's little uncertainty.

  
points
03
 Medium-sized task with some noticeable complexity. It might require a bit more thought or collaboration. The steps are well-defined, but it's not a trivial change.

  
points
05
 A significant piece of work. It has more complexity and a higher degree of uncertainty than smaller tasks. It might require some research, a design discussion, or collaboration across multiple teams.

  
points
08
 Complex task with a lot of uncertainty. It's often the largest size that a team will consider for a single sprint. It likely requires a detailed breakdown and multiple team members to complete.

  
points
13
 Any task that feels like it's a "13" or more is considered an Epic. These are too large and uncertain to be worked on in a single sprint and should be broken down.

  
Priority
High
 This is for work that is critical and needs to be addressed in the next sprint. It's often for urgent bugs, important features, or a major piece of technical debt that is blocking other work.

  
Priority
Low
 This is for work that is good to do but not urgent. It's a candidate for being worked on when there's extra time or when it rises in importance. It can stay in the backlog for a longer period.

  
Priority
Medium
 This is for important work that should be done soon, but isn't as urgent as a high-priority item. It will likely be considered for the next 1-2 sprints.

  
Sprint Status
Blocked
 Work items that cannot progress due to external dependencies, blockers, or issues that need resolution.

  
Sprint Status
Done
 Work items that have been completed, reviewed, and integrated. No further work is needed within the current sprint.

  
Sprint Status
In Progress
 Work items currently being worked on by a team member. Active development, research, or implementation is happening.

  
Sprint Status
Review
 Work items that are complete but need review, testing, or approval before being marked as done.

  
Sprint Status
To Do
 Work items that are committed to the sprint but haven't been started yet. These items are ready to be picked up by team members.

  
Technical Debt
Work that addresses shortcuts, workarounds, or suboptimal implementations that were done previously to meet deadlines. These items improve code quality, maintainability, or performance without adding new features.

  
Work Item
Bug
 Issues where the system is not working as intended or designed. Bugs represent deviations from expected behavior.

  
Work Item
Epic
 Large initiatives that are broken down into multiple smaller work items (user stories, tasks, bugs). Epics represent significant features or projects that span multiple sprints.

  
Work Item
Spike
 Time-boxed research, investigation, or proof-of-concept work to reduce uncertainty or explore solutions. Spikes help inform future development decisions.

  
Work Item
Task
 Used to break down larger user stories into smaller, manageable implementation pieces. They focus on the specific technical steps needed to deliver the user story's value.

  
Work Item
User Story
 User-facing features or functionality that can be framed from a user perspective, describing what someone wants to accomplish and why.

No labels
after freeze
automation
backlog
blocked
change-ack
change-nak
change-noreleng
changes
Closed As
Can't Fix
Closed As
Duplicate
Closed As
Fixed
Closed As
Fixed with Explanation
Closed As
Get back later
Closed As
Grooming
Closed As
Insufficient data
Closed As
Invalid
Closed As
It's all good
Closed As
taiga
Closed As
upstream
dev
docs
easyfix
epel
f26
f27
f28
f29
f30
f31
f32
f33
f34
f35
f36
f37
f38
f39
f40
f41
f42
f43
f44
fedora
groomed
high-gain
high-trouble
in-progress
in-review
investigation
legal
low-gain
low-trouble
mass rebuild
medium-gain
medium-trouble
meeting
mini-initiative
new_artifact
ops
pdc_retirement
rawhide
RCA
review
script
sidetarget
sprint-0
sprint-1
sprint-2
unfrozen
waiting on external
Backlog Status
Needs Review
Backlog Status
Ready
chore
documentation
points
01
points
02
points
03
points
05
points
08
points
13
Priority
High
Priority
Low
Priority
Medium
Sprint Status
Blocked
Sprint Status
Done
Sprint Status
In Progress
Sprint Status
Review
Sprint Status
To Do
Technical Debt
Work Item
Bug
Work Item
Epic
Work Item
Spike
Work Item
Task
Work Item
User Story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
releng/tooling!13031
No description provided.