Update CMDgitmodules to set `gclient-recursedeps` in .gitmodules file.

Note there's not presubmit checking that this stays in sync, but I think recursdeps is not updated often enough that it's a high priority. We will also need to run `gclient gitmodules` for all repos we care about to update the .gitmodules file after this lands.


Bug: b/368607402, b/371352079
Change-Id: Iac90f0f45a30e2c1fdccf6053f07ec34c4139df1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5903942
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Joanna Wang <jojwang@chromium.org>

gclient.py

@@ -3292,6 +3292,9 @@ def CMDgitmodules(parser, args):
     This command should be run in the root directory of the repo.
     It will create or update the .gitmodules file and include
     `gclient-condition` values. Commits in gitlinks will also be updated.
+    If you are running this command to set `gclient-recursedeps` for the first
+    time, you will need to delete the .gitmodules file (if any) before running
+    this command.
     """
     if gclient_utils.IsEnvCog():
         raise gclient_utils.Error(
@@ -3337,11 +3340,24 @@ def CMDgitmodules(parser, args):
     # some old branch which contains already contains submodules with .git.
     # This check makes the transition easier.
     strip_git_suffix = True
+    # Users may have an outdated depot_tools which doesn't set
+    # gclient-recursedeps, which may undo changes made by an up-to-date `gclient
+    # gitmodules` run. Users may also have an up-to-date depot_tools but an
+    # outdated chromium/src without gclient-recursedeps set which would cause
+    # confusion when they run `gclient gitmodules` and see unexpected
+    # gclient-recursedeps in the diff. We only want to set gclient-recursedeps
+    # if it's already set in the .gitmodules file or if there is no existing
+    # .gitmodules file so we know `gclient gitmodules` is being run for the
+    # first time.
+    set_recursedeps = True
     if os.path.exists(options.output_gitmodules):
         dot_git_pattern = re.compile('^(\s*)url(\s*)=.*\.git$')
         with open(options.output_gitmodules) as f:
             strip_git_suffix = not any(dot_git_pattern.match(l) for l in f)
+            set_recursedeps = any(
+                'gclient-recursedeps' in l for l in f)
 
+    recursedeps = ls.get('recursedeps')
     with open(options.output_gitmodules, 'w', newline='') as f:
         for path, dep in ls.get('deps').items():
             if path in options.skip_dep:
@@ -3354,6 +3370,7 @@ def CMDgitmodules(parser, args):
                 logging.error('error on %s; %s, not adding it', path,
                               dep["url"])
                 continue
+            isRecurseDeps = recursedeps and path in recursedeps
             if prefix_length:
                 path = path[prefix_length:]
 
@@ -3366,6 +3383,8 @@ def CMDgitmodules(parser, args):
             f.write(f'[submodule "{path}"]\n\tpath = {path}\n\turl = {url}\n')
             if 'condition' in dep:
                 f.write(f'\tgclient-condition = {dep["condition"]}\n')
+            if isRecurseDeps and set_recursedeps:
+                f.write('\tgclient-recursedeps = true\n')
             # Windows has limit how long, so let's chunk those calls.
             if len(cache_info) >= 100:
                 subprocess2.call(['git', 'update-index', '--add'] + cache_info)

testing_support/fake_repos.py

@@ -230,7 +230,7 @@ class FakeReposBase(object):
 
 class FakeRepos(FakeReposBase):
     """Implements populateGit()."""
-    NB_GIT_REPOS = 23
+    NB_GIT_REPOS = 24
 
     def populateGit(self):
         # Testing:
@@ -839,7 +839,11 @@ deps = {
      }],
      "dep_type": "cipd",
   },
-}""" % {
+}
+
+recursedeps = [
+  'chicken/dickens',
+]""" % {
                     'hash_2': self.git_hashes['repo_2'][1][0],
                     'hash_3': self.git_hashes['repo_3'][1][0],
                 },
@@ -872,13 +876,18 @@ deps = {
      }],
      "dep_type": "cipd",
   },
-}""" % {
+}
+
+recursedeps = [
+  'foo/chicken/dickens',
+]
+""" % {
                     'hash_2': self.git_hashes['repo_2'][1][0],
                     'hash_3': self.git_hashes['repo_3'][1][0],
                 },
             })
 
-        # gitmodules already present, test migration
+        # gitmodules already present, test migration, .git suffix
         self._commit_git(
             'repo_21',
             {
@@ -992,6 +1001,33 @@ deps = {
                 'origin': 'git/repo_23@3\n'
             })
 
+        # gitmodules already present, test migration, gclient-recursedeps
+        self._commit_git(
+            'repo_24',
+            {
+                'DEPS':
+                """
+use_relative_paths = True
+git_dependencies = "SYNC"
+deps = {
+  "bar": {
+    "url": 'https://example.googlesource.com/repo@%(hash)s',
+  },
+}
+
+recursedeps = [
+  'bar',
+]""" % {
+                    'hash': self.git_hashes['repo_2'][1][0],
+                },
+                '.gitmodules':
+                """
+[submodule "bar"]
+	path = bar
+	url = https://example.googlesource.com/repo"""
+            },
+        )  # Update `NB_GIT_REPOS` if you add more repos.
+
 
 class FakeRepoSkiaDEPS(FakeReposBase):
     """Simulates the Skia DEPS transition in Chrome."""

tests/gclient_git_smoketest.py

@@ -51,7 +51,7 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
                 '[submodule "some_repo"]', '\tpath = some_repo',
                 '\turl = /repo_2', '\tgclient-condition = not foo_checkout',
                 '[submodule "chicken/dickens"]', '\tpath = chicken/dickens',
-                '\turl = /repo_3'
+                '\turl = /repo_3', '\tgclient-recursedeps = true'
             ], contents)
 
     def testGitmodules_not_relative(self):
@@ -69,7 +69,7 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
                 '[submodule "some_repo"]', '\tpath = some_repo',
                 '\turl = /repo_2', '\tgclient-condition = not foo_checkout',
                 '[submodule "chicken/dickens"]', '\tpath = chicken/dickens',
-                '\turl = /repo_3'
+                '\turl = /repo_3', '\tgclient-recursedeps = true'
             ], contents)
 
     def testGitmodules_migration_no_git_suffix(self):
@@ -99,6 +99,34 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
                 '\turl = https://example.googlesource.com/repo',
             ], contents)
 
+    def testGitmodules_migration_recursdeps(self):
+        self.gclient(['config', self.git_base + 'repo_24', '--name', 'foo'],
+                     cwd=self.git_base + 'repo_24')
+        # We are not running gclient sync since dependencies don't exist
+        self.gclient(['gitmodules'], cwd=self.git_base + 'repo_24')
+
+        gitmodules = os.path.join(self.git_base, 'repo_24', '.gitmodules')
+        with open(gitmodules) as f:
+            contents = f.read().splitlines()
+            self.assertEqual([
+                '[submodule "bar"]',
+                '\tpath = bar',
+                '\turl = https://example.googlesource.com/repo',
+            ], contents)
+        # force migration
+        os.remove(gitmodules)
+        self.gclient(['gitmodules'], cwd=self.git_base + 'repo_24')
+
+        gitmodules = os.path.join(self.git_base, 'repo_24', '.gitmodules')
+        with open(gitmodules) as f:
+            contents = f.read().splitlines()
+            self.assertEqual([
+                '[submodule "bar"]',
+                '\tpath = bar',
+                '\turl = https://example.googlesource.com/repo',
+                '\tgclient-recursedeps = true'
+            ], contents)
+
     def testGitmodules_not_in_gclient(self):
         with self.assertRaisesRegex(AssertionError, 'from a gclient workspace'):
             self.gclient(['gitmodules'], cwd=self.root_dir)