From 3a7480576fc7c868046baa5a5f3d915b9fa87dac Mon Sep 17 00:00:00 2001
From: Wez Furlong <wez@fb.com>
Date: Mon, 4 May 2020 17:42:38 -0700
Subject: [PATCH] getdeps: improve build invalidation for dependencies

Summary:
We didn't do a great job of recognizing that we'd need to
build a project when one of its dependencies had changed: we relied
chiefly on the dependency hash for this and could fail to handle
changes in individual source files.

This commit helps to improve this situation by checking to see if
any installed files in the dependencies of a manifest are newer than
the most recent built time of a given manifest.  If so, we'll trigger
a build.  We try to be reasonably smart about deciding when to trigger
a cmake reconfigure if it looks like cmake files in the deps have
been changed.

Reviewed By: xavierd

Differential Revision: D21364132

fbshipit-source-id: 7534496e10d1f532aa9cf865900ace84a8785327
---
 build/fbcode_builder/getdeps.py         | 53 ++++++++++++++++++++++++-
 build/fbcode_builder/getdeps/fetcher.py |  9 +++++
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/build/fbcode_builder/getdeps.py b/build/fbcode_builder/getdeps.py
index 7f8b785d1..a130a4ffb 100755
--- a/build/fbcode_builder/getdeps.py
+++ b/build/fbcode_builder/getdeps.py
@@ -21,7 +21,11 @@ import getdeps.cache as cache_module
 from getdeps.buildopts import setup_build_options
 from getdeps.dyndeps import create_dyn_dep_munger
 from getdeps.errors import TransientFailure
-from getdeps.fetcher import SystemPackageFetcher
+from getdeps.fetcher import (
+    SystemPackageFetcher,
+    file_name_is_cmake_file,
+    list_files_under_dir_newer_than_timestamp,
+)
 from getdeps.load import ManifestLoader
 from getdeps.manifest import ManifestParser
 from getdeps.platform import HostType
@@ -467,6 +471,17 @@ class BuildCmd(ProjectCmdBase):
                     cached_project, fetcher, m, built_marker, project_hash
                 )
 
+                if os.path.exists(built_marker):
+                    # We've previously built this. We may need to reconfigure if
+                    # our deps have changed, so let's check them.
+                    dep_reconfigure, dep_build = self.compute_dep_change_status(
+                        m, built_marker, loader
+                    )
+                    if dep_reconfigure:
+                        reconfigure = True
+                    if dep_build:
+                        sources_changed = True
+
                 if sources_changed or reconfigure or not os.path.exists(built_marker):
                     if os.path.exists(built_marker):
                         os.unlink(built_marker)
@@ -491,6 +506,42 @@ class BuildCmd(ProjectCmdBase):
 
             install_dirs.append(inst_dir)
 
+    def compute_dep_change_status(self, m, built_marker, loader):
+        reconfigure = False
+        sources_changed = False
+        st = os.lstat(built_marker)
+
+        ctx = loader.ctx_gen.get_context(m.name)
+        dep_list = sorted(m.get_section_as_dict("dependencies", ctx).keys())
+        for dep in dep_list:
+            if reconfigure and sources_changed:
+                break
+
+            dep_manifest = loader.load_manifest(dep)
+            dep_root = loader.get_project_install_dir(dep_manifest)
+            for dep_file in list_files_under_dir_newer_than_timestamp(
+                dep_root, st.st_mtime
+            ):
+                if os.path.basename(dep_file) == ".built-by-getdeps":
+                    continue
+                if file_name_is_cmake_file(dep_file):
+                    if not reconfigure:
+                        reconfigure = True
+                        print(
+                            f"Will reconfigure cmake because {dep_file} is newer than {built_marker}"
+                        )
+                else:
+                    if not sources_changed:
+                        sources_changed = True
+                        print(
+                            f"Will run build because {dep_file} is newer than {built_marker}"
+                        )
+
+                if reconfigure and sources_changed:
+                    break
+
+        return reconfigure, sources_changed
+
     def compute_source_change_status(
         self, cached_project, fetcher, m, built_marker, project_hash
     ):
diff --git a/build/fbcode_builder/getdeps/fetcher.py b/build/fbcode_builder/getdeps/fetcher.py
index 13ce32719..f6e0f7c7c 100644
--- a/build/fbcode_builder/getdeps/fetcher.py
+++ b/build/fbcode_builder/getdeps/fetcher.py
@@ -363,6 +363,15 @@ def copy_if_different(src_name, dest_name):
     return True
 
 
+def list_files_under_dir_newer_than_timestamp(dir_to_scan, ts):
+    for root, _dirs, files in os.walk(dir_to_scan):
+        for src_file in files:
+            full_name = os.path.join(root, src_file)
+            st = os.lstat(full_name)
+            if st.st_mtime > ts:
+                yield full_name
+
+
 class ShipitPathMap(object):
     def __init__(self):
         self.roots = []