From 3148ab4888f488bd6d0a54af0eba0402c21c6fac Mon Sep 17 00:00:00 2001 From: memsharded Date: Mon, 21 Feb 2022 17:38:48 +0100 Subject: [PATCH 1/5] fix overrides --- conans/client/graph/graph_builder.py | 3 + conans/model/requires.py | 2 + .../graph/conflict_diamond_test.py | 93 ++++++++----------- 3 files changed, 42 insertions(+), 56 deletions(-) diff --git a/conans/client/graph/graph_builder.py b/conans/client/graph/graph_builder.py index 1d448c18852..e4e2392d982 100644 --- a/conans/client/graph/graph_builder.py +++ b/conans/client/graph/graph_builder.py @@ -76,6 +76,9 @@ def _expand_require(self, require, node, graph, profile_host, profile_build, gra prev_ref = prev_node.ref if prev_node else prev_require.ref if prev_require.force or prev_require.override: # override require.ref = prev_ref + # When closing a loop, if the current version comes forced because force or override + # Then it gains the force trait + require.force = prev_require.force else: self._conflicting_version(require, node, prev_require, prev_node, prev_ref, base_previous) diff --git a/conans/model/requires.py b/conans/model/requires.py index e16b25399a2..8da8e669e3c 100644 --- a/conans/model/requires.py +++ b/conans/model/requires.py @@ -212,6 +212,8 @@ def aggregate(self, other): self.libs |= other.libs self.run = self.run or other.run self.visible |= other.visible + # The force trait is also defined from an override + self.force |= other.force or other.override # TODO: self.package_id_mode => Choose more restrictive? def transform_downstream(self, pkg_type, require, dep_pkg_type): diff --git a/conans/test/integration/graph/conflict_diamond_test.py b/conans/test/integration/graph/conflict_diamond_test.py index 068b45837f0..ab2c7bbed64 100644 --- a/conans/test/integration/graph/conflict_diamond_test.py +++ b/conans/test/integration/graph/conflict_diamond_test.py @@ -1,66 +1,47 @@ -import os -import textwrap -import unittest - import pytest -from conans.util.env import environment_update -from conans.paths import CONANFILE -from conans.test.utils.tools import TestClient, load -import json - - -@pytest.mark.xfail(reason="Conflict Output have changed") -class ConflictDiamondTest(unittest.TestCase): - conanfile = textwrap.dedent(""" - from conan import ConanFile +from conans.test.assets.genconanfile import GenConanfile +from conans.test.utils.tools import TestClient - class HelloReuseConan(ConanFile): - name = "%s" - version = "%s" - requires = %s - """) - def _export(self, name, version, deps=None, export=True): - deps = ", ".join(['"%s"' % d for d in deps or []]) or '""' - conanfile = self.conanfile % (name, version, deps) - files = {CONANFILE: conanfile} - self.client.save(files, clean_first=True) - if export: - self.client.run("export . --user=lasote --channel=stable") +class TestConflictDiamondTest: + def test_basic(self): + c = TestClient() + c.save({"math/conanfile.py": GenConanfile("math"), + "engine/conanfile.py": GenConanfile("engine", "1.0").with_requires("math/1.0"), + "ai/conanfile.py": GenConanfile("ai", "1.0").with_requires("math/1.0.1"), + "game/conanfile.py": GenConanfile("game", "1.0").with_requires("engine/1.0", + "ai/1.0"), + }) + c.run("create math --version=1.0") + c.run("create math --version=1.0.1") + c.run("create math --version=1.0.2") + c.run("create engine") + c.run("create ai") + c.run("install game", assert_error=True) + assert "version conflict" in c.out - def setUp(self): - self.client = TestClient() - self._export("hello0", "0.1") - self._export("hello0", "0.2") - self._export("hello1", "0.1", ["hello0/0.1@lasote/stable"]) - self._export("Hello2", "0.1", ["hello0/0.2@lasote/stable"]) + def _game_conanfile(version, reverse=False): + if reverse: + return GenConanfile("game", "1.0")\ + .with_requirement(f"math/{version}", override=True)\ + .with_requirement("engine/1.0")\ + .with_requirement("ai/1.0") + else: + return GenConanfile("game", "1.0").with_requirement("engine/1.0") \ + .with_requirement("ai/1.0") \ + .with_requirement(f"math/{version}", override=True) - def test_conflict(self): - """ There is a conflict in the graph: branches with requirement in different - version, Conan will raise - """ - self._export("Hello3", "0.1", ["hello1/0.1@lasote/stable", "hello2/0.1@lasote/stable"], - export=False) - self.client.run("install . --build missing", assert_error=True) - self.assertIn("Conflict in hello2/0.1@lasote/stable:\n" - " 'hello2/0.1@lasote/stable' requires 'hello0/0.2@lasote/stable' " - "while 'hello1/0.1@lasote/stable' requires 'hello0/0.1@lasote/stable'.\n" - " To fix this conflict you need to override the package 'hello0' in " - "your root package.", self.client.out) - self.assertNotIn("Generated conaninfo.txt", self.client.out) + for v in ("1.0", "1.0.1", "1.0.2"): + c.save({"game/conanfile.py": _game_conanfile(v)}) + c.run("install game") + c.assert_listed_require({f"math/{v}": "Cache"}) - def test_override_silent(self): - """ There is a conflict in the graph, but the consumer project depends on the conflicting - library, so all the graph will use the version from the consumer project - """ - self._export("Hello3", "0.1", - ["hello1/0.1@lasote/stable", "hello2/0.1@lasote/stable", - "hello0/0.1@lasote/stable"], export=False) - self.client.run("install . --build missing", assert_error=False) - self.assertIn("hello2/0.1@lasote/stable: requirement hello0/0.2@lasote/stable overridden" - " by Hello3/0.1 to hello0/0.1@lasote/stable", - self.client.out) + # Check that order of requirements doesn't affect + for v in ("1.0", "1.0.1", "1.0.2"): + c.save({"game/conanfile.py": _game_conanfile(v, reverse=True)}) + c.run("install game") + c.assert_listed_require({f"math/{v}": "Cache"}) @pytest.mark.xfail(reason="UX conflict error to be completed") From d1909c2f684571dadff5631c37f927d1e31c898b Mon Sep 17 00:00:00 2001 From: memsharded Date: Mon, 21 Feb 2022 17:40:34 +0100 Subject: [PATCH 2/5] fixing override bug --- conans/client/graph/graph_builder.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/conans/client/graph/graph_builder.py b/conans/client/graph/graph_builder.py index e4e2392d982..1d448c18852 100644 --- a/conans/client/graph/graph_builder.py +++ b/conans/client/graph/graph_builder.py @@ -76,9 +76,6 @@ def _expand_require(self, require, node, graph, profile_host, profile_build, gra prev_ref = prev_node.ref if prev_node else prev_require.ref if prev_require.force or prev_require.override: # override require.ref = prev_ref - # When closing a loop, if the current version comes forced because force or override - # Then it gains the force trait - require.force = prev_require.force else: self._conflicting_version(require, node, prev_require, prev_node, prev_ref, base_previous) From af809f4370c9d1b823716afc890b36548190f0fd Mon Sep 17 00:00:00 2001 From: memsharded Date: Mon, 21 Feb 2022 23:03:37 +0100 Subject: [PATCH 3/5] wip --- conans/cli/formatters/graph.py | 4 + conans/client/graph/graph_error.py | 29 +----- conans/client/graph/printer.py | 89 ------------------- .../graph/conflict_diamond_test.py | 38 ++------ .../graph/core/graph_manager_test.py | 13 --- 5 files changed, 14 insertions(+), 159 deletions(-) delete mode 100644 conans/client/graph/printer.py diff --git a/conans/cli/formatters/graph.py b/conans/cli/formatters/graph.py index 2a0212d0848..cecdbb0e954 100644 --- a/conans/cli/formatters/graph.py +++ b/conans/cli/formatters/graph.py @@ -66,6 +66,10 @@ def _format_resolved(title, reqs_to_print): reason = f": {reason}" if reason else "" output.info(" {}{}".format(d, reason), Color.BRIGHT_CYAN) + if graph.error: + output.info("Graph error", Color.BRIGHT_RED) + output.info(" {}".format(graph.error), Color.BRIGHT_RED) + def print_graph_packages(graph): # I am excluding the "download"-"cache" or remote information, that is not diff --git a/conans/client/graph/graph_error.py b/conans/client/graph/graph_error.py index d4f9c4ba708..37cb8118b88 100644 --- a/conans/client/graph/graph_error.py +++ b/conans/client/graph/graph_error.py @@ -2,6 +2,7 @@ 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" @@ -16,6 +17,9 @@ 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}." return self.kind @staticmethod @@ -81,28 +85,3 @@ def conflict_config(node, require, prev_node, prev_require, base_previous, if prev_node: prev_node.error = result return result - - def report_graph_error(self): - # FIXME: THis is completely broken and useless - # print("REPORTING GRAPH ERRORS") - conflict_nodes = [n for n in self.nodes if n.conflict] - # print("PROBLEMATIC NODES ", conflict_nodes) - for node in conflict_nodes: # At the moment there should be only 1 conflict at most - conflict = node.conflict - # print("CONFLICT ", conflict) - if conflict[0] == GraphError.LOOP: - loop_ref = node.ref - parent = node.dependants[0] - parent_ref = parent.src.ref - msg = "Loop detected in context host: '{}' requires '{}' which "\ - "is an ancestor too" - msg = msg.format(parent_ref, loop_ref) - raise ConanException(msg) - elif conflict[0] == GraphError.VERSION_CONFLICT: - raise ConanException( - "There was a version conflict building the dependency graph") - elif conflict[0] == GraphError.PROVIDE_CONFLICT: - raise ConanException( - "There was a provides conflict building the dependency graph") - - raise ConanException("Thre was an error in the graph: {}".format(self.error)) diff --git a/conans/client/graph/printer.py b/conans/client/graph/printer.py deleted file mode 100644 index 6444e807e7a..00000000000 --- a/conans/client/graph/printer.py +++ /dev/null @@ -1,89 +0,0 @@ -import copy -from collections import OrderedDict - - -from conans.client.graph.graph import BINARY_SKIP, RECIPE_CONSUMER, RECIPE_VIRTUAL,\ - RECIPE_EDITABLE -from conans.cli.output import Color, ConanOutput -from conans.model.package_ref import PkgReference - - -def _get_python_requires(conanfile): - result = set() - python_requires = getattr(conanfile, "python_requires", None) - if python_requires: - result.update(conanfile.python_requires.all_refs()) - return result - - -def _print_deprecated(deps_graph): - out = ConanOutput() - deprecated = {} - for node in deps_graph.nodes: - if node.conanfile.deprecated: - deprecated[node.ref] = node.conanfile.deprecated - - if deprecated: - out.info("Deprecated", Color.BRIGHT_YELLOW) - for d, reason in deprecated.items(): - reason = " in favor of '{}'".format(reason) if isinstance(reason, str) else "" - out.info(" {}{}".format(d, reason), Color.BRIGHT_CYAN) - - -def print_graph(deps_graph): - out = ConanOutput() - requires = OrderedDict() - build_requires = OrderedDict() - python_requires = set() - build_time_nodes = deps_graph.build_time_nodes() - for node in sorted(deps_graph.nodes): - python_requires.update(_get_python_requires(node.conanfile)) - if node.recipe in (RECIPE_CONSUMER, RECIPE_VIRTUAL): - continue - pref = PkgReference(node.ref, node.package_id) - if node in build_time_nodes: # TODO: May use build_require_context information - build_requires.setdefault(pref, []).append(node) - else: - requires.setdefault(pref, []).append(node) - - out.info("Requirements", Color.BRIGHT_YELLOW) - - def _recipes(nodes): - for _, list_nodes in nodes.items(): - node = list_nodes[0] # For printing recipes, we can use the first one - if node.recipe == RECIPE_EDITABLE: - from_text = "from user folder" - else: - from_text = ("from local cache" if not node.remote - else "from '%s'" % node.remote.name) - out.info(" %s %s - %s" % (str(node.ref), from_text, node.recipe), - Color.BRIGHT_CYAN) - - _recipes(requires) - if python_requires: - out.info("Python requires", Color.BRIGHT_YELLOW) - for p in python_requires: - out.info(" %s" % str(p), Color.BRIGHT_CYAN) - out.info("Packages", Color.BRIGHT_YELLOW) - - def _packages(nodes): - for package_id, list_nodes in nodes.items(): - # The only way to have more than 1 states is to have 2 - # and one is BINARY_SKIP (privates) - binary = set(n.binary for n in list_nodes) - if len(binary) > 1: - binary.remove(BINARY_SKIP) - assert len(binary) == 1 - binary = binary.pop() - out.info(" %s - %s" % (str(package_id), binary), Color.BRIGHT_CYAN) - _packages(requires) - - if build_requires: - out.info("Build requirements", Color.BRIGHT_YELLOW) - _recipes(build_requires) - out.info("Build requirements packages", Color.BRIGHT_YELLOW) - _packages(build_requires) - - _print_deprecated(deps_graph) - - out.writeln("") diff --git a/conans/test/integration/graph/conflict_diamond_test.py b/conans/test/integration/graph/conflict_diamond_test.py index ab2c7bbed64..57aeb2d7949 100644 --- a/conans/test/integration/graph/conflict_diamond_test.py +++ b/conans/test/integration/graph/conflict_diamond_test.py @@ -1,11 +1,13 @@ -import pytest - from conans.test.assets.genconanfile import GenConanfile from conans.test.utils.tools import TestClient class TestConflictDiamondTest: - def test_basic(self): + def test_version_diamond_conflict(self): + """ + test that we obtain a version conflict with a diamond, and that we can fix it by + defining an override in the "game" consumer + """ c = TestClient() c.save({"math/conanfile.py": GenConanfile("math"), "engine/conanfile.py": GenConanfile("engine", "1.0").with_requires("math/1.0"), @@ -19,7 +21,7 @@ def test_basic(self): c.run("create engine") c.run("create ai") c.run("install game", assert_error=True) - assert "version conflict" in c.out + assert "Version conflict: ai/1.0->math/1.0.1, game/1.0->math/1.0" in c.out def _game_conanfile(version, reverse=False): if reverse: @@ -42,31 +44,3 @@ def _game_conanfile(version, reverse=False): c.save({"game/conanfile.py": _game_conanfile(v, reverse=True)}) c.run("install game") c.assert_listed_require({f"math/{v}": "Cache"}) - - -@pytest.mark.xfail(reason="UX conflict error to be completed") -def test_create_werror(): - client = TestClient() - client.save({"conanfile.py": """from conan import ConanFile -class Pkg(ConanFile): -pass - """}) - client.run("export . --name=LibA --version=0.1 --user=user --channel=channel") - client.run("export conanfile.py --name=LibA --version=0.2 --user=user --channel=channel") - client.save({"conanfile.py": """from conan import ConanFile -class Pkg(ConanFile): -requires = "LibA/0.1@user/channel" - """}) - client.run("export ./ --name=LibB --version=0.1 --user=user --channel=channel") - client.save({"conanfile.py": """from conan import ConanFile -class Pkg(ConanFile): -requires = "LibA/0.2@user/channel" - """}) - client.run("export . --name=LibC --version=0.1 --user=user --channel=channel") - client.save({"conanfile.py": """from conan import ConanFile -class Pkg(ConanFile): -requires = "LibB/0.1@user/channel", "LibC/0.1@user/channel" - """}) - client.run("create ./conanfile.py consumer/0.1@lasote/testing", assert_error=True) - self.assertIn("ERROR: Conflict in LibC/0.1@user/channel", - client.out) diff --git a/conans/test/integration/graph/core/graph_manager_test.py b/conans/test/integration/graph/core/graph_manager_test.py index 09d4e7e8f63..d185fb114de 100644 --- a/conans/test/integration/graph/core/graph_manager_test.py +++ b/conans/test/integration/graph/core/graph_manager_test.py @@ -884,19 +884,6 @@ def test_diamond(self): self._check_node(libb, "libb/0.1#123", deps=[liba], dependents=[app]) self._check_node(liba, "liba/0.2#123", dependents=[libb, app]) - # FIXME: Remove - def test_a_ver_si_me_entero(self): - # consumer -> dep1 -> dep2 - # \-> dep3 - self.recipe_cache("dep2/0") - self.recipe_cache("dep1/0", ["dep2/0"]) - self.recipe_cache("dep3/0") - consumer = self.consumer_conanfile(GenConanfile("consumer", "0").with_require("dep1/0") - .with_require("dep3/0")) - deps_graph = self.build_consumer(consumer) - - self.assertEqual(4, len(deps_graph.nodes)) - def test_diamond_conflict(self): # app -> libb0.1 -> liba0.2 (overriden to lib0.2) # \-> --------- ->/ From e12c45fde9cc7dac613eae51ea8a2ab8ecf94236 Mon Sep 17 00:00:00 2001 From: memsharded Date: Mon, 21 Feb 2022 23:17:13 +0100 Subject: [PATCH 4/5] fix test --- conans/test/integration/remote/download_retries_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conans/test/integration/remote/download_retries_test.py b/conans/test/integration/remote/download_retries_test.py index 1469a0ea6f6..dcaeab5bc81 100644 --- a/conans/test/integration/remote/download_retries_test.py +++ b/conans/test/integration/remote/download_retries_test.py @@ -58,4 +58,4 @@ def get(self, *args, **kwargs): requester_class=BuggyRequester) client.run("install --reference=pkg/0.1@lasote/stable", assert_error=True) self.assertEqual(str(client.out).count("Waiting 0 seconds to retry..."), 2) - self.assertEqual(str(client.out).count("Error 200 downloading"), 3) + self.assertEqual(str(client.out).count("Error 200 downloading"), 4) From 74e809b0c34c91b1b5b830bfce91f2d2196aa2f2 Mon Sep 17 00:00:00 2001 From: memsharded Date: Mon, 21 Feb 2022 23:34:42 +0100 Subject: [PATCH 5/5] fix test --- conans/test/functional/revisions_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conans/test/functional/revisions_test.py b/conans/test/functional/revisions_test.py index cf88c821d61..ef9373e7f4a 100644 --- a/conans/test/functional/revisions_test.py +++ b/conans/test/functional/revisions_test.py @@ -83,7 +83,7 @@ def test_diamond_revisions_conflict(self): self.c_v2.create(project, conanfile=GenConanfile().with_requirement(lib2).with_requirement(lib3), assert_error=True) - self.assertIn("ERROR: version conflict", self.c_v2.out) + self.assertIn("ERROR: Version conflict", self.c_v2.out) # self.assertIn("Different revisions of {} has been requested".format(lib1), self.c_v2.out) def test_alias_to_a_rrev(self):