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

Add a method accepting std::string for filter_provides() #1102

Closed
wants to merge 22 commits into from

Conversation

j-mracek
Copy link
Contributor

This is required for Python binding, because when string argument is provided, binding convert it to vector of characters. The change might be problematic because filter_provides({"dnf"}) stop to work.

@@ -281,6 +281,18 @@ class PackageQuery : public PackageSet {
/// @since 5.0
//
// @replaces libdnf/sack/query.hpp:method:addFilter(int keyname, int cmp_type, const char *match) - cmp_type = HY_PKG_PROVIDES
void filter_provides(const std::string & pattern, libdnf5::sack::QueryCmp cmp_type = libdnf5::sack::QueryCmp::EQ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we go with this version the docs need an update, it says A vector of strings .. but it takes just a string.

This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_provides({"dnf"}) stops to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_name({"dnf"}) and
filter_name({"dnf", "acpi"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_epoch({"0"}) and
filter_epoch({"0", "1"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_version({"0.4"}) and
filter_version({"0.3", "1.0"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_release({"0.4"}) and
filter_release({"0.3", "1.0"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_arch({"noarch"}) and
filter_arch({"src", "nosrc"}) stop to work.

Additionally the commit modifies set for source package. Source packages
have architecture "src" and "nosrc".
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_evr({"5-6"}) and
filter_evr({"6-7", "5-6"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_nevra({"pkg-6-7.arch"}) and
filter_nevra({"pkg-6-7.arch", "pkg-0-7.arch"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_sourcerpm({"pkg-6-7.arch"}) and
filter_sourcerpm({"pkg-6-7.arch", "pkg-0-7.arch"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_url({"<url>"}) and
filter_url({"<url>", "<url>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_summary({"<something>"}) and
filter_summary({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_description({"<something>"}) and
filter_description({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_requires({"<something>"}) and
filter_requires({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_conflicts({"<something>"}) and
filter_conflicts({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_obsoletes({"<something>"}) and
filter_obsoletes({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_recommends({"<something>"}) and
filter_recommends({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_suggests({"<something>"}) and
filter_suggests({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_enhances({"<something>"}) and
filter_enhances({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_supplements({"<something>"}) and
filter_supplements({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_file({"<something>"}) and
filter_file({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_location({"<something>"}) and
filter_location({"<something>", "<something>"}) stop to work.
This is required for Python binding, because when a string argument is
provided, the binding converts it to a vector of characters. The change
might be problematic because filter_repo_id({"<something>"}) and
filter_repo_id({"<something>", "<something>"}) stop to work.
@j-mracek j-mracek marked this pull request as ready for review December 18, 2023 11:42
@j-mracek
Copy link
Contributor Author

@jan-kolarik This PR does not change API, but some project might got an issue with compilation. What do you suggest with this PR?

@jan-kolarik
Copy link
Member

@jan-kolarik This PR does not change API, but some project might got an issue with compilation. What do you suggest with this PR?

Hmm, could you please specify the exact issue with compiling the project after these changes are merged?

@j-mracek
Copy link
Contributor Author

The problem is described in commit message: The change might be problematic because filter_arch({"noarch"}) and
filter_arch({"src", "nosrc"}) stop to work. It is a reason why commit 8492018 also modify additional lines (e.g. query.filter_provides({"prv-all"});). The additional overload method cause that compiler is unable to pick the correct methods because both std::string and std::vectorstd::string accept char *.

@jan-kolarik
Copy link
Member

The problem is described in commit message: The change might be problematic because filter_arch({"noarch"}) and filter_arch({"src", "nosrc"}) stop to work. It is a reason why commit 8492018 also modify additional lines (e.g. query.filter_provides({"prv-all"});). The additional overload method cause that compiler is unable to pick the correct methods because both std::string and std::vectorstd::string accept char *.

Thanks, I see now. Well, I feel that all changes that could result in breaking the existing client code should be delivered with the major version bump. Therefore, I'd like to bundle this together with other changes in #1091. Is it OK like that?

@jan-kolarik
Copy link
Member

@j-mracek Could you please target the PR against the https://github.com/rpm-software-management/dnf5/tree/dnf5-5.2.0.0 branch?

@j-mracek
Copy link
Contributor Author

Opened - #1216

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.

3 participants