Skip to content

Conversation

tdawson
Copy link
Member

@tdawson tdawson commented Feb 28, 2025

This adds buildroot functionality to eln-extras
It needs a bit of cleaning up (squashing), and at least one more test, but it works.

@yselkowitz yselkowitz linked an issue Feb 28, 2025 that may be closed by this pull request
@yselkowitz yselkowitz marked this pull request as draft February 28, 2025 16:25
@tdawson tdawson force-pushed the buildroot-extras branch from 66571a6 to be06ae0 Compare March 4, 2025 16:06
@tdawson
Copy link
Member Author

tdawson commented Mar 4, 2025

I have ran tests and it looks like things are working.
I have cleaned up the commits.
I think this is ready to merge.

@yselkowitz
Copy link
Member

Looking at the results you uploaded, a few issues are apparent:

  1. RPMs which are listed in ELN should not be listed in Extras. This was previously working for runtime dependencies but isn't now.
  2. Buildroot results seem to be overwriting, not supplementing, runtime results. This is affecting both the RPMs displayed for an SRPM (e.g. phonon) as well as the level of packages (e.g. qt6-qtwebengine).

@tdawson
Copy link
Member Author

tdawson commented Aug 29, 2025

I just ran a full test and I am not seeing point 2 happening.
I checked both phonon-qt6 and qt6-webengine. With this patch there are more runtime dependencies due to the increase in packages. phonon-qt6 went from 8 to 9 . qt6-webengine went from 35 to 37.

I am seeing AppStream, BaseOS and CRB packages in the package list. I think I know how to get those out.

I'm also seeing packages in the "buildroot" repo. Most of those are related to ghc. I believe these are possibly ELN packages that are no longer in ELN. I'm not going to touch those with this code, at least not on this pass.

@tdawson
Copy link
Member Author

tdawson commented Aug 29, 2025

Looking at the "before" views, it seems this "Listing packages in ELN" has nothing to do with this patch.
If you look at Content Resolver right now, it lists plenty of packages in AppStream, BaseOS and CRB.
So, I'm going to strike that from the "problems" of this patch. That is a separate problem, that needs it's own issue.
https://tiny.distro.builders/view-packages--view-eln-extras.html

@yselkowitz
Copy link
Member

There is a difference. Yes, currently some source packages which are in ELN are also listed in ELN Extras, but only when the binary packages listed in Extras are NOT listed in the ELN runtime set (required+deps, not including builddeps). Some of these might be shipped in BaseOS/AppStream/CRB, but remember that CR doesn't really distinguish by repo. One example of this is git-email (subpackage of git), where git and some of its subpackages are listed, but git-email is not. This is simply a matter of adding to the configs to shift them over to the ELN list (if they are supposed to be shipped in RHEL).

It's been a while since last time, but IIRC there were binary packages in the Extras results that were already listed in the ELN runtime set, and that was the regression. There is a part of the code that essentially says "if this RPM is listed in the base config list, exclude it from the add-on list", and that wasn't seeming to happen after the first patch.

Could you please upload the results of your latest test so that I can review them?

@tdawson
Copy link
Member Author

tdawson commented Aug 29, 2025

I guess I'm not seeing what you are seeing.
I've uploaded my tests and told you offline where to get them.

@yselkowitz
Copy link
Member

To demonstrate point 1:

If you run the following, this shows the RPMs (binary packages) listed in both the ELN runtime set and ELN Extras:

$ diff -u <(curl -s https://tiny.distro.builders/view-binary-package-name-list--view-eln.txt) <(curl -s https://tiny.distro.builders/view-all-binary-package-name-list--view-eln-extras.txt) | grep '^ '
 gdal-libs
 libyuv
 python3-colorama
 python3-jmespath
 socat

Generally, that would return no overlaps due to the deduplication code in CR; the only things listed there are very specific, e.g. packages shipped on only some arches in ELN but are used on all arches in Extras (e.g. python-colorama) or packages which would otherwise not be listed in the runtime but are runtime deps of placeholders (e.g. socat).

However, when I make the same comparison against these results, there's a TON of overlap:

$ diff -u out.2025-08-29-0936/view-binary-package-name-list--view-eln.txt out.2025-08-29-0936/view-all-binary-package-name-list--view-eln-extras.txt  | grep '^ ' | wc -l
2209

zlib-ng is just one example of packages which are required in ELN and listed again in ELN Extras. This indicates that the deduplication isn't happening with the build dep detection.

@tdawson
Copy link
Member Author

tdawson commented Aug 29, 2025

I believe the ELN packages showing up in Extra's is due to a race condition. Doing the buildroot for Extra's is just showing up the problem more.

Here is where the analyzer checks to see if a package is already in the list of ELN (base_vew) packages.
https://github.com/fedora-eln/content-resolver/blob/master/content_resolver/analyzer.py#L1744

This is in the _analyze_view function, which get's called by the _analyze_views function.
https://github.com/fedora-eln/content-resolver/blob/master/content_resolver/analyzer.py#L1784

_analyze_views does the base_view first, which is great. Except that it does it just once per pass.

_analyze_views get's called by each pass here
https://github.com/fedora-eln/content-resolver/blob/master/content_resolver/analyzer.py#L3470

So, here's the real problem. If a package is in extras on the first pass, but not in ELN on the first pass, it get's put in the extras list. There is no second check. So even if the package get's put on ELN on the second pass, it still stays on the extras list.
I did a look through the logs of my tests, and the number of "more" packages in extras on the first pass went up by at least 15,000 packages when we add the buildroot. Thus the amount of packages that are going to hit this race condition is alot more.

But again, this has nothing to do with this pull request. It's just that it's showing up more.

@tdawson
Copy link
Member Author

tdawson commented Aug 29, 2025

If you grab files from the currently Content Resolver:
https://tiny.distro.builders/view-all-binary-package-nevr-list--view-eln.txt
https://tiny.distro.builders/view-all-binary-package-name-list--view-eln-extras.txt

and compare them then you see that we have the following duplicate packages both in Extras and ELN
abseil-cpp-20250512.1-1.eln151
cmake-rpm-macros-3.31.6-4.eln150
fftw-libs-double-3.3.10-16.eln151
flexiblas-netlib64-3.4.5-5.eln151
flexiblas-openblas-openmp64-3.4.5-5.eln151
gi-docgen-2025.4-4.eln151
gi-docgen-fonts-2025.4-4.eln151
google-noto-sans-fonts-20250801-4.eln150
iptables-legacy-devel-1.8.11-11.eln150
iptables-legacy-libs-1.8.11-11.eln150
libabigail-2.8-2.eln150
libappstream-glib-0.8.3-4.eln150
libdbusmenu-16.04.0-30.eln150
libdbusmenu-gtk3-16.04.0-30.eln150
libkdumpfile-devel-0.5.5-3.eln150
libvoikko-4.3.3-4.eln151
libyuv-0-0.57.20240704git96bbdb5.eln150
mingw64-crt-13.0.0-2.eln150
mingw64-filesystem-150-3.eln151
mingw64-libgcc-15.2.1-2.eln151
mingw64-libstdc++-15.2.1-2.eln151
mingw64-win-iconv-0.0.10-3.eln150
mingw64-winpthreads-13.0.0-2.eln150
mingw64-zlib-1.3.1-5.eln150
mingw-binutils-generic-2.44-3.eln150
mingw-filesystem-base-150-3.eln151
openblas-openmp64-0.3.29-2.eln151
openblas-serial-0.3.29-2.eln151
perl-BSD-Resource-1.291.100-30.eln150
perl-Class-Method-Modifiers-2.15-7.eln150
perl-common-sense-3.7.5-20.eln150
perl-Devel-GlobalDestruction-0.14-27.eln150
perl-Exporter-Tiny-1.006002-9.eln150
perl-Import-Into-1.002005-29.eln150
perl-MailTools-2.22-3.eln150
perl-Module-Runtime-0.018-2.eln150
perl-Moo-2.005005-9.eln150
perl-Net-SMTP-SSL-1.04-27.eln150
perl-Pod-Parser-1.67-6.eln150
perl-Role-Tiny-2.002004-14.eln150
perl-strictures-2.000006-22.eln150
perl-Sub-Exporter-Progressive-0.001013-27.eln150
perl-Sub-Quote-2.006009-1.eln151
perl-XString-0.005-17.eln150
python3-colorama-0.4.6-13.eln151
python3-hatch-vcs-0.5.0-4.eln151
python3-jmespath-1.0.1-12.eln151
python3-markdown-3.8.2-4.eln151
python3-setuptools_scm-8.3.1-7.eln151
python3-smartypants-2.0.1-26.eln151
python3-typogrify-2.0.7-26.eln151
snappy-devel-1.2.2-2.eln150
socat-1.8.0.3-1.eln150
systemd-boot-unsigned-258rc3-2.eln151
systemd-rpm-macros-258
rc3-2.eln151
uchardet-0.0.8-8.eln150
voikko-fi-2.5-9.eln150
woff2-1.0.2-23.eln150
zlib-ng-2.2.5-1.eln150

I guess those are supposed to be there.

@tdawson
Copy link
Member Author

tdawson commented Aug 29, 2025

Looking at your comment, you are comparing two different files, one with all, one without. If you do them both all, you will see this.

$ diff -u <(curl -s https://tiny.distro.builders/view-all-binary-package-name-list--view-eln.txt) <(curl -s https://tiny.distro.builders/view-all-binary-package-name-list--view-eln-extras.txt) | grep '^ ' | wc -l
60

@yselkowitz
Copy link
Member

Looking at your comment, you are comparing two different files, one with all, one without.

This is on purpose. The assumption (whether correct or not) is that packages that are only buildroot deps (iow not listed in the runtime set which is required+deps) will not be shipped, therefore having them as dependencies in an add-on (which could only depend on the shipped repos of the base) will need to somehow accommodate the lack of availability of the dependency. While it's not 100% accurate since CR configs aren't completely thorough, it mostly works right now where we're just looking at runtime deps of Extras. (It also highlights a few packages where we can improve the composes, e.g. python-colorama as recently discussed).

Granted, once we start looking at buildroot deps of Extras, there's going to be more overlaps because CRB isn't well accounted for in ELN configs. Some of that is just indicating where we can make the ELN configs more thorough, and perhaps some further changes will be necessary to better handle these. But, as a simple example, all the zlib-ng-compat subpackages are listed as Required in ELN, and if deduplication were working, it shouldn't show up in Extras; the fact that it is still listed would indicate that it's not working.

@tdawson
Copy link
Member Author

tdawson commented Sep 2, 2025

I'll be honest. I really don't know what you want.
You are acting like this functionality was already in Content Resolver.
I showed you that it wasn't, yet you want it in as a new feature claiming it's a "bug of my new code".

I don't even know why you are concered. Does this affect ELN going to RHEL 11? No.
Does it show what packages need to be in CRB? Yes.

To me, this seems like a feature, not a bug.

@tdawson
Copy link
Member Author

tdawson commented Sep 12, 2025

I have created the feature request for removing eln packages from eln-extras view.
#97
It will be a seperate pull request, not related to this pull request.

@tdawson
Copy link
Member Author

tdawson commented Sep 12, 2025

Because this feature, more than doubles the processing time of Content Resolver, I am recommending this does not get merged until this other feature is implemented that should cut down the time.
#92

@tdawson tdawson changed the title DRAFT: Buildroot for eln-extras Buildroot for eln-extras Sep 22, 2025
@tdawson tdawson marked this pull request as ready for review September 22, 2025 19:22
@tdawson
Copy link
Member Author

tdawson commented Sep 22, 2025

After offline discussion with Yaakov, it was decided to merge this today (Monday Sep. 22) so we could see what the results would look like after a run or three.
Merging.

@tdawson tdawson merged commit 7280ef9 into fedora-eln:master Sep 22, 2025
1 check passed
tdawson added a commit to tdawson/content-resolver that referenced this pull request Sep 24, 2025
tdawson added a commit that referenced this pull request Sep 24, 2025
Revert "Merge pull request #83 from tdawson/buildroot-extras"
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.

Build dependency support for Addons

2 participants