[bitbake-devel] [PATCH] gitsm: Check all submodules for updates

[William A. Kennington III]

Currently, our gitsm fetcher is inheriting the need_update function from
the git fetcher. This results in our do_fetch checking for updates only
if the SRCREV is missing from the top-level project.

Unfortunately, this doesn't take into account the dependent SRCREVs
missing from submodules due to a lack of the submodule existing
during the first checkout, or arbitrary download cache purges of
submodule data.

The most trivial way to encounter this is to have gitsm fetch a software
project before and after adding submodules. When it goes to use the
update that has submodules it will think that it has all of the needed
revisions since it checked the top-level repo. However none of those
submodules will exist in the downloads cache since they weren't included
in the original commit.

Tested:
    Ran the unit test suite with just the new unit test to make sure it
    fails repeatedly. Then applied the fix and ran it some more to
    ensure the fix works.

Signed-off-by: William A. Kennington III <wak at google.com>
---
 lib/bb/fetch2/gitsm.py | 24 ++++++++++++++++++++++++
 lib/bb/tests/fetch.py  | 19 +++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/lib/bb/fetch2/gitsm.py b/lib/bb/fetch2/gitsm.py
index b21fed26..f1ecd170 100644
--- a/lib/bb/fetch2/gitsm.py
+++ b/lib/bb/fetch2/gitsm.py
@@ -36,6 +36,7 @@ from   bb.fetch2.git import Git
 from   bb.fetch2 import runfetchcmd
 from   bb.fetch2 import logger
 from   bb.fetch2 import Fetch
+from   bb.fetch2 import FetchData
 from   bb.fetch2 import BBFetchException
 
 class GitSM(Git):
@@ -147,6 +148,29 @@ class GitSM(Git):
 
         return submodules != []
 
+    def need_update(self, ud, d):
+        if Git.need_update(self, ud, d):
+            return True
+        class NeedUpdateException(Exception):
+            pass
+        def need_update_submodule(ud, url, module, modpath, d):
+            url += ";bareclone=1;nobranch=1"
+            lud = FetchData(url, d, False)
+            lud.setup_localpath(d)
+            if lud.lockfile:
+                lf = bb.utils.lockfile(lud.lockfile)
+            try:
+                if lud.method.need_update(lud, d):
+                    raise NeedUpdateException
+            finally:
+                if lud.lockfile:
+                    bb.utils.unlockfile(lf)
+        try:
+            self.process_submodules(ud, ud.clonedir, need_update_submodule, d)
+            return False
+        except NeedUpdateException:
+            return True
+
     def download(self, ud, d):
         def download_submodule(ud, url, module, modpath, d):
             url += ";bareclone=1;nobranch=1"
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 25732627..429998b3 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -942,6 +942,25 @@ class FetcherNetworkTest(FetcherTest):
         self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/extern/json/config')), msg='Missing submodule config "extern/json"')
         self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/extern/sanitizers/config')), msg='Missing submodule config "extern/sanitizers"')
 
+    def test_git_submodule_update_CLI11(self):
+        """ Prevent regression on update detection not finding missing submodule, or modules without needed commits """
+        url = "gitsm://github.com/CLIUtils/CLI11;protocol=git;rev=cf6a99fa69aaefe477cc52e3ef4a7d2d7fa40714"
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+
+        # CLI11 that pulls in a newer nlohmann-json
+        url = "gitsm://github.com/CLIUtils/CLI11;protocol=git;rev=49ac989a9527ee9bb496de9ded7b4872c2e0e5ca"
+        fetcher = bb.fetch.Fetch([url], self.d)
+        fetcher.download()
+        # Previous cwd has been deleted
+        os.chdir(os.path.dirname(self.unpackdir))
+        fetcher.unpack(self.unpackdir)
+
+        repo_path = os.path.join(self.tempdir, 'unpacked', 'git')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/extern/googletest/config')), msg='Missing submodule config "extern/googletest"')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/extern/json/config')), msg='Missing submodule config "extern/json"')
+        self.assertTrue(os.path.exists(os.path.join(repo_path, '.git/modules/extern/sanitizers/config')), msg='Missing submodule config "extern/sanitizers"')
+
     def test_git_submodule_aktualizr(self):
         url = "gitsm://github.com/advancedtelematic/aktualizr;branch=master;protocol=git;rev=d00d1a04cc2366d1a5f143b84b9f507f8bd32c44"
         fetcher = bb.fetch.Fetch([url], self.d)
-- 
2.21.0