fix recursive build requires

conans/client/graph/graph.py

@@ -33,13 +33,20 @@ def __init__(self, ref, conanfile, recipe=None):
         self.remote = None
         self.binary_remote = None
         self.build_require = False
+        self.private = False
         self.revision_pinned = False  # The revision has been specified by the user
         # all the public deps to which this node belongs
         self.public_deps = None  # {ref.name: Node}
         # all the public deps only in the closure of this node
         self.public_closure = None  # {ref.name: Node}
         self.ancestors = None  # set{ref.name}
 
+    def update_ancestors(self, ancestors):
+        # When a diamond is closed, it is necessary to update all upstream ancestors, recursively
+        self.ancestors.update(ancestors)
+        for n in self.neighbors():
+            n.update_ancestors(ancestors)
+
     @property
     def package_id(self):
         return self._package_id

conans/client/graph/graph_builder.py

@@ -129,7 +129,7 @@ def _load_deps(self,  dep_graph, node, down_reqs, down_ref, down_options,
 
     def _handle_require(self, name, node, require, dep_graph, check_updates, update,
                         remote_name, processed_profile, new_reqs, new_options):
-        if name in node.ancestors:
+        if name in node.ancestors or name == node.name:
             raise ConanException("Loop detected: '%s' requires '%s' which is an ancestor too"
                                  % (node.ref, require.ref))
 
@@ -142,6 +142,8 @@ def _handle_require(self, name, node, require, dep_graph, check_updates, update,
 
             new_node.public_closure = OrderedDict([(new_node.ref.name, new_node)])
             node.public_closure[name] = new_node
+            # New nodes will inherit the private property of its ancestor
+            new_node.private = require.private or node.private
             if require.private or require.build_require:
                 new_node.public_deps = node.public_closure
             else:
@@ -154,6 +156,7 @@ def _handle_require(self, name, node, require, dep_graph, check_updates, update,
                 for dep_node_name, dep_node in node.public_deps.items():
                     if dep_node is node:
                         continue
+
                     if dep_node_name in new_node.ancestors:
                         dep_node.public_closure[new_node.name] = new_node
 
@@ -177,16 +180,19 @@ def _handle_require(self, name, node, require, dep_graph, check_updates, update,
                                      "    To change it, override it in your base requirements"
                                      % (node.ref, require.ref, previous.ref))
 
-            previous.ancestors.add(node.name)
-            previous.ancestors.update(node.ancestors)
+            # Add current ancestors to the previous node
+            previous.update_ancestors(node.ancestors.union([node.name]))
             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:
+            for name, n in previous.public_closure.items():
+                if n.build_require or n.private:
                     continue
-                if dep_node_name in previous.ancestors:
-                    dep_node.public_closure.update(previous.public_closure)
+                node.public_closure[name] = n
+                for dep_node_name in node.ancestors:
+                    dep_node = node.public_deps.get(dep_node_name)
+                    if dep_node:
+                        dep_node.public_closure[name] = n
 
             # RECURSION!
             if self._recurse(previous.public_closure, new_reqs, new_options):

conans/client/graph/graph_manager.py

@@ -199,7 +199,7 @@ def _recurse_build_requires(self, graph, builder, binaries_analyzer, check_updat
                                     # (no conflicts)
                                     # but the dict key is not used at all
                                     package_build_requires[build_require.name] = build_require
-                                else:  # Profile one
+                                elif build_require.name != node.name:  # Profile one
                                     new_profile_build_requires.append(build_require)
 
             if package_build_requires:

conans/model/workspace.py

@@ -44,19 +44,24 @@ def generate(self, cwd, graph, output):
         if self._ws_generator == "cmake":
             cmake = ""
             add_subdirs = ""
+            # To avoid multiple additions (can happen for build_requires repeated nodes)
+            unique_refs = OrderedDict()
             for node in graph.ordered_iterate():
                 if node.recipe != RECIPE_EDITABLE:
                     continue
-                ref = node.ref
+                unique_refs[node.ref] = node
+            for ref, node in unique_refs.items():
                 ws_pkg = self._workspace_packages[ref]
                 layout = self._cache.package_layout(ref)
                 editable = layout.editable_cpp_info()
 
                 conanfile = node.conanfile
-                build = editable.folder(ref, EditableLayout.BUILD_FOLDER, conanfile.settings,
-                                        conanfile.options)
-                src = editable.folder(ref, EditableLayout.SOURCE_FOLDER, conanfile.settings,
-                                      conanfile.options)
+                src = build = None
+                if editable:
+                    build = editable.folder(ref, EditableLayout.BUILD_FOLDER, conanfile.settings,
+                                            conanfile.options)
+                    src = editable.folder(ref, EditableLayout.SOURCE_FOLDER, conanfile.settings,
+                                          conanfile.options)
                 if src is not None:
                     src = os.path.join(ws_pkg.root_folder, src).replace("\\", "/")
                     cmake += 'set(PACKAGE_%s_SRC "%s")\n' % (ref.name, src)

conans/test/functional/graph/graph_manager_base.py

@@ -52,7 +52,7 @@ def _cache_recipe(self, reference, test_conanfile, revision=None):
         manifest = FileTreeManifest.create(self.cache.export(ref))
         manifest.save(self.cache.export(ref))
 
-    def build_graph(self, content, profile_build_requires=None, ref=None):
+    def build_graph(self, content, profile_build_requires=None, ref=None, create_ref=None):
         path = temp_folder()
         path = os.path.join(path, "conanfile.py")
         save(path, str(content))
@@ -69,7 +69,7 @@ def build_graph(self, content, profile_build_requires=None, ref=None):
         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,
+        deps_graph, _ = self.manager.load_graph(path, create_ref, graph_info,
                                                 build_mode, check_updates, update,
                                                 remote_name, recorder)
         self.binary_installer.install(deps_graph, False, graph_info)
@@ -81,7 +81,11 @@ def _check_node(self, node, ref, deps, build_deps, dependents, closure, public_d
         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))
+
+        dependants = node.inverse_neighbors()
+        self.assertEqual(len(dependants), len(dependents))
+        for d in dependents:
+            self.assertIn(d, dependants)
 
         public_deps = {n.name: n for n in public_deps}
         self.assertEqual(node.public_deps, public_deps)

conans/test/functional/graph/graph_manager_test.py

@@ -128,10 +128,10 @@ def test_consecutive_diamonds(self):
         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(libc, "libc/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[libd], 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(libb, "libb/0.1@user/testing#123", deps=[liba], build_deps=[],
-                         dependents=[app], closure=[liba], public_deps=public_deps)
+                         dependents=[libd], 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)
 
@@ -224,7 +224,7 @@ def test_multiple_transitive(self):
         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])
+                         dependents=[liba, 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])
@@ -262,6 +262,14 @@ def test_loop(self):
                                      "requires 'libc/0.1@user/testing'"):
             self.build_graph(TestConanFile("app", "0.1", requires=[libc_ref]))
 
+    def test_self_loop(self):
+        ref1 = "base/1.0@user/testing"
+        self._cache_recipe(ref1, TestConanFile("base", "0.1"))
+        ref = ConanFileReference.loads("base/aaa@user/testing")
+        with self.assertRaisesRegexp(ConanException, "Loop detected: 'base/aaa@user/testing' "
+                                     "requires 'base/aaa@user/testing'"):
+            self.build_graph(TestConanFile("base", "aaa", requires=[ref1]), ref=ref, create_ref=ref)
+
     @parameterized.expand([("recipe", ), ("profile", )])
     def test_basic_build_require(self, build_require):
         # app -(br)-> tool0.1
@@ -345,7 +353,7 @@ def test_transitive_build_require_recipe_profile(self):
                          closure=[mingw_app, lib], public_deps=[app, lib])
 
         self._check_node(lib, "lib/0.1@user/testing#123", deps=[], build_deps=[mingw_lib, gtest],
-                         dependents=[lib], closure=[mingw_lib, gtest], public_deps=[app, lib])
+                         dependents=[app], closure=[mingw_lib, gtest], public_deps=[app, lib])
         self._check_node(gtest, "gtest/0.1@user/testing#123", deps=[], build_deps=[mingw_gtest],
                          dependents=[lib], closure=[mingw_gtest], public_deps=[gtest, lib])
         # MinGW leaf nodes
@@ -454,6 +462,149 @@ def configure(self):
                                      "tried to change zlib/0.1@user/testing option shared to True"):
             self.build_graph(TestConanFile("app", "0.1", requires=["lib/0.1@user/testing"]))
 
+    def test_consecutive_diamonds_build_requires(self):
+        # app -> libe0.1 -------------> libd0.1 -> libb0.1 -------------> liba0.1
+        #    \-(build-require)-> libf0.1 ->/    \-(build-require)->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],
+                                                   build_requires=[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],
+                                                    build_requires=[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, liba, libd, libe]
+        self._check_node(app, "app/0.1@None/None", deps=[libe], build_deps=[libf], dependents=[],
+                         closure=[libf, libe, libd, libb, liba], public_deps=public_deps)
+        self._check_node(libe, "libe/0.1@user/testing#123", deps=[libd], build_deps=[],
+                         dependents=[app], closure=[libd, libb, liba],
+                         public_deps=public_deps)
+
+        public_deps = [app, libb, liba, libd, libe, libf]
+        self._check_node(libf, "libf/0.1@user/testing#123", deps=[libd], build_deps=[],
+                         dependents=[app], closure=[libd, libb, liba],
+                         public_deps=public_deps)
+
+        public_deps = [app, libb, liba, libd, libe]
+        self._check_node(libd, "libd/0.1@user/testing#123", deps=[libb], build_deps=[libc],
+                         dependents=[libe, libf], closure=[libc, libb, liba],
+                         public_deps=public_deps)
+        public_deps = [libd, libb, libc, liba]
+        self._check_node(libc, "libc/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[libd], closure=[liba], public_deps=public_deps)
+        public_deps = [app, libb, liba, libd, libe]
+        self._check_node(libb, "libb/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[libd], 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_consecutive_diamonds_private(self):
+        # app -> libe0.1 -------------> libd0.1 -> libb0.1 -------------> liba0.1
+        #    \-(private-req)-> libf0.1 ->/    \-(private-req)->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],
+                                                   private_requires=[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],
+                                                    private_requires=[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
+
+        main_public_deps = [app, libb, liba, libd, libe]
+        self._check_node(app, "app/0.1@None/None", deps=[libe, libf], build_deps=[], dependents=[],
+                         closure=[libe, libf, libd, libb, liba], public_deps=main_public_deps)
+        self._check_node(libe, "libe/0.1@user/testing#123", deps=[libd], build_deps=[],
+                         dependents=[app], closure=[libd, libb, liba],
+                         public_deps=main_public_deps)
+
+        libf_public_deps = [libb, liba, libd, libe, libf]
+        self._check_node(libf, "libf/0.1@user/testing#123", deps=[libd], build_deps=[],
+                         dependents=[app], closure=[libd, libb, liba],
+                         public_deps=libf_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=main_public_deps)
+
+        libc_public_deps = [libb, libc, liba]
+        self._check_node(libc, "libc/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[libd], closure=[liba], public_deps=libc_public_deps)
+
+        self._check_node(libb, "libb/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[libd], closure=[liba], public_deps=main_public_deps)
+        self._check_node(liba, "liba/0.1@user/testing#123", deps=[], build_deps=[],
+                         dependents=[libb, libc], closure=[], public_deps=main_public_deps)
+
+    def test_parallel_diamond_build_requires(self):
+        # app --------> libb0.1 ---------> liba0.1
+        #    \--------> libc0.1 ----------->/
+        #    \-(build_require)-> 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],
+                                                    build_requires=[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]
+        self._check_node(app, "app/0.1@None/None", deps=[libb, libc], build_deps=[libd],
+                         dependents=[], closure=[libd, 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(libc, "libc/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app], closure=[liba], public_deps=public_deps)
+        public_deps = [app, libd, libb, libc, liba]
+        self._check_node(libd, "libd/0.1@user/testing#123", deps=[liba], build_deps=[],
+                         dependents=[app], closure=[liba], public_deps=public_deps)
+        public_deps = [app, libb, libc, liba]
+        self._check_node(liba, "liba/0.1@user/testing#123", deps=[], build_deps=[],
+                         dependents=[libb, libc, libd], closure=[], public_deps=public_deps)
+
     def test_conflict_private(self):
         liba_ref = "liba/0.1@user/testing"
         liba_ref2 = "liba/0.2@user/testing"