-
Notifications
You must be signed in to change notification settings - Fork 103
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
Fix repository specific excludes #234
Conversation
There is also another way how to fix it. What is the desired behavior of I believe it should return dnf5/libdnf/common/sack/match_string.cpp Lines 88 to 97 in 1329442
|
It would also make sense to me. It seems like a cleaner solution, especially since we have tests for this. |
d2d3bd9
to
c6d3929
Compare
I tried to fix the behavior of |
c6d3929
to
5fb701a
Compare
The PR looks good, but I think that in the commit msg: |
I should have written the commit message more clearly. The part you mentioned is an example of clearly wrong behaviour of current (unfixed by this pach) implementation. Let me try to re-formulate the commit message. |
Current implementation sometimes behaves wrong in case the `cmp` is negative (e.g. NOT_GLOB) and either the `values` or `patterns` is a vector. For example here are the results of unfixed code: A) match_string("fedora", NOT_GLOB, ["fedora", "testing"]) -> true (which is wrong) B) match_string("fedora", NOT_GLOB, ["updates", "testing"]) -> true (correct) C) match_string("fedora", NOT_GLOB, []) -> false (which is wrong) The new implementation in this patch follows these rules: 1. in case the `cmp` is positive, return true if any of values match at least one of patterns (nothing has changed in this case) 2. if the `cmp` is negative, return true if none of the values match none of the patterns With the patch the exapmle calls will result with: A) match_string("fedora", NOT_GLOB, ["fedora", "testing"]) -> false B) match_string("fedora", NOT_GLOB, ["updates", "testing"]) -> true C) match_string("fedora", NOT_GLOB, []) -> true Resolves #233
No need to filter repositories if the disable_excludes vector is empty.
5fb701a
to
303d0ba
Compare
I see, thanks, it is much more understandable for me now. |
rq.filter_id(disable_excludes, libdnf::sack::QueryCmp::NOT_GLOB) in case of empty
disable_excludes
filters out all repositories.Resolves #233