Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add the possibility to load filelists conditionally #2012

Merged
merged 5 commits into from
Jan 26, 2024
Merged

Add the possibility to load filelists conditionally #2012

merged 5 commits into from
Jan 26, 2024

Conversation

jan-kolarik
Copy link
Member

@jan-kolarik jan-kolarik commented Nov 7, 2023 •
edited
Loading

  • Read new optional_metadata_types configuration option from libdnf
  • Request filelists metadata by individual commands
  • Add heuristics to detect filename patterns in spec arguments

Overall, the behavior introduced here is taken from the existing implemention in DNF5 (see this PR).

Currently, the changes should not introduce any new behavior by default. The introduction of dropping the filelists loading by default should come after the system-wide change proposal is accepted.

Requires changes from: rpm-software-management/libdnf#1635.

Related:

Targetted for: https://issues.redhat.com/browse/RHEL-12355.

@m-blaha
Copy link
Member

m-blaha commented Nov 7, 2023 •
edited
Loading

Although according to packaging guidelines there should not be file dependencies out of '/etc', '/usr/bin/', and '/usr/sbin' (see https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_dependencies), there still are such packages in F39:

kata-containers-3.0.1-1.fc38.1.x86_64
   /usr/libexec/virtiofsd
mod_nss-1.0.17-18.fc39.x86_64
   /usr/lib64/libnssckbi.so
mpibash-mpich-examples-1.3-16.fc39.x86_64
   /usr/lib64/mpich/bin/mpibash_mpich
mpibash-openmpi-examples-1.3-16.fc39.x86_64
   /usr/lib64/openmpi/bin/mpibash_openmpi
rt-5.0.4-2.fc39.noarch
   /usr/share/fonts/google-droid-sans-fonts/DroidSans.ttf
   /usr/share/fonts/google-droid-sans-fonts/DroidSansFallbackFull.ttf
scalasca-mpich-2.6.1-2.fc38.x86_64
   /usr/lib64/mpich/bin/scorep-config
scalasca-openmpi-2.6.1-2.fc38.x86_64
   /usr/lib64/openmpi/bin/scorep-config
sqlninja-0.2.999-0.20.alpha1.fc39.noarch
   /usr/share/sqlninja/backscan.pl
   /usr/share/sqlninja/bruteforce.pl
   /usr/share/sqlninja/dirshell.pl
   /usr/share/sqlninja/dns.pl
   /usr/share/sqlninja/escalation.pl
   /usr/share/sqlninja/fingerprint.pl
   /usr/share/sqlninja/getdata.pl
   /usr/share/sqlninja/icmp.pl
   /usr/share/sqlninja/metasploit.pl
   /usr/share/sqlninja/resurrectxp.pl
   /usr/share/sqlninja/revshell.pl
   /usr/share/sqlninja/session.pl
   /usr/share/sqlninja/sqlcmd.pl
   /usr/share/sqlninja/test.pl
   /usr/share/sqlninja/upload.pl
   /usr/share/sqlninja/utils.pl
rt-5.0.5-2.fc39.noarch
   /usr/share/fonts/google-droid-sans-fonts/DroidSans.ttf
   /usr/share/fonts/google-droid-sans-fonts/DroidSansFallbackFull.ttf

What's the plan for these packages which will become uninstallable? Should we at least file a bug on them to fix their dependencies?

@jan-kolarik
Copy link
Member Author

Although according to packaging guidelines there should not be file dependencies out of '/etc', '/usr/bin/', and '/usr/sbin' (see https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_dependencies), there still are such packages in F39:

kata-containers-3.0.1-1.fc38.1.x86_64
   /usr/libexec/virtiofsd
mod_nss-1.0.17-18.fc39.x86_64
   /usr/lib64/libnssckbi.so
mpibash-mpich-examples-1.3-16.fc39.x86_64
   /usr/lib64/mpich/bin/mpibash_mpich
mpibash-openmpi-examples-1.3-16.fc39.x86_64
   /usr/lib64/openmpi/bin/mpibash_openmpi
rt-5.0.4-2.fc39.noarch
   /usr/share/fonts/google-droid-sans-fonts/DroidSans.ttf
   /usr/share/fonts/google-droid-sans-fonts/DroidSansFallbackFull.ttf
scalasca-mpich-2.6.1-2.fc38.x86_64
   /usr/lib64/mpich/bin/scorep-config
scalasca-openmpi-2.6.1-2.fc38.x86_64
   /usr/lib64/openmpi/bin/scorep-config
sqlninja-0.2.999-0.20.alpha1.fc39.noarch
   /usr/share/sqlninja/backscan.pl
   /usr/share/sqlninja/bruteforce.pl
   /usr/share/sqlninja/dirshell.pl
   /usr/share/sqlninja/dns.pl
   /usr/share/sqlninja/escalation.pl
   /usr/share/sqlninja/fingerprint.pl
   /usr/share/sqlninja/getdata.pl
   /usr/share/sqlninja/icmp.pl
   /usr/share/sqlninja/metasploit.pl
   /usr/share/sqlninja/resurrectxp.pl
   /usr/share/sqlninja/revshell.pl
   /usr/share/sqlninja/session.pl
   /usr/share/sqlninja/sqlcmd.pl
   /usr/share/sqlninja/test.pl
   /usr/share/sqlninja/upload.pl
   /usr/share/sqlninja/utils.pl
rt-5.0.5-2.fc39.noarch
   /usr/share/fonts/google-droid-sans-fonts/DroidSans.ttf
   /usr/share/fonts/google-droid-sans-fonts/DroidSansFallbackFull.ttf

What's the plan for these packages which will become uninstallable? Should we at least file a bug on them to fix their dependencies?

Yeah I guess this is a good topic for discussion for our next team meeting. The problem that was already mentioned in the change proposal discussion is that the rule is of SHOULD NOT matter, which means that having such dependencies might be acceptable with a valid excuse. Therefore, we should be aware that there could always be some of these packages present.

It would be great to at least provide users with a hint in case of failures without filelists. However, I'm not certain if this is currently feasible without making changes to the solver.

@jan-kolarik jan-kolarik mentioned this pull request Nov 7, 2023
Merged
@jan-kolarik jan-kolarik marked this pull request as ready for review November 7, 2023 15:15
@jan-kolarik
Copy link
Member Author

@m-blaha I forgot to mention that there is already an existing bug tracking these issues, and there are submissions for individual packages.

@m-blaha
Copy link
Member

m-blaha commented Nov 8, 2023

@m-blaha I forgot to mention that there is already an existing bug tracking these issues, and there are submissions for individual packages.

Oh great, I didn't know about it. In that case consider my comment irrelevant :)

@j-mracek j-mracek self-assigned this Nov 8, 2023
@jan-kolarik jan-kolarik mentioned this pull request Dec 8, 2023
Merged
j-mracek
j-mracek reviewed Dec 12, 2023
View reviewed changes
dnf/base.py Show resolved Hide resolved
@jan-kolarik
util: Add function for detecting file in specs
03f2508 
A helper function used to detect if any spec is a filename pattern.
@jan-kolarik jan-kolarik force-pushed the jkolarik/optional-filelists branch from 938d3bf to f1520c7 Compare January 12, 2024 09:44
@jan-kolarik jan-kolarik mentioned this pull request Jan 12, 2024
Merged
j-mracek
j-mracek reviewed Jan 16, 2024
View reviewed changes
dnf/cli/cli.py Outdated Show resolved Hide resolved
@jan-kolarik jan-kolarik force-pushed the jkolarik/optional-filelists branch 2 times, most recently from c9e98a2 to 44c8de1 Compare January 18, 2024 12:05
@jan-kolarik
base: Setup filelists metadata load flags
1da2afe 
Add the filelists metadata load flag based on the optional_metadata_types option.
@jan-kolarik jan-kolarik force-pushed the jkolarik/optional-filelists branch from 44c8de1 to 2ca7603 Compare January 19, 2024 05:48
@j-mracek
Copy link
Contributor

j-mracek commented Jan 19, 2024 •
edited
Loading

LGTM - the last simplification was really great idea. There is a related comment - rpm-software-management/libdnf#1635 (comment)

This PR cannot be merged without modification of Requires: python3-hawkey version

@jan-kolarik jan-kolarik mentioned this pull request Jan 22, 2024
Closed
@jan-kolarik jan-kolarik force-pushed the jkolarik/optional-filelists branch 2 times, most recently from f640227 to 51f4f4a Compare January 25, 2024 09:16
@j-mracek j-mracek removed the blocked label Jan 25, 2024
@jan-kolarik jan-kolarik force-pushed the jkolarik/optional-filelists branch from 51f4f4a to e9b3954 Compare January 25, 2024 11:14
@j-mracek
Copy link
Contributor

LGTM, CI passed locally.

@j-mracek j-mracek merged commit 0e40047 into master Jan 26, 2024
3 of 6 checks passed
@j-mracek j-mracek deleted the jkolarik/optional-filelists branch January 26, 2024 08:33
gotmax23 added a commit to gotmax23/fedrq-mirror that referenced this pull request Sep 20, 2024
@gotmax23
backends dnf: add load_filelists implementation
ed87b42 
Relates: rpm-software-management/dnf#2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j-mracek j-mracek j-mracek left review comments

Assignees

@j-mracek j-mracek

Labels
None yet
Projects
DNF team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jan-kolarik @m-blaha @j-mracek