Repository: conan-io/conan

Issue:
[workspaces] Handling of transitive requirements broken
With conan 1.13.0, the handling of transitive requirements appears broken. I don't have a reproducible test case right now (I can try to construct one), but the situation is as follows:

I want to have a workspace with packages A and B. A depends on C, which has a transitive requirement on D. B depends on D directly.

If I run a regular conan install for A, the conanbuildinfo.cmake contains the definitions for C and D, in particular the CONAN_CMAKE_MODULE_PATH contains both C's and D's directories, which is necessary to process the cmake files of C.

However, with conan workspace install, only B's conanbuildinfo.cmake contains the definitions for D, while A's conanbuildinfo.cmake containts only the definitions for C, which leads to cmake errors, since D's cmake modules cannot be found.

Discussion:

Patch:
diff --git a/conans/client/graph/graph_builder.py b/conans/client/graph/graph_builder.py
index caeab2a9a..d368e0a9a 100644
--- a/conans/client/graph/graph_builder.py
+++ b/conans/client/graph/graph_builder.py
@@ -178,8 +178,16 @@ class DepsGraphBuilder(object):
                                      % (node.ref, require.ref, previous.ref))
 
             previous.ancestors.add(node.name)
+            previous.ancestors.update(node.ancestors)
             node.public_closure[name] = previous
             dep_graph.add_edge(node, previous, require.private, require.build_require)
+            # Update the closure of each dependent
+            for dep_node_name, dep_node in previous.public_deps.items():
+                if dep_node is previous:
+                    continue
+                if dep_node_name in previous.ancestors:
+                    dep_node.public_closure.update(previous.public_closure)
+
             # RECURSION!
             if self._recurse(previous.public_closure, new_reqs, new_options):
                 self._load_deps(dep_graph, previous, new_reqs, node.ref,
diff --git a/conans/client/graph/graph_manager.py b/conans/client/graph/graph_manager.py
index 9fb42d985..8e61481b1 100644
--- a/conans/client/graph/graph_manager.py
+++ b/conans/client/graph/graph_manager.py
@@ -109,7 +109,8 @@ class GraphManager(object):
         processed_profile = ProcessedProfile(profile, create_reference)
         ref = None
         if isinstance(reference, list):  # Install workspace with multiple root nodes
-            conanfile = self._loader.load_virtual(reference, processed_profile)
+            conanfile = self._loader.load_virtual(reference, processed_profile,
+                                                  scope_options=False)
             root_node = Node(ref, conanfile, recipe=RECIPE_VIRTUAL)
         elif isinstance(reference, ConanFileReference):
             if not self._cache.config.revisions_enabled and reference.revision is not None:


Test patch:
diff --git a/conans/test/functional/graph/graph_manager_test.py b/conans/test/functional/graph/graph_manager_test.py
index 3919ee951..75d58e61a 100644
--- a/conans/test/functional/graph/graph_manager_test.py
+++ b/conans/test/functional/graph/graph_manager_test.py
@@ -1,111 +1,11 @@
-import os
-import unittest
-
-from mock import Mock
-
-from conans.client.cache.cache import ClientCache
 from conans.client.graph.graph import RECIPE_CONSUMER, RECIPE_INCACHE
-from conans.client.graph.graph_manager import GraphManager
-from conans.client.graph.proxy import ConanProxy
-from conans.client.graph.python_requires import ConanPythonRequire
-from conans.client.graph.range_resolver import RangeResolver
-from conans.client.installer import BinaryInstaller
-from conans.client.loader import ConanFileLoader
-from conans.client.recorder.action_recorder import ActionRecorder
-from conans.model.graph_info import GraphInfo
-from conans.model.manifest import FileTreeManifest
-from conans.model.options import OptionsValues
-from conans.model.profile import Profile
+from conans.errors import ConanException
 from conans.model.ref import ConanFileReference
-from conans.test.unittests.model.transitive_reqs_test import MockSearchRemote
+from conans.test.functional.graph.graph_manager_base import GraphManagerTest
 from conans.test.utils.conanfile import TestConanFile
-from conans.test.utils.test_files import temp_folder
-from conans.test.utils.tools import TestBufferConanOutput
-from conans.util.files import save
-from conans.errors import ConanException
 from parameterized import parameterized
 
 
-class GraphManagerTest(unittest.TestCase):
-
-    def setUp(self):
-        self.output = TestBufferConanOutput()
-        cache_folder = temp_folder()
-        cache = ClientCache(cache_folder, os.path.join(cache_folder, ".conan"), self.output)
-        self.cache = cache
-        self.remote_search = MockSearchRemote()
-        remote_manager = None
-        self.resolver = RangeResolver(cache, self.remote_search)
-        proxy = ConanProxy(cache, self.output, remote_manager)
-        self.loader = ConanFileLoader(None, self.output, ConanPythonRequire(None, None))
-        self.manager = GraphManager(self.output, cache, remote_manager, self.loader, proxy,
-                                    self.resolver)
-        hook_manager = Mock()
-        recorder = Mock()
-        self.binary_installer = BinaryInstaller(cache, self.output, remote_manager, recorder,
-                                                hook_manager)
-
-    def _cache_recipe(self, reference, test_conanfile, revision=None):
-        if isinstance(test_conanfile, TestConanFile):
-            test_conanfile.info = True
-        ref = ConanFileReference.loads(reference)
-        save(self.cache.conanfile(ref), str(test_conanfile))
-        with self.cache.package_layout(ref).update_metadata() as metadata:
-            metadata.recipe.revision = revision or "123"
-        manifest = FileTreeManifest.create(self.cache.export(ref))
-        manifest.save(self.cache.export(ref))
-
-    def build_graph(self, content, profile_build_requires=None, ref=None):
-        path = temp_folder()
-        path = os.path.join(path, "conanfile.py")
-        save(path, str(content))
-        self.loader.cached_conanfiles = {}
-
-        profile = Profile()
-        if profile_build_requires:
-            profile.build_requires = profile_build_requires
-        profile.process_settings(self.cache)
-        update = check_updates = False
-        recorder = ActionRecorder()
-        remote_name = None
-        build_mode = []
-        ref = ref or ConanFileReference(None, None, None, None, validate=False)
-        options = OptionsValues()
-        graph_info = GraphInfo(profile, options, root_ref=ref)
-        deps_graph, _ = self.manager.load_graph(path, None, graph_info,
-                                                build_mode, check_updates, update,
-                                                remote_name, recorder)
-        self.binary_installer.install(deps_graph, False, graph_info)
-        return deps_graph
-
-    def _check_node(self, node, ref, deps, build_deps, dependents, closure, public_deps):
-        conanfile = node.conanfile
-        ref = ConanFileReference.loads(str(ref))
-        self.assertEqual(node.ref.full_repr(), ref.full_repr())
-        self.assertEqual(conanfile.name, ref.name)
-        self.assertEqual(len(node.dependencies), len(deps) + len(build_deps))
-        self.assertEqual(len(node.dependants), len(dependents))
-
-        public_deps = {n.name: n for n in public_deps}
-        self.assertEqual(node.public_deps, public_deps)
-
-        # The recipe requires is resolved to the reference WITH revision!
-        self.assertEqual(len(deps), len(conanfile.requires))
-        for dep in deps:
-            self.assertEqual(conanfile.requires[dep.name].ref,
-                             dep.ref)
-
-        self.assertEqual(closure, node.public_closure)
-        libs = []
-        envs = []
-        for n in closure:
-            libs.append("mylib%s%slib" % (n.ref.name, n.ref.version))
-            envs.append("myenv%s%senv" % (n.ref.name, n.ref.version))
-        self.assertEqual(conanfile.deps_cpp_info.libs, libs)
-        env = {"MYENV": envs} if envs else {}
-        self.assertEqual(conanfile.deps_env_info.vars, env)
-
-
 class TransitiveGraphTest(GraphManagerTest):
     def test_basic(self):
         # say/0.1
@@ -190,6 +90,151 @@ class TransitiveGraphTest(GraphManagerTest):
         self._check_node(liba, "liba/0.1@user/testing#123", deps=[], build_deps=[],
                          dependents=[libb, libc], closure=[], public_deps=[app, libb, libc, liba])
 
+    def test_consecutive_diamonds(self):
+        # app -> libe0.1 -> libd0.1 -> libb0.1 -> liba0.1
+        #    \-> libf0.1 ->/    \-> libc0.1 ->/
+        liba_ref = "liba/0.1@user/testing"
+        libb_ref = "libb/0.1@user/testing"
+        libc_ref = "libc/0.1@user/testing"
+        libd_ref = "libd/0.1@user/testing"
+        libe_ref = "libe/0.1@user/testing"
+        libf_ref = "libf/0.1@user/testing"
+        self._cache_recipe(liba_ref, TestConanFile("liba", "0.1"))
+        self._cache_recipe(libb_ref, TestConanFile("libb", "0.1", requires=[liba_ref]))
+        self._cache_recipe(libc_ref, TestConanFile("libc", "0.1", requires=[liba_ref]))
+        self._cache_recipe(libd_ref, TestConanFile("libd", "0.1", requires=[libb_ref, libc_ref]))
+        self._cache_recipe(libe_ref, TestConanFile("libe", "0.1", requires=[libd_ref]))
+        self._cache_recipe(libf_ref, TestConanFile("libf", "0.1", requires=[libd_ref]))
+        deps_graph = self.build_graph(TestConanFile("app", "0.1", requires=[libe_ref, libf_ref]))
+
+        self.assertEqual(7, len(deps_graph.nodes))
+        app = deps_graph.root
+        libe = app.dependencies[0].dst
+        libf = app.dependencies[1].dst
+        libd = libe.dependencies[0].dst
+        libb = libd.dependencies[0].dst
+        libc = libd.dependencies[1].dst
+        liba = libc.dependencies[0].dst
+
+        public_deps = [app, libb, libc, liba, libd, libe, libf]
+        self._check_node(app, "app/0.1@None/None", deps=[libe, libf], build_deps=[], dependents=[],
+                         closure=[libe, libf, libd, libb, libc, liba], public_deps=public_deps)
+        self._check_node(libe, "libe/0.1@user/testing#123", deps=[libd], build_deps=[],
+                         dependents=[app], closure=[libd, libb, libc, liba],
+                         public_deps=public_deps)
+        self._check_node(libf, "libf/0.1@user/testing#123", deps=[libd], build_deps=[],
+                         dependents=[app], closure=[libd, libb, libc, liba],
+                         public_deps=public_deps)
+        self._check_node(libd, "libd/0.1@user/testing#123", deps=[libb, libc], build_deps=[],
+                         dependents=[libe, libf], closure=[libb, libc, liba],
+                         public_deps=public_deps)
+        self._check_node(libb, "libb/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app], closure=[liba], public_deps=public_deps)
+        self._check_node(libb, "libb/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app], closure=[liba], public_deps=public_deps)
+        self._check_node(liba, "liba/0.1@user/testing#123", deps=[], build_deps=[],
+                         dependents=[libb, libc], closure=[], public_deps=public_deps)
+
+    def test_parallel_diamond(self):
+        # app -> libb0.1 -> liba0.1
+        #    \-> libc0.1 ->/
+        #    \-> libd0.1 ->/
+        liba_ref = "liba/0.1@user/testing"
+        libb_ref = "libb/0.1@user/testing"
+        libc_ref = "libc/0.1@user/testing"
+        libd_ref = "libd/0.1@user/testing"
+        self._cache_recipe(liba_ref, TestConanFile("liba", "0.1"))
+        self._cache_recipe(libb_ref, TestConanFile("libb", "0.1", requires=[liba_ref]))
+        self._cache_recipe(libc_ref, TestConanFile("libc", "0.1", requires=[liba_ref]))
+        self._cache_recipe(libd_ref, TestConanFile("libd", "0.1", requires=[liba_ref]))
+        deps_graph = self.build_graph(TestConanFile("app", "0.1",
+                                                    requires=[libb_ref, libc_ref, libd_ref]))
+
+        self.assertEqual(5, len(deps_graph.nodes))
+        app = deps_graph.root
+        libb = app.dependencies[0].dst
+        libc = app.dependencies[1].dst
+        libd = app.dependencies[2].dst
+        liba = libb.dependencies[0].dst
+
+        public_deps = [app, libb, libc, liba, libd]
+        self._check_node(app, "app/0.1@None/None", deps=[libb, libc, libd], build_deps=[],
+                         dependents=[], closure=[libb, libc, libd, liba], public_deps=public_deps)
+        self._check_node(libb, "libb/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app], closure=[liba], public_deps=public_deps)
+        self._check_node(libc, "libc/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app], closure=[liba], public_deps=public_deps)
+        self._check_node(libd, "libd/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app], closure=[liba], public_deps=public_deps)
+        self._check_node(liba, "liba/0.1@user/testing#123", deps=[], build_deps=[],
+                         dependents=[libb, libc, libd], closure=[], public_deps=public_deps)
+
+    def test_nested_diamond(self):
+        # app --------> libb0.1 -> liba0.1
+        #    \--------> libc0.1 ->/
+        #     \-> libd0.1 ->/
+        liba_ref = "liba/0.1@user/testing"
+        libb_ref = "libb/0.1@user/testing"
+        libc_ref = "libc/0.1@user/testing"
+        libd_ref = "libd/0.1@user/testing"
+        self._cache_recipe(liba_ref, TestConanFile("liba", "0.1"))
+        self._cache_recipe(libb_ref, TestConanFile("libb", "0.1", requires=[liba_ref]))
+        self._cache_recipe(libc_ref, TestConanFile("libc", "0.1", requires=[liba_ref]))
+        self._cache_recipe(libd_ref, TestConanFile("libd", "0.1", requires=[libc_ref]))
+        deps_graph = self.build_graph(TestConanFile("app", "0.1",
+                                                    requires=[libb_ref, libc_ref, libd_ref]))
+
+        self.assertEqual(5, len(deps_graph.nodes))
+        app = deps_graph.root
+        libb = app.dependencies[0].dst
+        libc = app.dependencies[1].dst
+        libd = app.dependencies[2].dst
+        liba = libb.dependencies[0].dst
+
+        public_deps = [app, libb, libc, liba, libd]
+        self._check_node(app, "app/0.1@None/None", deps=[libb, libc, libd], build_deps=[],
+                         dependents=[], closure=[libb, libd, libc, liba], public_deps=public_deps)
+        self._check_node(libb, "libb/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app], closure=[liba], public_deps=public_deps)
+        self._check_node(libc, "libc/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app, libd], closure=[liba], public_deps=public_deps)
+        self._check_node(libd, "libd/0.1@user/testing#123", deps=[libc], build_deps=[],
+                         dependents=[app], closure=[libc, liba], public_deps=public_deps)
+        self._check_node(liba, "liba/0.1@user/testing#123", deps=[], build_deps=[],
+                         dependents=[libb, libc], closure=[], public_deps=public_deps)
+
+    def test_multiple_transitive(self):
+        # https://github.com/conanio/conan/issues/4720
+        # app -> libb0.1  -> libc0.1 -> libd0.1
+        #    \--------------->/          /
+        #     \------------------------>/
+        libb_ref = "libb/0.1@user/testing"
+        libc_ref = "libc/0.1@user/testing"
+        libd_ref = "libd/0.1@user/testing"
+        self._cache_recipe(libd_ref, TestConanFile("libd", "0.1"))
+        self._cache_recipe(libc_ref, TestConanFile("libc", "0.1", requires=[libd_ref]))
+        self._cache_recipe(libb_ref, TestConanFile("libb", "0.1", requires=[libc_ref]))
+        deps_graph = self.build_graph(TestConanFile("liba", "0.1",
+                                                    requires=[libd_ref, libc_ref, libb_ref]))
+
+        self.assertEqual(4, len(deps_graph.nodes))
+        liba = deps_graph.root
+        libd = liba.dependencies[0].dst
+        libc = liba.dependencies[1].dst
+        libb = liba.dependencies[2].dst
+
+        self._check_node(libd, "libd/0.1@user/testing#123", deps=[], build_deps=[],
+                         dependents=[libb, libc], closure=[], public_deps=[liba, libb, libc, libd])
+        self._check_node(liba, "liba/0.1@None/None", deps=[libd, libc, libb], build_deps=[],
+                         dependents=[],
+                         closure=[libb, libc, libd], public_deps=[liba, libb, libc, libd])
+        self._check_node(libb, "libb/0.1@user/testing#123", deps=[libc], build_deps=[],
+                         dependents=[liba], closure=[libc, libd],
+                         public_deps=[liba, libb, libc, libd])
+        self._check_node(libc, "libc/0.1@user/testing#123", deps=[libd], build_deps=[],
+                         dependents=[liba, libb], closure=[libd],
+                         public_deps=[liba, libb, libc, libd])
+
     def test_diamond_conflict(self):
         # app -> libb0.1 -> liba0.1
         #    \-> libc0.1 -> liba0.2
diff --git a/conans/test/functional/graph/transitive_deps_test.py b/conans/test/functional/graph/transitive_deps_test.py
new file mode 100644
index 000000000..2a59107f4
--- /dev/null
+++ b/conans/test/functional/graph/transitive_deps_test.py
@@ -0,0 +1,48 @@
+# coding=utf-8
+
+import os
+import unittest
+
+from parameterized import parameterized
+
+from conans.model.ref import PackageReference
+from conans.test.utils.conanfile import TestConanFile
+from conans.test.utils.tools import TestClient
+from conans.util.files import load
+
+
+class TransitiveDepsTest(unittest.TestCase):
+
+    def setUp(self):
+        # Create dependency graph: C -> B -> A
+        self.t = TestClient()
+
+        self.t.save({"A/conanfile.py": str(TestConanFile("AAAA", "1.0"))})
+        self.t.run("export A user/channel")
+
+        self.t.save({"B/conanfile.py": str(TestConanFile("BBBB", "1.0",
+                                                         requires=["AAAA/1.0@user/channel"]))})
+        self.t.run("export B user/channel")
+        self.t.save({"C/conanfile.py":
+                     str(TestConanFile("CCCC", "1.0", requires=["BBBB/1.0@user/channel"])) +
+                    "    generators='cmake'"})
+        self.t.run("export C user/channel")
+
+    @parameterized.expand([(True, ), (False, )])
+    def test_consume(self, inverse_order):
+        deps = sorted(["AAAA/1.0@user/channel", "CCCC/1.0@user/channel"], reverse=inverse_order)
+
+        # Create D -> A,C
+        self.t.save({"D/conanfile.py": str(TestConanFile("DDDD", "1.0",
+                                                         requires=deps))})
+        self.t.run("create ./D user/channel --build=missing")
+
+        # C is being built
+        pref_c = PackageReference.loads(
+            "CCCC/1.0@user/channel:aef99e6ca67e3d2da47927c4ec1ec7129129943f")
+        self.assertIn("{} - Build".format(pref_c), self.t.out)
+        build_folder = self.t.cache.build(pref=pref_c)
+
+        # It contains information about AAAA
+        conanbuildinfo = load(os.path.join(build_folder, "conanbuildinfo.cmake"))
+        self.assertIn("set(CONAN_AAAA_ROOT ", conanbuildinfo)
diff --git a/conans/test/functional/workspace/workspace_test.py b/conans/test/functional/workspace/workspace_test.py
index 6e5031be0..a29d31664 100644
--- a/conans/test/functional/workspace/workspace_test.py
+++ b/conans/test/functional/workspace/workspace_test.py
@@ -189,6 +189,96 @@ class WorkspaceTest(unittest.TestCase):
             for f in ("conanbuildinfo.cmake", "conaninfo.txt", "conanbuildinfo.txt"):
                 self.assertTrue(os.path.exists(os.path.join(client.current_folder, sub, f)))
 
+    def multiple_roots_test(self):
+        # https://github.com/conan-io/conan/issues/4720
+        client = TestClient()
+
+        def files(name, depend=None):
+            deps = ('"Hello%s/0.1@lasote/stable"' % depend) if depend else "None"
+            return {"conanfile.py": conanfile_build.format(deps=deps, name=name)}
+
+        client.save(files("D"), path=os.path.join(client.current_folder, "D"))
+        client.save(files("C", "D"), path=os.path.join(client.current_folder, "C"))
+        client.save(files("A", "C"), path=os.path.join(client.current_folder, "A"))
+        client.save(files("B", "D"), path=os.path.join(client.current_folder, "B"))
+
+        project = dedent("""
+            editables:
+                HelloD/0.1@lasote/stable:
+                    path: D
+                HelloB/0.1@lasote/stable:
+                    path: B
+                HelloC/0.1@lasote/stable:
+                    path: C
+                HelloA/0.1@lasote/stable:
+                    path: A
+            layout: layout
+            root: HelloA/0.1@lasote/stable, HelloB/0.1@lasote/stable
+            """)
+        layout = dedent("""
+            [build_folder]
+
+            """)
+        client.save({"conanws.yml": project,
+                     "layout": layout})
+        client.run("workspace install conanws.yml")
+        self.assertIn("HelloA/0.1@lasote/stable from user folder - Editable", client.out)
+        self.assertIn("HelloB/0.1@lasote/stable from user folder - Editable", client.out)
+        self.assertIn("HelloC/0.1@lasote/stable from user folder - Editable", client.out)
+        self.assertIn("HelloD/0.1@lasote/stable from user folder - Editable", client.out)
+
+        a_cmake = load(os.path.join(client.current_folder, "A", "conanbuildinfo.cmake"))
+        self.assertIn("set(CONAN_LIBS helloC helloD ${CONAN_LIBS})", a_cmake)
+        b_cmake = load(os.path.join(client.current_folder, "B", "conanbuildinfo.cmake"))
+        self.assertIn("set(CONAN_LIBS helloD ${CONAN_LIBS})", b_cmake)
+
+    def transitivity_test(self):
+        # https://github.com/conan-io/conan/issues/4720
+        client = TestClient()
+
+        def files(name, depend=None):
+            if isinstance(depend, list):
+                deps = ", ".join(["'Hello%s/0.1@lasote/stable'" % d for d in depend])
+            else:
+                deps = ('"Hello%s/0.1@lasote/stable"' % depend) if depend else "None"
+            return {"conanfile.py": conanfile_build.format(deps=deps, name=name)}
+
+        client.save(files("D"), path=os.path.join(client.current_folder, "D"))
+        client.save(files("C", "D"), path=os.path.join(client.current_folder, "C"))
+        client.save(files("B", "C"), path=os.path.join(client.current_folder, "B"))
+
+        client.save(files("A", ["D", "C", "B"]), path=os.path.join(client.current_folder, "A"))
+
+        project = dedent("""
+            editables:
+                HelloD/0.1@lasote/stable:
+                    path: D
+                HelloB/0.1@lasote/stable:
+                    path: B
+                HelloC/0.1@lasote/stable:
+                    path: C
+                HelloA/0.1@lasote/stable:
+                    path: A
+            layout: layout
+            root: HelloA/0.1@lasote/stable
+            """)
+        layout = dedent("""
+            [build_folder]
+
+            """)
+        client.save({"conanws.yml": project,
+                     "layout": layout})
+        client.run("workspace install conanws.yml")
+        self.assertIn("HelloA/0.1@lasote/stable from user folder - Editable", client.out)
+        self.assertIn("HelloB/0.1@lasote/stable from user folder - Editable", client.out)
+        self.assertIn("HelloC/0.1@lasote/stable from user folder - Editable", client.out)
+        self.assertIn("HelloD/0.1@lasote/stable from user folder - Editable", client.out)
+
+        a_cmake = load(os.path.join(client.current_folder, "A", "conanbuildinfo.cmake"))
+        self.assertIn("set(CONAN_LIBS helloB helloC helloD ${CONAN_LIBS})", a_cmake)
+        b_cmake = load(os.path.join(client.current_folder, "B", "conanbuildinfo.cmake"))
+        self.assertIn("set(CONAN_LIBS helloC helloD ${CONAN_LIBS})", b_cmake)
+
     def simple_build_test(self):
         client = TestClient()