Skip to content
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

Improve provide conflict error message #13661

Merged
merged 2 commits into from
Apr 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions conans/client/graph/graph_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
from conans.client.graph.graph import DepsGraph, Node, CONTEXT_HOST, \
CONTEXT_BUILD, TransitiveRequirement, RECIPE_VIRTUAL
from conans.client.graph.graph import RECIPE_SYSTEM_TOOL
from conans.client.graph.graph_error import GraphError
from conans.client.graph.graph_error import GraphLoopError, GraphConflictError, GraphMissingError, \
GraphRuntimeError, GraphError
from conans.client.graph.profile_node_definer import initialize_conanfile_profile
from conans.client.graph.provides import check_graph_provides
from conans.errors import ConanException
Expand Down Expand Up @@ -77,7 +78,7 @@ def _expand_require(self, require, node, graph, profile_host, profile_build, gra
# print(" Existing previous requirements from ", base_previous, "=>", prev_require)

if prev_require is None:
raise GraphError.loop(node, require, prev_node)
raise GraphLoopError(node, require, prev_node)

prev_ref = prev_node.ref if prev_node else prev_require.ref
if prev_require.force or prev_require.override: # override
Expand Down Expand Up @@ -112,12 +113,12 @@ def _conflicting_version(require, node,
if version_range.contains(prev_ref.version, resolve_prereleases):
require.ref = prev_ref
else:
raise GraphError.conflict(node, require, prev_node, prev_require, base_previous)
raise GraphConflictError(node, require, prev_node, prev_require, base_previous)

elif prev_version_range is not None:
# TODO: Check user/channel conflicts first
if not prev_version_range.contains(require.ref.version, resolve_prereleases):
raise GraphError.conflict(node, require, prev_node, prev_require, base_previous)
raise GraphConflictError(node, require, prev_node, prev_require, base_previous)
else:
def _conflicting_refs(ref1, ref2):
ref1_norev = copy.copy(ref1)
Expand All @@ -135,7 +136,7 @@ def _conflicting_refs(ref1, ref2):
# As we are closing a diamond, there can be conflicts. This will raise if so
conflict = _conflicting_refs(prev_ref, require.ref)
if conflict: # It is possible to get conflict from alias, try to resolve it
raise GraphError.conflict(node, require, prev_node, prev_require, base_previous)
raise GraphConflictError(node, require, prev_node, prev_require, base_previous)

@staticmethod
def _prepare_node(node, profile_host, profile_build, down_options):
Expand Down Expand Up @@ -194,7 +195,7 @@ def _resolve_alias(self, node, require, graph):
self._check_update)
conanfile_path, recipe_status, remote, new_ref = result
except ConanException as e:
raise GraphError.missing(node, require, str(e))
raise GraphMissingError(node, require, str(e))

dep_conanfile = self._loader.load_basic(conanfile_path)
try:
Expand Down Expand Up @@ -244,7 +245,7 @@ def _create_new_node(self, node, require, graph, profile_host, profile_build, gr
self._resolver.resolve(require, str(node.ref), self._remotes, self._update)
resolved = self._resolve_recipe(require.ref, graph_lock)
except ConanException as e:
raise GraphError.missing(node, require, str(e))
raise GraphMissingError(node, require, str(e))

new_ref, dep_conanfile, recipe_status, remote = resolved
# If the node is virtual or a test package, the require is also "root"
Expand Down Expand Up @@ -280,12 +281,12 @@ def _create_new_node(self, node, require, graph, profile_host, profile_build, gr
graph.add_node(new_node)
graph.add_edge(node, new_node, require)
if node.propagate_downstream(require, new_node):
raise GraphError.runtime(node, new_node)
raise GraphRuntimeError(node, new_node)

# This is necessary to prevent infinite loops even when visibility is False
ancestor = node.check_loops(new_node)
if ancestor is not None:
raise GraphError.loop(new_node, require, ancestor)
raise GraphLoopError(new_node, require, ancestor)

return new_node

Expand Down
127 changes: 60 additions & 67 deletions conans/client/graph/graph_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,73 +2,66 @@


class GraphError(ConanException):
# TODO: refactor into multiple classes, do not do type by attribute "kind"
LOOP = "graph loop"
VERSION_CONFLICT = "version conflict"
PROVIDE_CONFLICT = "provide conflict"
MISSING_RECIPE = "missing recipe"
RUNTIME = "runtime"
pass

def __init__(self, kind):
self.kind = kind

class GraphConflictError(GraphError):

def __init__(self, node, require, prev_node, prev_require, base_previous):
self.node = node
self.require = require
self.prev_node = prev_node
self.prev_require = prev_require
self.base_previous = base_previous

def __str__(self):
return f"Version conflict: {self.node.ref}->{self.require.ref}, "\
f"{self.base_previous.ref}->{self.prev_require.ref}."


class GraphLoopError(GraphError):

def __init__(self, node, require, ancestor):
self.node = node
self.require = require
self.ancestor = ancestor

def __str__(self):
return "There is a cycle/loop in the graph:\n"\
f" Initial ancestor: {self.ancestor}\n" \
f" Require: {self.require.ref}\n" \
f" Dependency: {self.node}"


class GraphMissingError(GraphError):

def __init__(self, node, require, missing_error):
self.node = node
self.require = require
self.missing_error = missing_error

def __str__(self):
return f"Package '{self.require.ref}' not resolved: {self.missing_error}."


class GraphProvidesError(GraphError):

def __init__(self, node, conflicting_node):
self.node = node
self.conflicting_node = conflicting_node
node.error = conflicting_node.error

def __str__(self):
return f"Provide Conflict: Both '{self.node.ref}' and '{self.conflicting_node.ref}' " \
f"provide '{self.node.conanfile.provides}'."


class GraphRuntimeError(GraphError):

def __init__(self, node, conflicting_node):
self.node = node
self.conflicting_node = conflicting_node

def __str__(self):
# TODO: Nicer error reporting
if self.kind == GraphError.MISSING_RECIPE:
return f"Package '{self.require.ref}' not resolved: {self.missing_error}"
elif self.kind == GraphError.VERSION_CONFLICT:
return f"Version conflict: {self.node.ref}->{self.require.ref}, "\
f"{self.base_previous.ref}->{self.prev_require.ref}."
elif self.kind == GraphError.LOOP:
return "There is a cycle/loop in the graph:\n"\
f" Initial ancestor: {self.ancestor}\n" \
f" Require: {self.require.ref}\n" \
f" Dependency: {self.node}"
return self.kind

@staticmethod
def loop(node, require, ancestor):
result = GraphError(GraphError.LOOP)
result.node = node
result.require = require
result.ancestor = ancestor
node.error = ancestor.error = result
return result

@staticmethod
def runtime(node, conflicting_node):
result = GraphError(GraphError.RUNTIME)
result.node = node
result.conflicting_node = conflicting_node
node.error = conflicting_node.error = result
return result

@staticmethod
def provides(node, conflicting_node):
result = GraphError(GraphError.PROVIDE_CONFLICT)
result.node = node
result.conflicting_node = conflicting_node
node.error = conflicting_node.error = result
return result

@staticmethod
def missing(node, require, missing_error):
result = GraphError(GraphError.MISSING_RECIPE)
result.node = node
result.require = require
result.missing_error = missing_error
node.error = result
return result

@staticmethod
def conflict(node, require, prev_node, prev_require, base_previous):
result = GraphError(GraphError.VERSION_CONFLICT)
result.node = node
result.require = require
result.prev_node = prev_node
result.prev_require = prev_require
result.base_previous = base_previous
node.error = base_previous.error = result
if prev_node:
prev_node.error = result
return result
return f"Runtime Error: Could not process '{self.node.ref}' with " \
f"'{self.conflicting_node.ref}'."
8 changes: 4 additions & 4 deletions conans/client/graph/provides.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from conans.client.graph.graph_error import GraphError
from conans.client.graph.graph_error import GraphProvidesError
from conans.model.recipe_ref import RecipeReference


Expand All @@ -18,18 +18,18 @@ def check_graph_provides(dep_graph):
for provide in dep_provides:
# First check if collides with current node
if current_provides is not None and provide in current_provides:
raise GraphError.provides(node, dep_node)
raise GraphProvidesError(node, dep_node)

# Then, check if collides with other requirements
new_req = dep_require.copy_requirement()
new_req.ref = RecipeReference(provide, new_req.ref.version, new_req.ref.user,
new_req.ref.channel)
existing = node.transitive_deps.get(new_req)
if existing is not None:
raise GraphError.provides(existing.node, dep_node)
raise GraphProvidesError(existing.node, dep_node)
else:
existing_provide = provides.get(new_req)
if existing_provide is not None:
raise GraphError.provides(existing_provide, dep_node)
raise GraphProvidesError(existing_provide, dep_node)
else:
provides[new_req] = dep_node
20 changes: 10 additions & 10 deletions conans/test/integration/graph/core/graph_manager_test.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest
from parameterized import parameterized

from conans.client.graph.graph_error import GraphError
from conans.client.graph.graph_error import GraphMissingError, GraphLoopError, GraphConflictError
from conans.errors import ConanException
from conans.test.integration.graph.core.graph_manager_base import GraphManagerTest
from conans.test.utils.tools import GenConanfile
Expand Down Expand Up @@ -56,7 +56,7 @@ def test_dependency_missing(self):
deps_graph = self.build_consumer(consumer, install=False)

# TODO: Better error handling
assert deps_graph.error.kind == GraphError.MISSING_RECIPE
assert type(deps_graph.error) == GraphMissingError

self.assertEqual(1, len(deps_graph.nodes))
app = deps_graph.root
Expand Down Expand Up @@ -1402,7 +1402,7 @@ def test_diamond_conflict(self):
consumer = self.recipe_consumer("app/0.1", ["libb/0.1", "libc/0.1"])
deps_graph = self.build_consumer(consumer, install=False)

assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
assert type(deps_graph.error) == GraphConflictError

self.assertEqual(4, len(deps_graph.nodes))
app = deps_graph.root
Expand All @@ -1426,7 +1426,7 @@ def test_shared_conflict_shared(self):

deps_graph = self.build_consumer(consumer, install=False)

assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
assert type(deps_graph.error) == GraphConflictError

self.assertEqual(4, len(deps_graph.nodes))
app = deps_graph.root
Expand Down Expand Up @@ -1455,7 +1455,7 @@ def test_private_conflict(self):

deps_graph = self.build_consumer(consumer, install=False)

assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
assert type(deps_graph.error) == GraphConflictError

self.assertEqual(5, len(deps_graph.nodes))
app = deps_graph.root
Expand Down Expand Up @@ -1636,7 +1636,7 @@ def test_loop(self):

deps_graph = self.build_consumer(consumer, install=False)
# TODO: Better error modeling
assert deps_graph.error.kind == GraphError.LOOP
assert type(deps_graph.error) == GraphLoopError

self.assertEqual(4, len(deps_graph.nodes))

Expand Down Expand Up @@ -1686,7 +1686,7 @@ def test_diamond_conflict(self):

deps_graph = self.build_consumer(consumer, install=False)
assert deps_graph.error is not False
assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
assert type(deps_graph.error) == GraphConflictError

self.assertEqual(2, len(deps_graph.nodes))
app = deps_graph.root
Expand Down Expand Up @@ -1757,7 +1757,7 @@ def test_diamond_reverse_order_conflict(self):
consumer = self.recipe_consumer("app/0.1", ["dep1/2.0", "dep2/1.0"])
deps_graph = self.build_consumer(consumer, install=False)

assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
assert type(deps_graph.error) == GraphConflictError

self.assertEqual(3, len(deps_graph.nodes))
app = deps_graph.root
Expand Down Expand Up @@ -1991,7 +1991,7 @@ def test_project_require_transitive_conflict(self):
build=False, run=True),
install=False)

assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
assert type(deps_graph.error) == GraphConflictError

def test_project_require_apps_transitive(self):
# project -> app1 (app type) -> lib
Expand Down Expand Up @@ -2040,7 +2040,7 @@ def test_project_require_apps_transitive_conflict(self):
"app2/0.1"),
install=False)

assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
assert type(deps_graph.error) == GraphConflictError

def test_project_require_private(self):
# project -(!visible)-> app1 -> lib1
Expand Down
10 changes: 5 additions & 5 deletions conans/test/integration/graph/core/test_build_requires.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from parameterized import parameterized

from conans.client.graph.graph_error import GraphError
from conans.client.graph.graph_error import GraphConflictError, GraphLoopError, GraphRuntimeError
from conans.model.recipe_ref import RecipeReference
from conans.test.integration.graph.core.graph_manager_base import GraphManagerTest
from conans.test.utils.tools import GenConanfile, NO_SETTINGS_PACKAGE_ID, TestClient
Expand Down Expand Up @@ -203,7 +203,7 @@ def test_build_require_transitive_shared(self):
deps_graph = self.build_graph(GenConanfile("app", "0.1").with_require("lib/0.1"),
install=False)

assert deps_graph.error.kind == GraphError.RUNTIME
assert type(deps_graph.error) == GraphRuntimeError

self.assertEqual(6, len(deps_graph.nodes))
app = deps_graph.root
Expand Down Expand Up @@ -508,7 +508,7 @@ def test_conflict_diamond(self):
"libc/0.1"),
install=False)

assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
assert type(deps_graph.error) == GraphConflictError

# Build requires always apply to the consumer
self.assertEqual(4, len(deps_graph.nodes))
Expand Down Expand Up @@ -617,7 +617,7 @@ def test_direct_loop_error(self):
deps_graph = self.build_graph(GenConanfile("app", "0.1").with_build_requires("cmake/0.1"),
install=False)

assert deps_graph.error.kind == GraphError.LOOP
assert type(deps_graph.error) == GraphLoopError

# Build requires always apply to the consumer
self.assertEqual(2, len(deps_graph.nodes))
Expand All @@ -636,7 +636,7 @@ def test_indirect_loop_error(self):
deps_graph = self.build_graph(GenConanfile().with_build_requires("cmake/0.1"),
install=False)

assert deps_graph.error.kind == GraphError.LOOP
assert type(deps_graph.error) == GraphLoopError

# Build requires always apply to the consumer
self.assertEqual(4, len(deps_graph.nodes))
Expand Down
1 change: 0 additions & 1 deletion conans/test/integration/graph/core/test_options.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from conans.client.graph.graph_error import GraphError
from conans.test.assets.genconanfile import GenConanfile
from conans.test.integration.graph.core.graph_manager_base import GraphManagerTest
from conans.test.integration.graph.core.graph_manager_test import _check_transitive
Expand Down
Loading