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.

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

Limit exclude search for only NEVRA #788

Merged
merged 3 commits into from
Aug 3, 2023

Conversation

j-mracek
Copy link
Contributor

@j-mracek j-mracek commented Aug 2, 2023

The issue was created by adding binary search that is enabled by default.

Closes: #763

CI-Test: rpm-software-management/ci-dnf-stack#1353

The issue was created by adding binary search that is enabled by
default.

Closes: rpm-software-management#763
@m-blaha
Copy link
Member

m-blaha commented Aug 2, 2023

I might be worth clarifying in man dnf5 (and in man dnf.conf once it's ready) that --exclude argument accepts <package-name-spec> instead of <package-spec>.
According to man dnf5-specs:
...
The <package-spec> argument is matched against package NEVRAs, provides and file provides.
...
<package-name-spec> is matched against NEVRAs only.
...

.with_nevra = true,
.with_provides = false,
.with_filenames = false,
.with_binaries = false};
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the number of occurrences of ResolveSpecSettings usages, it almost seems that it would be better to use with_binaries = false as the default value for this struct.

Copy link
Member

Choose a reason for hiding this comment

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

There are also multiple places where also GoalJobSettings (which is a subclass of ResolveSpecSettings) is used. And on top of that, almost every Goal class method accepts GoalJobSettings argument with default value const libdnf5::GoalJobSettings & settings = libdnf5::GoalJobSettings().
I guess all calls of these methods need to be checked after change of default value of with_binaries.

@m-blaha
Copy link
Member

m-blaha commented Aug 2, 2023

There still are places where with_binaries=false should have been added (like

pattern + '*', {.ignore_case = false, .with_provides = false, .with_filenames = false}, true);
).
I'm just guessing that you were searching for ResolveSpecSettings to find all affected places. Maybe you should search for with_filenames instead to cover additional occurrences.

It prevents similar to exclude packages according to binary name.
@j-mracek
Copy link
Contributor Author

j-mracek commented Aug 2, 2023

I found additional occurrence. Thank you for suggestions.

@j-mracek
Copy link
Contributor Author

j-mracek commented Aug 2, 2023

There is a file org.rpm.dnf.v0.rpm.Rpm.xml that describes with_<type> possibilities for DBUS. Shall I update it also?

@m-blaha
Copy link
Member

m-blaha commented Aug 2, 2023

There is a file org.rpm.dnf.v0.rpm.Rpm.xml that describes with_<type> possibilities for DBUS. Shall I update it also?

Hm, support for the with_binaries is not implemented yet in the dnfdaemon. So it does not make sense to add it to the documentation (which is generated from this file).

@m-blaha
Copy link
Member

m-blaha commented Aug 2, 2023

There is a file org.rpm.dnf.v0.rpm.Rpm.xml that describes with_<type> possibilities for DBUS. Shall I update it also?

Hm, support for the with_binaries is not implemented yet in the dnfdaemon. So it does not make sense to add it to the documentation (which is generated from this file).

Oh, I didn't notice that you've added the implementation :)
In that case please add it also to the xml file.

@j-mracek
Copy link
Contributor Author

j-mracek commented Aug 3, 2023

Fixed

Copy link
Member

@m-blaha m-blaha left a comment

Choose a reason for hiding this comment

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

Thans!

@m-blaha m-blaha added this pull request to the merge queue Aug 3, 2023
Merged via the queue into rpm-software-management:main with commit 9c3d4ea Aug 3, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Exclude handling is different in dnf5
3 participants