Remove .git suffix from git submodules

.git suffix is not well supported by the systems that utilize
submodules. Since some tools expect DEPS to have .git suffix, it's not
possible to simply update DEPS without breaking the tools.

This patch updates `gclient gitmodules` to omit .git suffix and trailing
slashes. However, if `.gitmodule` already exists, we are expecting the
tool to generate the same result if DEPS didn't change.

R=aravindvasudev@google.com

Bug: b/303296048
Change-Id: I30497b9d5f4e9681e16b4a913151de0f4a7c9545
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5008942
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>

gclient.py

@@ -2874,6 +2874,18 @@ def CMDgitmodules(parser, args):
             prefix_length = len(delta_path.replace(os.path.sep, '/')) + 1
 
     cache_info = []
+
+    # Git submodules shouldn't use .git suffix since it's not well supported.
+    # However, we can't update .gitmodules files since there is no guarantee
+    # that user has the latest version of depot_tools, and also they are not on
+    # some old branch which contains already contains submodules with .git.
+    # This check makes the transition easier.
+    strip_git_suffix = 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)
+
     with open(options.output_gitmodules, 'w', newline='') as f:
         for path, dep in ls.get('deps').items():
             if path in options.skip_dep:
@@ -2889,6 +2901,11 @@ def CMDgitmodules(parser, args):
             if prefix_length:
                 path = path[prefix_length:]
 
+            if strip_git_suffix:
+                if url.endswith('.git'):
+                    url = url[:-4]  # strip .git
+                url = url.rstrip('/')  # remove trailing slash for consistency
+
             cache_info += ['--cacheinfo', f'160000,{commit},{path}']
             f.write(f'[submodule "{path}"]\n\tpath = {path}\n\turl = {url}\n')
             if 'condition' in dep:

testing_support/fake_repos.py

@@ -162,14 +162,10 @@ class FakeReposBase(object):
         except (OSError, subprocess2.CalledProcessError):
             return False
         for repo in ['repo_%d' % r for r in range(1, self.NB_GIT_REPOS + 1)]:
-            # TODO(crbug.com/114712) use git.init -b and remove 'checkout' once
-            # git is upgraded to 2.28 on all builders.
-            subprocess2.check_call(
-                ['git', 'init', '-q',
-                 join(self.git_base, repo)])
-            subprocess2.check_call(
-                ['git', 'checkout', '-q', '-b', DEFAULT_BRANCH],
-                cwd=join(self.git_base, repo))
+            subprocess2.check_call([
+                'git', 'init', '-b', DEFAULT_BRANCH, '-q',
+                join(self.git_base, repo)
+            ])
             self.git_hashes[repo] = [(None, None)]
         self.populateGit()
         self.initialized = True
@@ -214,7 +210,7 @@ class FakeReposBase(object):
 
 class FakeRepos(FakeReposBase):
     """Implements populateGit()."""
-    NB_GIT_REPOS = 20
+    NB_GIT_REPOS = 21
 
     def populateGit(self):
         # Testing:
@@ -862,6 +858,29 @@ deps = {
                 },
             })
 
+        # gitmodules already present, test migration
+        self._commit_git(
+            'repo_21',
+            {
+                'DEPS':
+                """
+use_relative_paths = True
+git_dependencies = "SYNC"
+deps = {
+  "bar": {
+    "url": 'https://example.googlesource.com/repo.git@%(hash)s',
+  },
+}""" % {
+                    'hash': self.git_hashes['repo_2'][1][0],
+                },
+                '.gitmodules':
+                """
+[submodule "bar"]
+	path = bar
+	url = invalid/repo_url.git"""
+            },
+        )
+
 
 class FakeRepoSkiaDEPS(FakeReposBase):
     """Simulates the Skia DEPS transition in Chrome."""

tests/gclient_git_smoketest.py

@@ -70,6 +70,33 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
                 '\turl = /repo_3'
             ], contents)
 
+    def testGitmodules_migration_no_git_suffix(self):
+        self.gclient(['config', self.git_base + 'repo_21', '--name', 'foo'],
+                     cwd=self.git_base + 'repo_21')
+        # We are not running gclient sync since dependencies don't exist
+        self.gclient(['gitmodules'], cwd=self.git_base + 'repo_21')
+
+        gitmodules = os.path.join(self.git_base, 'repo_21', '.gitmodules')
+        with open(gitmodules) as f:
+            contents = f.read().splitlines()
+            self.assertEqual([
+                '[submodule "bar"]',
+                '\tpath = bar',
+                '\turl = https://example.googlesource.com/repo.git',
+            ], contents)
+        # force migration
+        os.remove(gitmodules)
+        self.gclient(['gitmodules'], cwd=self.git_base + 'repo_21')
+
+        gitmodules = os.path.join(self.git_base, 'repo_21', '.gitmodules')
+        with open(gitmodules) as f:
+            contents = f.read().splitlines()
+            self.assertEqual([
+                '[submodule "bar"]',
+                '\tpath = bar',
+                '\turl = https://example.googlesource.com/repo',
+            ], contents)
+
     def testGitmodules_not_in_gclient(self):
         with self.assertRaisesRegex(AssertionError, 'from a gclient workspace'):
             self.gclient(['gitmodules'], cwd=self.root_dir)