Skip to content

Feature/migrate to dnf5 swap#112

Draft
bhoy-troy wants to merge 9 commits into
fedora-eln:masterfrom
bhoy-troy:feature/migrate-to-dnf5-swap
Draft

Feature/migrate to dnf5 swap#112
bhoy-troy wants to merge 9 commits into
fedora-eln:masterfrom
bhoy-troy:feature/migrate-to-dnf5-swap

Conversation

@bhoy-troy
Copy link
Copy Markdown
Contributor

@bhoy-troy bhoy-troy commented May 14, 2026

Complete DNF5 Package Relationship Analysis

Summary

Implements full reverse dependency mapping for DNF5

Key Changes

  • Implemented DNF5 relationship analysis - Removed dead DNF4 code and built proper two-pass algorithm
  • Comprehensive documentation Started to add doc-strings to function
  • Code quality improvements - List comprehensions, dictionary literals, optimized filtering
  • Bug fixes - Fixed DnfErr typo, removed unused variables, specific exception handling
  • Package relationship data now populated (required_by, recommended_by, supplements)
  • Potential longer runtime without DNF4 caching, explainer in doc-strings

@bhoy-troy bhoy-troy marked this pull request as ready for review May 15, 2026 14:23
@bhoy-troy bhoy-troy marked this pull request as draft May 15, 2026 14:23
@yselkowitz yselkowitz linked an issue May 15, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Member

@yselkowitz yselkowitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments below. Some of the reformatting commits are too long to review thoroughly and with multiple types of changes in the same commit; any way to break those up more granularly?

Comment on lines +25 to +31
git-core \
python3-yaml \
python3-jinja2 \
python3-koji \
python3-pytest \
python3-flake8 \
python3-libdnf5
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to alphabetize these?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason, the linter I used just kept the same order and placed each dependency on a new line, but I can order them

Comment on lines +155 to +156
split_line = file_line.split()
line_len = len(split_line)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be done much earlier, and then used throughout the code, rather than just for this section?

Comment thread content_resolver/analyzer.py Outdated

elif len(file_line.split()) == 8 or len(file_line.split()) == 3:
pkg_name = file_line.split()[2]
elif line_len in (8, 3):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should be (3, 8)

Comment thread content_resolver/analyzer.py Outdated
required_pkgs.append(pkg_name)

elif len(file_line.split()) == 7 or len(file_line.split()) == 4:
elif line_len in (7, 4):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(4, 7)

Comment thread content_resolver/analyzer.py Outdated
continue

elif len(file_line.split()) == 6 or len(file_line.split()) == 5:
elif line_len in (6, 5):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(5, 6)


Performance Impact:
------------------
- ~2-5 seconds added per analysis
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And yet a run with the dnf5 code was much faster than with dnf4??

Comment thread Dockerfile
Comment on lines +5 to +6
RUN echo "tsflags=nodocs" >> /etc/dnf/dnf.conf && \
echo "install_weak_deps=False" >> /etc/dnf/dnf.conf
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be passed to the dnf command below: --setopt=tsflags=nodocs --setopt=install_weak_deps=False.

Copy link
Copy Markdown
Contributor Author

@bhoy-troy bhoy-troy May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm going to remove this, with the caveat that it may be needed again.

There was issues in the earlier part of the migration that was causing the build to crase, but setting the flags globally seems to resolve the build issues locally.

On retesting without these flags being set it seems to build as expected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scrap that idea, the error isn't building of the image, it builds as expected without the flags being set.

The issue is actually using DNF5 in content resolver, thats why I has to set the flags globally in the config file.

content-resolver-dev  | Resolving build group: repo-eln aarch64
content-resolver-dev  |   Adding packages...
content-resolver-dev  |   Adding groups...
content-resolver-dev  |   Resolving dependencies...
content-resolver-dev  |   Downloading packages...
content-resolver-dev  |   Running DNF transaction, writing RPMDB...
content-resolver-dev  | Error: call to ldconfig failed.
content-resolver-dev  | Error: call to ldconfig failed.
content-resolver-dev  |   Creating a DNF Query object...
content-resolver-dev  |   Done!  (88 packages in total)

@yselkowitz any chance these flags being set have anything to do the warnings you mentioned that are not being hihghlighted

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. What's the issue that you're trying to solve with those options?

They're pretty standard when it comes to building an image, but I definitely don't want to influence dnf5 when run by CR to skip weak deps, we specifically check for those. tsflags=nodocs doesn't affect dependencies though, so that should be safe to be global.

Comment thread content_resolver/analyzer.py Outdated
goal.add_install(pkg)
# TODO: add custom exceptions.MarkingError
except Exception:
except (UserAssertionError ,RepoRpmError) as e:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space here?

Copy link
Copy Markdown
Member

@yselkowitz yselkowitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this with content-resolver-input (eln tag only), and I was shocked how quickly it finished (just over 2 hours from scratch, including buildroot!). I was hoping that the switch to dnf5 might speed things up somewhat, but that is huge, as it makes testing against the real configs manageable.

The package count matched perfectly, but one major difference I did notice was that there were no buildroot warnings (as in https://tiny.distro.builders/view-errors--view-eln.html in the current deployment).

Buildroot warnings arise when build dependencies of SRPMs are not resolvable. Sometimes that is indicative of a real issue (unwanted build dep), but there are a number of intentionally filtered build deps which are "known problems", e.g. rust crate packages which are used for real packages written in rust when building for Fedora but still need to be vendored for RHEL, etc. None of those filtered dependencies showed up, but neither did the expected warnings. This is a bug that needs to be fixed.

I haven't finished analyzing the results for any other differences, but the package count matching is a good start.

@bhoy-troy
Copy link
Copy Markdown
Contributor Author

To reduce the noise with formatting I'll revert the changes in #514054c, we can create a seperate task to do the formatting and keep any logical changes out of it.

@bhoy-troy bhoy-troy force-pushed the feature/migrate-to-dnf5-swap branch from 2132dc1 to b2f7bc8 Compare May 26, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port CR from python3-dnf to python3-libdnf5

2 participants