#1060 Detect transitive stream collision
Merged 5 years ago by mprahl. Opened 5 years ago by cqi.

@@ -526,8 +526,14 @@ 

                  # Solve the deps and log the dependency issues.

                  problems = solver.solve(jobs)

                  if problems:

-                     raise RuntimeError("Problems were found during solve(): %s" % ", ".join(

-                                        str(p) for p in problems))

+                     problem_str = self._detect_transitive_stream_collision(problems)

+                     if problem_str:

+                         err_msg = problem_str

+                     else:

+                         err_msg = ', '.join(str(p) for p in problems)

+                     raise RuntimeError(

+                         'Problems were found during module dependency resolution: {}'

+                         .format(err_msg))

                  # Find out what was actually resolved by libsolv to be installed as a result

                  # of our jobs - those are the modules we are looking for.

                  newsolvables = solver.transaction().newsolvables()
@@ -596,3 +602,39 @@ 

          return set(frozenset(s2nsvc(s) for s in transactions[0])

                     for src_alternatives in alternatives.values()

                     for transactions in src_alternatives.values())

+ 

+     @staticmethod

+     def _detect_transitive_stream_collision(problems):

+         """Return problem description if transitive stream collision happens

+ 

+         Transitive stream collision could happen if different buildrequired

+         modules requires same module but with different streams. For example,

+ 

+         app:1 --br--> gtk:1 --req--> baz:1* -------- req --------> platform:f29

+              |                                                     ^

+              +--br--> foo:1 --req--> bar:1 --req--> baz:2* --req---|

+ 

+         as a result, ``baz:1`` will conflicts with ``baz:2``.

+ 

+         :param problems: list of problems returned from ``solv.Solver.solve``.

+         :return: a string of problem description if transitive stream collision

+             is detected. The description is provided by libsolv without

+             changed. If no such collision, None is returned.

+         :rtype: str or None

+         """

+ 

+         def find_conflicts_pairs():

+             for problem in problems:

+                 for rule in problem.findallproblemrules():

+                     info = rule.info()

+                     if info.type == solv.Solver.SOLVER_RULE_PKG_CONFLICTS:

+                         pair = [info.solvable.name, info.othersolvable.name]

+                         pair.sort()  # only for pretty print

+                         yield pair

+ 

+         formatted_conflicts_pairs = ', '.join(

+             '{} and {}'.format(*item) for item in find_conflicts_pairs()

+         )

+         if formatted_conflicts_pairs:

+             return 'The module has conflicting buildrequires of: {}'.format(

+                 formatted_conflicts_pairs)

file modified
+61 -15
@@ -261,22 +261,68 @@ 

  

          assert expanded == expected

  

-     def test_solve_stream_conflicts(self):

-         # app requires both gtk:1 and foo:1.

-         # gtk:1 requires bar:1

-         # foo:1 requires bar:2.

-         # We cannot install both bar:1 and bar:2 in the same time.

-         # Therefore the solving should fail.

-         modules = (

-             ("platform:f29:0:c0", {}),

-             ('gtk:1:1:c2', {'bar': ['1']}),

-             ('foo:1:1:c2', {'bar': ['2']}),

-             ('bar:1:0:c2', {'platform': ['f29']}),

-             ('bar:2:0:c2', {'platform': ['f29']}),

-         )

+     @pytest.mark.parametrize('app_buildrequires, modules, err_msg_regex', (

+         # app --br--> gtk:1 --req--> bar:1* ---req---> platform:f29

+         #    \--br--> foo:1 --req--> bar:2* ---req--/

+         (

+             {'gtk': '1', 'foo': '1'},

+             (

+                 ('platform:f29:0:c0', {}),

+                 ('gtk:1:1:c01', {'bar': ['1']}),

+                 ('bar:1:0:c02', {'platform': ['f29']}),

+                 ('foo:1:1:c03', {'bar': ['2']}),

+                 ('bar:2:0:c04', {'platform': ['f29']}),

+             ),

+             'bar:1:0:c02 and bar:2:0:c04',

+         ),

+         # app --br--> gtk:1 --req--> bar:1* ----------req----------> platform:f29

+         #    \--br--> foo:1 --req--> baz:1 --req--> bar:2* --req--/

+         (

+             {'gtk': '1', 'foo': '1'},

+             (

+                 ('platform:f29:0:c0', {}),

+ 

+                 ('gtk:1:1:c01', {'bar': ['1']}),

+                 ('bar:1:0:c02', {'platform': ['f29']}),

+ 

+                 ('foo:1:1:c03', {'baz': ['1']}),

+                 ('baz:1:1:c04', {'bar': ['2']}),

+                 ('bar:2:0:c05', {'platform': ['f29']}),

+             ),

+             'bar:1:0:c02 and bar:2:0:c05',

+         ),

+         # Test multiple conflicts pairs are detected.

+         # app --br--> gtk:1 --req--> bar:1* ---------req-----------\

+         #    \--br--> foo:1 --req--> baz:1 --req--> bar:2* ---req---> platform:f29

+         #    \--br--> pkga:1 --req--> perl:5' -------req-----------/

+         #    \--br--> pkgb:1 --req--> perl:6' -------req-----------/

+         (

+             {'gtk': '1', 'foo': '1', 'pkga': '1', 'pkgb': '1'},

+             (

+                 ('platform:f29:0:c0', {}),

+ 

+                 ('gtk:1:1:c01', {'bar': ['1']}),

+                 ('bar:1:0:c02', {'platform': ['f29']}),

+ 

+                 ('foo:1:1:c03', {'baz': ['1']}),

+                 ('baz:1:1:c04', {'bar': ['2']}),

+                 ('bar:2:0:c05', {'platform': ['f29']}),

+ 

+                 ('pkga:1:0:c06', {'perl': ['5']}),

+                 ('perl:5:0:c07', {'platform': ['f29']}),

+ 

+                 ('pkgb:1:0:c08', {'perl': ['6']}),

+                 ('perl:6:0:c09', {'platform': ['f29']}),

+             ),

+             # MMD Resolver should still catch a conflict

+             'bar:1:0:c02 and bar:2:0:c05',

+         ),

+     ))

+     def test_solve_stream_conflicts(self, app_buildrequires, modules, err_msg_regex):

          for n, req in modules:

              self.mmd_resolver.add_modules(self._make_mmd(n, req))

  

-         app = self._make_mmd("app:1:0", {'gtk': '1', 'foo': '1'})

-         with pytest.raises(RuntimeError):

+         app = self._make_mmd("app:1:0", app_buildrequires)

+ 

+         with pytest.raises(RuntimeError, match=err_msg_regex):

              self.mmd_resolver.solve(app)

For an explanation of transitive stream collision, please refer to the
source code docstring.

When such collision is detected, error is raised with proper error
message.

Signed-off-by: Chenxiong Qi cqi@redhat.com

I like the idea of this but the error messages are quite cryptic to a user. Perhaps you could change them to be a little clearer. For instance package bar:1:xyz conflicts with module\(bar\) provided by bar:2:xyz, => The module has conflicting buildrequires of bar:1 and bar:2.

Could you also fix the flake8 error?

I think the SOLVER_RULE_PKG_SAME_NAME (or something similar) was the right approach. I would prefer to change it there. also you probably do not need to go through all infos, just us rule.info() or smth like that to get the actual ruleinfo.

rebased onto a8c8eca9eaf965eb9cd7544f29b7d0bf234861f9

5 years ago

rebased onto 660b106c463bad6bdf679f73a4ff308989cc4961

5 years ago

rebased onto c2b51694b601c306f6bec8da57993f9319200658

5 years ago

rebased onto c2b51694b601c306f6bec8da57993f9319200658

5 years ago

rebased onto 68d44c41edf688071e81a815372062fd68de6861

5 years ago

rebased onto 96422b17f5788e9d360dfc1b522bae4d868fe56a

5 years ago

@ignatenkobrain rule.info() works. I updated patch with this change. Regarding the SOLVER_RULE_PKG_SAME_NAME, sorry for confusion, I got this value based on a demo script to simulate the mmd resolver, but MMDResolver just reports SOLVER_RULE_PKG_CONFLICTS.

@mprahl Your comments are addressed. Thanks.

The user will have no idea what solve() means. Perhaps you could say:
Problems were found during module dependency resolution: {}

Perhaps, you could log this error and return something generic to the user instead?

How about something like (not tested)?

conflict_pairs = find_conflicts_pairs()
if not conflicts_pairs:
    return None

conflict_pair_strs = []
for conflict_pair in conflict_pairs:
    conflict_pair_strs.append('{} and {}'.format(*conflict_pair)
return 'The module has conflicting buildrequires of: {}'.format(', and '.join(conflict_pair_strs))

How about something like (not tested)?
conflict_pairs = find_conflicts_pairs()
if not conflicts_pairs:
return None

conflict_pair_strs = []
for conflict_pair in conflict_pairs:
conflict_pair_strs.append('{} and {}'.format(*conflict_pair)
return 'The module has conflicting buildrequires of: {}'.format(', and '.join(conflict_pair_strs))

Maybe this instead:

conflict_pairs = find_conflicts_pairs()
if not conflicts_pairs:
    return None

return 'The module has conflicting buildrequires of: {}'.format(', '.join('{} and {}'.format(*conflict_pair) for conflict_pair in conflict_pairs))

Perhaps, you could log this error and return something generic to the user instead?

I have no idea actually. I didn't think about this because this patch does not change the original behavior when there is no transitive stream collision detected.

The user will have no idea what solve() means. Perhaps you could say:
Problems were found during module dependency resolution: {}

Done.

How about something like (not tested)?

Oh, the return type in docstring is not accurate. rtype should be str only, that is if there is no transitive stream collision, empty string is returned. Or we can also say false value is returned, so it is safe to call _detect_transitive_stream_collision as part of condition in if statement.

find_conflicts_pairs is a generator, so if not conflicts_pairs does not work properly as it's always false.

I'll update the docstring.

rebased onto 5fd229fa327efe8a75acf757c343d8d0118f1955

5 years ago

I'll update the docstring.

Done.

rebased onto 8c6bbb3f82e1ea4b5bf27cb8a39262154475f5db

5 years ago

rebased onto 1fc2728173aac4900cd990cf64969bb1698e2692

5 years ago

Maybe this instead:
conflict_pairs = find_conflicts_pairs()
if not conflicts_pairs:
return None

return 'The module has conflicting buildrequires of: {}'.format(', '.join('{} and {}'.format(*conflict_pair) for conflict_pair in conflict_pairs))

Ah, I got your point. Code is updated accordingly.

app --br--> gtk:1 --req--> bar:1* ---------req-----------\
   \--br--> foo:1 --req--> baz:1 --req--> bar:2* ---req---> platf>rm:f29
   \--br--> pkga:1 --req--> perl:5' -------req-----------/
   \--br--> pkgb:1 --req--> perl:6' -------req-----------/

@ignatenkobrain for this case added to test, mmd resolver does not report conflicts on perl:5 and perl:6. Is this an expected behavior?

You can view the logs from https://ci.centos.org/job/fm-orchestrator-pr/201/console by searching package bar:2:0:c05-0.x86_64 conflicts with module(bar) provided by bar:1:0:c02-0.x86_64.

rebased onto 14678929670193248140a5a4819f8436bed02806

5 years ago

Can you replace that print by log.debug or log.info? Or maybe if it's not needed at all, just remove it?

Once you replace the print(), +1.

rebased onto 879d0da

5 years ago

:thumbsup:

The tests pass locally. Something weird is going on with the container being run in Jenkins. We can debug this later.

Pull-Request has been merged by mprahl

5 years ago