extract get_dependencies method

Summary:
A number of places were extracting dependencies from manifests, but only one was adding in the implicit dependencies for build tools.

Extract the logic to one place and use so that a change in a tool like cmake will now correctly affect all tools using cmake, as it will be taken into account as a dependency hash when the manifest's hash is computed.

Tests for this change revealed that install_dirs needed to be populated in reverse order from the manifest topo-sort, so have also addressed that

Reviewed By: wittgenst

Differential Revision: D32730717

fbshipit-source-id: 1b2a25e460de6085d274c99acfd391b3bd259264

build/fbcode_builder/getdeps.py

@@ -581,7 +581,10 @@ class BuildCmd(ProjectCmdBase):
                 elif args.verbose:
                     print("found good %s" % built_marker)
 
-            install_dirs.append(inst_dir)
+            # Paths are resolved from front. We prepend rather than append as
+            # the last project in topo order is the project itself, which
+            # should be first in the path, then its deps and so on.
+            install_dirs.insert(0, inst_dir)
 
     def compute_dep_change_status(self, m, built_marker, loader):
         reconfigure = False
@@ -589,7 +592,7 @@ class BuildCmd(ProjectCmdBase):
         st = os.lstat(built_marker)
 
         ctx = loader.ctx_gen.get_context(m.name)
-        dep_list = sorted(m.get_section_as_dict("dependencies", ctx).keys())
+        dep_list = m.get_dependencies(ctx)
         for dep in dep_list:
             if reconfigure and sources_changed:
                 break

build/fbcode_builder/getdeps/builder.py

@@ -1359,12 +1359,12 @@ incremental = false
         is also cargo-builded and if yes then extract it's git configs and
         install dir
         """
-        dependencies = self.manifest.get_section_as_dict("dependencies", ctx=self.ctx)
+        dependencies = self.manifest.get_dependencies(self.ctx)
         if not dependencies:
             return []
 
         dep_to_git = {}
-        for dep in dependencies.keys():
+        for dep in dependencies:
             dep_manifest = self.loader.load_manifest(dep)
             dep_builder = dep_manifest.get("build", "builder", ctx=self.ctx)
             if dep_builder not in ["cargo", "nop"] or dep == "rust":
@@ -1374,7 +1374,7 @@ incremental = false
                 # toolchain.
                 continue
 
-            git_conf = dep_manifest.get_section_as_dict("git", ctx=self.ctx)
+            git_conf = dep_manifest.get_section_as_dict("git", self.ctx)
             if "repo_url" not in git_conf:
                 raise Exception(
                     "A cargo dependency requires git.repo_url to be defined."

build/fbcode_builder/getdeps/load.py

@@ -192,19 +192,7 @@ class ManifestLoader(object):
             # to produce the same order regardless of how they are listed
             # in the project manifest files.
             ctx = self.ctx_gen.get_context(m.name)
-            dep_list = sorted(m.get_section_as_dict("dependencies", ctx).keys())
+            dep_list = m.get_dependencies(ctx)
-            builder = m.get("build", "builder", ctx=ctx)
-            if builder in ("cmake", "python-wheel"):
-                dep_list.append("cmake")
-            elif builder == "autoconf" and m.name not in (
-                "autoconf",
-                "libtool",
-                "automake",
-            ):
-                # they need libtool and its deps (automake, autoconf) so add
-                # those as deps (but obviously not if we're building those
-                # projects themselves)
-                dep_list.append("libtool")
 
             dep_count = 0
             for dep_name in dep_list:
@@ -303,7 +291,7 @@ class ManifestLoader(object):
 
         manifest.update_hash(hasher, ctx)
 
-        dep_list = sorted(manifest.get_section_as_dict("dependencies", ctx).keys())
+        dep_list = manifest.get_dependencies(ctx)
         for dep in dep_list:
             dep_manifest = self.load_manifest(dep)
             dep_hash = self.get_project_hash(dep_manifest)

build/fbcode_builder/getdeps/manifest.py

@@ -246,6 +246,24 @@ class ManifestParser(object):
 
         return defval
 
+    def get_dependencies(self, ctx):
+        dep_list = list(self.get_section_as_dict("dependencies", ctx).keys())
+        dep_list.sort()
+        builder = self.get("build", "builder", ctx=ctx)
+        if builder in ("cmake", "python-wheel"):
+            dep_list.insert(0, "cmake")
+        elif builder == "autoconf" and self.name not in (
+            "autoconf",
+            "libtool",
+            "automake",
+        ):
+            # they need libtool and its deps (automake, autoconf) so add
+            # those as deps (but obviously not if we're building those
+            # projects themselves)
+            dep_list.insert(0, "libtool")
+
+        return dep_list
+
     def get_section_as_args(self, section, ctx=None):
         """Intended for use with the make.[build_args/install_args] and
         autoconf.args sections, this method collects the entries and returns an
@@ -290,9 +308,8 @@ class ManifestParser(object):
                 res.append((key, value))
         return res
 
-    def get_section_as_dict(self, section, ctx=None):
+    def get_section_as_dict(self, section, ctx):
         d = {}
-        ctx = ctx or {}
 
         for s in self._config.sections():
             if s != section:

build/fbcode_builder/getdeps/test/manifest_test.py

@@ -179,7 +179,7 @@ foo = bar
 foo = baz
 """,
         )
-        self.assertEqual(p.get_section_as_dict("cmake.defines"), {"foo": "bar"})
+        self.assertEqual(p.get_section_as_dict("cmake.defines", {}), {"foo": "bar"})
         self.assertEqual(
             p.get_section_as_dict("cmake.defines", {"test": "on"}), {"foo": "baz"}
         )

build/fbcode_builder/manifests/fizz

@@ -30,7 +30,7 @@ zlib
 zstd
 
 [dependencies.all(test=on, not(os=windows))]
-googletest_1_8
+googletest
 
 [shipit.pathmap]
 fbcode/fizz/public_tld = .