#5617 Report dependants when a conflict occurs #5684

uilianries · 2019-08-28T22:10:21Z

Update test to validate all conflict message
Retrieve dependents from previous graph node

Changelog: Feature: Report in more details which packages are in conflict
Docs: Omit

Refer to the issue that supports this Pull Request.
If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
I've read the Contributing guide.
I've followed the PEP8 style guides for Python code.
I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

^{Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.}

- Update test to validate all conflict message - Retrieve dependants from previous graph node Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded · 2019-08-29T14:58:19Z

conans/test/unittests/model/transitive_reqs_test.py

        self.retriever.save_recipe(libd_ref, libd_content)

-        with six.assertRaisesRegex(self, ConanException, "Conflict in LibB/0.1@user/testing"):
+        with six.assertRaisesRegex(self, ConanException, "Conflict in LibB/0.1@user/testing\n"


I am not sure I get this message. It says a conflict in LibB/0.1 against a requirement defined by itself?

Sounds weird, but the logic is correct.

I think it should be better this message:

Conflict in LibD/0.1@user/testing Requirement LibA/0.2@user/testing conflicts with already defined LibA/0.1@user/testing in LibB/0.1@user/testing To change it, override it in your base requirements

LibA/0.2@user/testing -> LibA/0.2@user/testing
LibA/0.2@user/testing -> LibB/0.1@user/testing
LibB/0.1@user/testing -> LibA/0.1@user/testing

I still don't understand this.

LibA/0.2@user/testing -> LibA/0.2@user/testing?
Is there a cyclic dependency LibA->LibB->LibA???

No, it should a root node actually.

LibA/0.2@user/testing -> root
LibA/0.2@user/testing -> LibB/0.1@user/testing
LibB/0.1@user/testing -> LibA/0.1@user/testing

The idea is illustrate that LibA/0.2@user/testing in under conflict with LibA/0.1@user/testing due LibB/0.1@user/testing

I raise the stakes and ask if this is a conflict 🙈

If I'm not missing anything, this is the base graph of dependencies before this test:

This test modifies LibD with requires = "LibB/0.1@user/testing", ("LibA/0.2@user/testing", "override") so:

We can consider two scenarios:

if LibD is overriding requirements from LibB there is no such a conflict, as both should point to LibA/0.2 now

every path to root (to consumer) defines a different set of dependencies, then we have two paths and one of them doesn't go through LibD so the dependency to LibA is not overridden in that one. In that case this message is ok, as the conflict is between LibB requiring LibA/0.1 (LibB is not overridden) and LibB requiring LibA/0.2 (if the library is overridden).

Please, tell me that my drawing is wrong, otherwise I would think that here we are not resolving the graph properly.

Yes, this is still a conflict. When there are diamond structures, the overrides, or evaluated transitive dependencies should be the same. In the chain consumer->LibC->LibB, LibC is expecting to use LibA/0.1, there is no one in the chain that specifies otherwise. A newly introduced branch LibD by the consumer shouldn't be changing that behavior without the consumer knowing. An example:

You have your app, that depends on Boost/1.70, which depends on OpenSSL/1.1 which depends on Zlib/1.2.11. All works good.

Now you introduce in your app a new dependency to Poco/1.2 that depends on OpenSSL/1.1. but also has an override to ZLib/1.2.8. You want to detect that, not to downgrade to zlib/1.2.8 without noticing.

Summary: parallel branches in the dependency graph should agree on the versions or they will conflict, they cannot override each other, as there is no "priority" among them.

Ok. Then it is a conflict and it is indeed a conflict between LibB and LibB (it would be really nice to improve the message, but it will require navigating through the graph and right now it is quite difficult).

memsharded · 2019-08-29T15:01:15Z

conans/client/graph/graph_builder.py

                                     "    To change it, override it in your base requirements"
-                                     % (consumer_ref, new_ref, previous_ref))
+                                     % (consumer_ref, new_ref, previous.ref,
+                                        next(iter(previous.dependants)).src))


Why only the first dependant?

The second is the package being built. But I understand your point, if we have more packages using defining the same version, then we should show them all.

This still sounds wrong. The first dependant is not necessarily the one causing the conflict (it can have the version overriden). The second one is not the one being built, could be just another package in the graph.

I see, but as described on #5617 (comment), is hard to track when some conflict occurs. Showing both package and their linked node (package), helps to understand the problem. About the override feature, that's true, but when overriding the version we won't see this error.

ericLemanissier · 2019-09-03T12:25:14Z

suggestion first posted on #3098 (comment): would it be possible to show the dependency chain from the package user asked to create/install/info to the offending package (the one which has no recipe, or the conflicting packages), something like ProjectA/1.0.0@user/stable -> ProjectB/1.0.0@user/stable -> ProjectC/1.0.0@user/stable -> DeepDep/1.0.0@user/stable
There can be several chain from ProjectA to DeepDep, showing any of them is a useful information, showing all of them would be even better.

jgsogo · 2020-01-15T18:37:07Z

Another user with the same issue in #6362. I'm sure we can agree on the message and improve the user experience for the next release.

memsharded

Please merge develop and solve conflicts.
I still need to understand that output...

memsharded · 2020-02-19T12:28:27Z

conans/client/graph/graph_builder.py

                                     "    To change it, override it in your base requirements"
-                                     % (consumer_ref, new_ref, previous_ref))
+                                     % (consumer_ref, new_ref, previous.ref,
+                                        next(iter(previous.dependants)).src))


This still sounds wrong. The first dependant is not necessarily the one causing the conflict (it can have the version overriden). The second one is not the one being built, could be just another package in the graph.

memsharded · 2020-02-19T12:32:42Z

conans/test/unittests/model/transitive_reqs_test.py

        self.retriever.save_recipe(libd_ref, libd_content)

-        with six.assertRaisesRegex(self, ConanException, "Conflict in LibB/0.1@user/testing"):
+        with six.assertRaisesRegex(self, ConanException, "Conflict in LibB/0.1@user/testing\n"


I still don't understand this.

LibA/0.2@user/testing -> LibA/0.2@user/testing?
Is there a cyclic dependency LibA->LibB->LibA???

…ure/conflict-ux

uilianries · 2020-02-19T13:23:40Z

I just merged the develop branch into this PR

memsharded · 2020-02-20T11:48:34Z

Test are broken, please do check when possible.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2020-02-20T18:22:29Z

All green! 👍

jgsogo

I agree with the changes, it is a step forward and will help some users.

If I can comment about the message itself, I would prefer something like:

Conflict in '{consumer_ref}':
    '{consumer_ref}' requires '{new_ref}' while '{next(iter(previous.dependants)).src}' requires '{previous.ref}'.
    To fix this conflict you need to override the package '{new_ref.name}' in your root package.

uilianries · 2020-03-04T21:40:37Z

@jgsogo your message looks better, I'll use it.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded · 2020-03-04T23:23:05Z

@uilianries tests broken :) Please fix them and I will merge this. Thanks!

uilianries · 2020-03-04T23:46:45Z

Thanks for pinging

Signed-off-by: Uilian Ries <uilianries@gmail.com>

jgsogo

🚂

conan-io#5617 Report dependants when a conflict occurs

2e52f8d

- Update test to validate all conflict message - Retrieve dependants from previous graph node Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded reviewed Aug 29, 2019

View reviewed changes

jgsogo mentioned this pull request Sep 3, 2019

Provide info of the dep graph when a recipe is missing #3098

Closed

jgsogo assigned uilianries and memsharded Oct 14, 2019

jgsogo mentioned this pull request Oct 14, 2019

report both conflicting packages #5617

Closed

3 tasks

jgsogo mentioned this pull request Jan 15, 2020

[feature] better diagnostic with regards to conflicts #6362

Closed

jgsogo added this to the 1.23 milestone Feb 4, 2020

memsharded reviewed Feb 19, 2020

View reviewed changes

Merge branch 'develop' of https://github.com/conan-io/conan into feat…

73f3c1f

…ure/conflict-ux

conan-io#5617 Check for alias before the exception

ab91aa3

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded requested a review from jgsogo March 2, 2020 15:05

jgsogo mentioned this pull request Mar 4, 2020

High level documentation of the graph model conan-io/docs#1577

Open

jgsogo reviewed Mar 4, 2020

View reviewed changes

conan-io#5617 Update conflict message

4bb3533

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries added 2 commits March 5, 2020 08:43

Merge branch 'develop' into feature/conflict-ux

25b2082

conan-io#5617 Fix tests for conflict message

37ad566

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded approved these changes Mar 5, 2020

View reviewed changes

jgsogo approved these changes Mar 5, 2020

View reviewed changes

jgsogo merged commit f495493 into conan-io:develop Mar 5, 2020

#5617 Report dependants when a conflict occurs #5684

#5617 Report dependants when a conflict occurs #5684

Uh oh!

Conversation

uilianries commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericLemanissier commented Sep 3, 2019

Uh oh!

jgsogo commented Jan 15, 2020

Uh oh!

memsharded left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

uilianries commented Feb 19, 2020

Uh oh!

memsharded commented Feb 20, 2020

Uh oh!

uilianries commented Feb 20, 2020

Uh oh!

jgsogo left a comment

Choose a reason for hiding this comment

Uh oh!

uilianries commented Mar 4, 2020

Uh oh!

memsharded commented Mar 4, 2020

Uh oh!

uilianries commented Mar 4, 2020

Uh oh!

jgsogo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

uilianries commented Aug 28, 2019 •

edited

Loading