-
Notifications
You must be signed in to change notification settings - Fork 154
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
Implement strict repo priority #459
Conversation
@@ -3028,8 +3034,8 @@ solver_ruleclass(Solver *solv, Id rid) | |||
return SOLVER_RULE_CHOICE; | |||
if (rid >= solv->recommendsrules && rid < solv->recommendsrules_end) | |||
return SOLVER_RULE_RECOMMENDS; | |||
if (rid >= solv->blackrules && rid < solv->blackrules_end) | |||
return SOLVER_RULE_BLACK; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: the block was duplicate from L3025
cc @wolfv |
src/rules.c
Outdated
{ | ||
s = pool->solvables + sid; | ||
int max_prio = 0; | ||
FOR_PROVIDES(p, pp, s->name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could cache the max prio here since it's the same for each s->name
id. @mlschroe do you have a suggestion how to make a map? maybe we can allocate an array with the size of pool->ss.nstrings
? Or is there a better way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E.g.
int* cache = solv_calloc(pool->ss.nstrings, sizeof(int));
then we can use that null-initialized buffer to cache values:
if (cache[s->name] == 0)
{
For provides ...
}
else
{
max_prio = cache[s->name] - 1;
}
...
I would do it a bit different:
|
(The code above is completely untested) |
FOR_PROVIDES(p2, pp2, s->name) | ||
{ | ||
Solvable *s2 = pool->solvables + p2; | ||
if (s->name != s2->name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this probably never happens in the case of conda repodata, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good to do anyway as the strict repo prio code will probably be used by other distros as well.
src/rules.c
Outdated
Solvable *s2 = pool->solvables + p2; | ||
if (s->name != s2->name) | ||
{ | ||
MAPCLR(&priomap, p2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why mapclr here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's wrong. you mustn't clear the bits in the first loop, as the second loop also does a MAPTST.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, it only clears if the names are different. It's still wrong, though ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I think I got it, from conda
packages names of providers are always the same so I take a wrong shortcut to clear them if names differ.
In the case where s->name != s2->name
we don't clear s2
but just continue to not prevent to add rules for s2->name
later right?
I'm not sure "strict repo priority" is the correct wording for this behavior. This describes as "weak repo priority" to me, since this explicitly makes it so higher NVRs in another repo still wins (priorities normally make it so lower NVRs win anyway, at least in DNF and Zypper). |
The code doesn't check the version/release at all, so I don't understand your comment. |
I was mostly going off #458. |
c982536
to
2689fed
Compare
@mlschroe I updated that PR, if you have opportunity to take a look it would be great :) |
add specific rule add solver flag add problem message/debug
it would prevent those solvables for being excluded by strict channel priority
2689fed
to
a371cee
Compare
@mlschroe we've been using this in mamba for a while and seems fine! Any objection to merging it? |
+1 on getting this in. This appears to be fixing all the channel priority issues we've had with earlier versions. |
I am not getting clear error messages on conflicts with mamba and strict channel priority. Is there a way to get better error messages? Please see: mamba-org/mamba#1217 |
Description
Implement strict repo priority
Add specific rule
Add solver flag
Add problem message/debug
Closes #458