| 1 |
commit: 0ddfb5802d98c618bd20742be49a520c9a54b394 |
| 2 |
Author: Zac Medico <zmedico <AT> gentoo <DOT> org> |
| 3 |
AuthorDate: Fri Feb 2 06:08:59 2018 +0000 |
| 4 |
Commit: Zac Medico <zmedico <AT> gentoo <DOT> org> |
| 5 |
CommitDate: Fri Feb 2 12:01:25 2018 +0000 |
| 6 |
URL: https://gitweb.gentoo.org/proj/portage.git/commit/?id=0ddfb580 |
| 7 |
|
| 8 |
dep_zapdeps: sort by new_slot_count for DNF only (bug 645002) |
| 9 |
|
| 10 |
Sorting of choices by new_slot_count causes the order of |
| 11 |
choices specified in the ebuild to be discarded, and |
| 12 |
new_slot_count may have variance related to the order that |
| 13 |
packages are added to the graph. This variance can |
| 14 |
contribute to outcomes that appear to be random, like when |
| 15 |
catalyst stage1 builds sometimes pull in paludis to |
| 16 |
satisfy perl-cleaner dependencies. |
| 17 |
|
| 18 |
Meanwhile, the order specified in the ebuild has no |
| 19 |
variance, and the preferences that it specifies can serve |
| 20 |
as a crucial sources of guidance. Therefore, take advantage |
| 21 |
of the order specified in the ebuild whenever possible, and |
| 22 |
use new_slot_count only when it is needed to select optimal |
| 23 |
choices from DNF (as in bug 632026). |
| 24 |
|
| 25 |
This fixes random outcomes in the unit test for bug 645002. |
| 26 |
Previously, the unit test pulled in paludis randomly unless |
| 27 |
portage-utils was added to the arguments. With this patch, |
| 28 |
the portage-utils argument is no longer needed for the unit |
| 29 |
test to succeed consistently. The perl-cleaner dependencies |
| 30 |
do not have any overlapping || deps, so the DNF conversion |
| 31 |
and new_slot_count sorting do not apply, and the first |
| 32 |
choice is preferred regardless of the number of slots that |
| 33 |
it pulls in: |
| 34 |
|
| 35 |
|| ( |
| 36 |
( sys-apps/portage app-portage/portage-utils ) |
| 37 |
sys-apps/pkgcore |
| 38 |
sys-apps/paludis |
| 39 |
) |
| 40 |
|
| 41 |
The _overlap_dnf function now returns the argument object |
| 42 |
when there is no overlap, so OverlapDNFTestCase has to be |
| 43 |
adjusted to account for this. |
| 44 |
|
| 45 |
Bug: https://bugs.gentoo.org/645002 |
| 46 |
Fixes: 9fdaf9bdbdf5 ("dep_check: use DNF to optimize overlapping virtual || deps (bug 632026)") |
| 47 |
|
| 48 |
pym/portage/dep/dep_check.py | 49 ++++++++++++++++++---- |
| 49 |
pym/portage/tests/dep/test_overlap_dnf.py | 2 +- |
| 50 |
.../resolver/test_virtual_minimize_children.py | 12 +++--- |
| 51 |
3 files changed, 47 insertions(+), 16 deletions(-) |
| 52 |
|
| 53 |
diff --git a/pym/portage/dep/dep_check.py b/pym/portage/dep/dep_check.py |
| 54 |
index 7e5a3186e..2896e2389 100644 |
| 55 |
--- a/pym/portage/dep/dep_check.py |
| 56 |
+++ b/pym/portage/dep/dep_check.py |
| 57 |
@@ -298,7 +298,8 @@ class _dep_choice(SlotObject): |
| 58 |
__slots__ = ('atoms', 'slot_map', 'cp_map', 'all_available', |
| 59 |
'all_installed_slots', 'new_slot_count') |
| 60 |
|
| 61 |
-def dep_zapdeps(unreduced, reduced, myroot, use_binaries=0, trees=None): |
| 62 |
+def dep_zapdeps(unreduced, reduced, myroot, use_binaries=0, trees=None, |
| 63 |
+ minimize_slots=False): |
| 64 |
""" |
| 65 |
Takes an unreduced and reduced deplist and removes satisfied dependencies. |
| 66 |
Returned deplist contains steps that must be taken to satisfy dependencies. |
| 67 |
@@ -314,7 +315,8 @@ def dep_zapdeps(unreduced, reduced, myroot, use_binaries=0, trees=None): |
| 68 |
for x, satisfied in zip(unreduced, reduced): |
| 69 |
if isinstance(x, list): |
| 70 |
unresolved += dep_zapdeps(x, satisfied, myroot, |
| 71 |
- use_binaries=use_binaries, trees=trees) |
| 72 |
+ use_binaries=use_binaries, trees=trees, |
| 73 |
+ minimize_slots=minimize_slots) |
| 74 |
elif not satisfied: |
| 75 |
unresolved.append(x) |
| 76 |
return unresolved |
| 77 |
@@ -386,7 +388,8 @@ def dep_zapdeps(unreduced, reduced, myroot, use_binaries=0, trees=None): |
| 78 |
for x, satisfied in zip(deps, satisfieds): |
| 79 |
if isinstance(x, list): |
| 80 |
atoms = dep_zapdeps(x, satisfied, myroot, |
| 81 |
- use_binaries=use_binaries, trees=trees) |
| 82 |
+ use_binaries=use_binaries, trees=trees, |
| 83 |
+ minimize_slots=minimize_slots) |
| 84 |
else: |
| 85 |
atoms = [x] |
| 86 |
if vardb is None: |
| 87 |
@@ -663,9 +666,28 @@ def dep_zapdeps(unreduced, reduced, myroot, use_binaries=0, trees=None): |
| 88 |
for choices in choice_bins: |
| 89 |
if len(choices) < 2: |
| 90 |
continue |
| 91 |
- # Prefer choices with all_installed_slots for bug #480736, and |
| 92 |
- # choices with a smaller number of new slots for bug #632026. |
| 93 |
- choices.sort(key=lambda x: (not x.all_installed_slots, x.new_slot_count)) |
| 94 |
+ |
| 95 |
+ sort_keys = [] |
| 96 |
+ # Prefer choices with all_installed_slots for bug #480736. |
| 97 |
+ sort_keys.append(lambda x: not x.all_installed_slots) |
| 98 |
+ |
| 99 |
+ if minimize_slots: |
| 100 |
+ # Prefer choices having fewer new slots. When used with DNF form, |
| 101 |
+ # this can eliminate unecessary packages that depclean would |
| 102 |
+ # ultimately eliminate (see bug 632026). Only use this behavior |
| 103 |
+ # when deemed necessary by the caller, since this will discard the |
| 104 |
+ # order specified in the ebuild, and the preferences specified |
| 105 |
+ # there can serve as a crucial sources of guidance (see bug 645002). |
| 106 |
+ |
| 107 |
+ # NOTE: Under some conditions, new_slot_count value may have some |
| 108 |
+ # variance from one calculation to the next because it depends on |
| 109 |
+ # the order that packages are added to the graph. This variance can |
| 110 |
+ # contribute to outcomes that appear to be random. Meanwhile, |
| 111 |
+ # the order specified in the ebuild is without variance, so it |
| 112 |
+ # does not have this problem. |
| 113 |
+ sort_keys.append(lambda x: x.new_slot_count) |
| 114 |
+ |
| 115 |
+ choices.sort(key=lambda x: tuple(f(x) for f in sort_keys)) |
| 116 |
for choice_1 in choices[1:]: |
| 117 |
cps = set(choice_1.cp_map) |
| 118 |
for choice_2 in choices: |
| 119 |
@@ -792,8 +814,11 @@ def dep_check(depstring, mydbapi, mysettings, use="yes", mode=None, myuse=None, |
| 120 |
except ParseError as e: |
| 121 |
return [0, "%s" % (e,)] |
| 122 |
|
| 123 |
+ dnf = False |
| 124 |
if mysettings.local_config: # if not repoman |
| 125 |
+ orig_split = mysplit |
| 126 |
mysplit = _overlap_dnf(mysplit) |
| 127 |
+ dnf = mysplit is not orig_split |
| 128 |
|
| 129 |
mysplit2 = dep_wordreduce(mysplit, |
| 130 |
mysettings, mydbapi, mode, use_cache=use_cache) |
| 131 |
@@ -805,7 +830,7 @@ def dep_check(depstring, mydbapi, mysettings, use="yes", mode=None, myuse=None, |
| 132 |
writemsg("mysplit2: %s\n" % (mysplit2), 1) |
| 133 |
|
| 134 |
selected_atoms = dep_zapdeps(mysplit, mysplit2, myroot, |
| 135 |
- use_binaries=use_binaries, trees=trees) |
| 136 |
+ use_binaries=use_binaries, trees=trees, minimize_slots=dnf) |
| 137 |
|
| 138 |
return [1, selected_atoms] |
| 139 |
|
| 140 |
@@ -817,6 +842,12 @@ def _overlap_dnf(dep_struct): |
| 141 |
"|| ( foo bar ) || ( bar baz )" as in bug #632026. Non-overlapping |
| 142 |
groups are excluded from the conversion, since DNF leads to exponential |
| 143 |
explosion of the formula. |
| 144 |
+ |
| 145 |
+ When dep_struct does not contain any overlapping groups, no DNF |
| 146 |
+ conversion will be performed, and dep_struct will be returned as-is. |
| 147 |
+ Callers can detect this case by checking if the returned object has |
| 148 |
+ the same identity as dep_struct. If the identity is different, then |
| 149 |
+ DNF conversion was performed. |
| 150 |
""" |
| 151 |
if not _contains_disjunction(dep_struct): |
| 152 |
return dep_struct |
| 153 |
@@ -847,6 +878,7 @@ def _overlap_dnf(dep_struct): |
| 154 |
|
| 155 |
# group together disjunctions having atom.cp overlap |
| 156 |
traversed = set() |
| 157 |
+ overlap = False |
| 158 |
for cp in overlap_graph: |
| 159 |
if cp in traversed: |
| 160 |
continue |
| 161 |
@@ -863,6 +895,7 @@ def _overlap_dnf(dep_struct): |
| 162 |
stack.append(other_cp) |
| 163 |
|
| 164 |
if len(disjunctions) > 1: |
| 165 |
+ overlap = True |
| 166 |
# convert overlapping disjunctions to DNF |
| 167 |
result.extend(_dnf_convert( |
| 168 |
sorted(disjunctions.values(), key=order_key))) |
| 169 |
@@ -870,7 +903,7 @@ def _overlap_dnf(dep_struct): |
| 170 |
# pass through non-overlapping disjunctions |
| 171 |
result.append(disjunctions.popitem()[1]) |
| 172 |
|
| 173 |
- return result |
| 174 |
+ return result if overlap else dep_struct |
| 175 |
|
| 176 |
|
| 177 |
def _iter_flatten(dep_struct): |
| 178 |
|
| 179 |
diff --git a/pym/portage/tests/dep/test_overlap_dnf.py b/pym/portage/tests/dep/test_overlap_dnf.py |
| 180 |
index 79b3e8e46..ee48e5556 100644 |
| 181 |
--- a/pym/portage/tests/dep/test_overlap_dnf.py |
| 182 |
+++ b/pym/portage/tests/dep/test_overlap_dnf.py |
| 183 |
@@ -12,7 +12,7 @@ class OverlapDNFTestCase(TestCase): |
| 184 |
test_cases = ( |
| 185 |
( |
| 186 |
'|| ( cat/A cat/B ) cat/E || ( cat/C cat/D )', |
| 187 |
- ['cat/E', ['||', 'cat/A', 'cat/B'], ['||', 'cat/C', 'cat/D']], |
| 188 |
+ [['||', 'cat/A', 'cat/B'], 'cat/E', ['||', 'cat/C', 'cat/D']], |
| 189 |
), |
| 190 |
( |
| 191 |
'|| ( cat/A cat/B ) cat/D || ( cat/B cat/C )', |
| 192 |
|
| 193 |
diff --git a/pym/portage/tests/resolver/test_virtual_minimize_children.py b/pym/portage/tests/resolver/test_virtual_minimize_children.py |
| 194 |
index 287445e58..b566cb592 100644 |
| 195 |
--- a/pym/portage/tests/resolver/test_virtual_minimize_children.py |
| 196 |
+++ b/pym/portage/tests/resolver/test_virtual_minimize_children.py |
| 197 |
@@ -168,17 +168,15 @@ class VirtualMinimizeChildrenTestCase(TestCase): |
| 198 |
} |
| 199 |
|
| 200 |
test_cases = ( |
| 201 |
- # Test bug 645002, where we want to prefer choices |
| 202 |
- # based on the number of new slots rather than the total |
| 203 |
- # number of slots. This is necessary so that perl-cleaner's |
| 204 |
- # deps are satisfied by the ( portage portage-utils ) |
| 205 |
- # choice which has a larger total number of slots than the |
| 206 |
- # paludis choice. |
| 207 |
+ # Test bug 645002, where paludis was selected to satisfy a |
| 208 |
+ # perl-cleaner dependency because that choice contained fewer |
| 209 |
+ # packages than the ( portage portage-utils ) choice which |
| 210 |
+ # should have been preferred according to the order of |
| 211 |
+ # choices specified in the ebuild. |
| 212 |
ResolverPlaygroundTestCase( |
| 213 |
[ |
| 214 |
'app-admin/perl-cleaner', |
| 215 |
'virtual/package-manager', |
| 216 |
- 'app-portage/portage-utils', |
| 217 |
], |
| 218 |
all_permutations=True, |
| 219 |
success=True, |
Archives