Use git ls-files -s to include gitlinks from staged patches.

Bug: 1471685
Change-Id: I3fdf9630ed0e55dc10a1c578c8e052f76461e800
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4776220
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>

gclient.py

@@ -884,18 +884,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     # If dependencies are configured within git submodules, add them to deps.
     if self.git_dependencies_state in (gclient_eval.SUBMODULES,
                                        gclient_eval.SYNC):
-      gitsubmodules = self.ParseGitSubmodules()
-      # TODO(crbug.com/1471685): Temporary hack. In case of applied patches
-      # where the changes are staged but not committed, any gitlinks from
-      # the patch are not returned in ParseGitSubmodules. In these cases,
-      # DEPS does have the commits from the patch, so we use those commits
-      # instead.
-      if self.git_dependencies_state == gclient_eval.SYNC:
-        for sub in gitsubmodules:
-          if sub not in deps:
-            deps[sub] = gitsubmodules[sub]
-      elif self.git_dependencies_state == gclient_eval.SUBMODULES:
-        deps.update(gitsubmodules)
+      deps.update(self.ParseGitSubmodules())
 
     deps_to_add = self._deps_to_objects(
         self._postprocess_deps(deps, rel_prefix), self._use_relative_paths)
@@ -952,18 +941,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
                       filepath)
       return {}
 
-    # Get submodule commit hashes
-    #  Output Format: `<mode> SP <type> SP <object> TAB <file>`.
-    result = subprocess2.check_output(['git', 'ls-tree', '-r', 'HEAD'],
-                                      cwd=cwd).decode('utf-8')
-
-    commit_hashes = {}
-    for r in result.splitlines():
-      # ['<mode>', '<type>', '<commit_hash>', '<path>'].
-      record = r.strip().split(maxsplit=3)  # path can contain spaces.
-      if record[0] == '160000':  # Only add gitlinks
-        commit_hashes[record[3]] = record[2]
-
     # Get .gitmodules fields
     gitmodules_entries = subprocess2.check_output(
         ['git', 'config', '--file', filepath, '-l']).decode('utf-8')
@@ -988,6 +965,19 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       if sub_key in ('url', 'gclient-condition', 'path'):
         gitmodules[submodule][sub_key] = value
 
+    paths = [module['path'] for module in gitmodules.values()]
+    # Get submodule commit hashes. We use `gitt ls-files -s` to ensure
+    # we capture gitlink changes from staged applied patches.
+    # Output Format: `<mode> SP <type> SP <object> TAB <file>`.
+    result = subprocess2.check_output(['git', 'ls-files', '-s'] + paths,
+                                      cwd=cwd).decode('utf-8')
+    commit_hashes = {}
+    for r in result.splitlines():
+      # ['<mode>', '<commit_hash>', '<stage_number>', '<path>'].
+      record = r.strip().split(maxsplit=3)  # path can contain spaces.
+      assert record[0] == '160000', 'file is not a gitlink: %s' % record
+      commit_hashes[record[3]] = record[1]
+
     # Structure git submodules into a dict of DEPS git url entries.
     submodules = {}
     for module in gitmodules.values():

tests/gclient_test.py

@@ -1432,12 +1432,11 @@ class GclientTest(trial_dir.TestCase):
     }]
     write('.gclient', 'solutions = %s' % repr(solutions))
 
-    ls_tree = """160000 commit be8c5114d606692dc783b60cf256690b62fbad17\tfoo/bar
-    100644 blob daf2de9caad4a70e6bb1047a6b50c412066f68b1\tREADME.txt
-    160000 commit 3ad3b564f8ae456f286446d091709f5a09fa4a93\taaaaaa
-    160000 commit 956df937508b65b5e72a4cf02696255be3631b78\ta.a.a/a
-    160000 commit b9f77763f0fab67eeeb6371492166567a8b7a3d2\ta_b/c
-    160000 commit b9f77763f0fab67eeeb6371492166567a8b7a3d2\ta b/c"""
+    ls_files = """160000 be8c5114d606692dc783b60cf256690b62fbad17 0\tfoo/bar
+    160000 3ad3b564f8ae456f286446d091709f5a09fa4a93 0\taaaaaa
+    160000 956df937508b65b5e72a4cf02696255be3631b78 0\ta.a.a/a
+    160000 b9f77763f0fab67eeeb6371492166567a8b7a3d2 0\ta_b/c
+    160000 b9f77763f0fab67eeeb6371492166567a8b7a3d2 0\ta b/c"""
 
     git_config = """submodule.foo/bar.path=foo/bar
     submodule.foo/bar.url=http://example.com/foo/bar
@@ -1453,7 +1452,8 @@ class GclientTest(trial_dir.TestCase):
 
     os_path_isfile_mock = mock.MagicMock(return_value=True)
     subprocess2_check_output_mock = mock.MagicMock(
-        side_effect=[ls_tree.encode(), git_config.encode()])
+        side_effect=[git_config.encode(),
+                     ls_files.encode()])
 
     options, _ = gclient.OptionParser().parse_args([])
     client = gclient.GClient.LoadCurrentConfig(options)
@@ -1498,6 +1498,14 @@ class GclientTest(trial_dir.TestCase):
                           'b9f77763f0fab67eeeb6371492166567a8b7a3d2'),
               }
           })
+      subprocess2_check_output_mock.assert_has_calls([
+          mock.call(['git', 'config', '--file', mock.ANY, '-l']),
+          mock.call([
+              'git', 'ls-files', '-s', 'foo/bar', 'aaaaaa', 'a.a.a/a', 'a_b/c',
+              'a b/c'
+          ],
+                    cwd=mock.ANY)
+      ])
 
   def testParseGitSubmodules_UsesAbsolutePath(self):
     """ParseGitSubmodules uses absolute path when use_relative_path is not
@@ -1509,12 +1517,11 @@ class GclientTest(trial_dir.TestCase):
     }]
     write('.gclient', 'solutions = %s' % repr(solutions))
 
-    ls_tree = """160000 commit be8c5114d606692dc783b60cf256690b62fbad17\tfoo/bar
-    100644 blob daf2de9caad4a70e6bb1047a6b50c412066f68b1\tREADME.txt
-    160000 commit 3ad3b564f8ae456f286446d091709f5a09fa4a93\taaaaaa
-    160000 commit 956df937508b65b5e72a4cf02696255be3631b78\ta.a.a/a
-    160000 commit b9f77763f0fab67eeeb6371492166567a8b7a3d2\ta_b/c
-    160000 commit b9f77763f0fab67eeeb6371492166567a8b7a3d2\ta b/c"""
+    ls_files = """160000 be8c5114d606692dc783b60cf256690b62fbad17 0\tfoo/bar
+    160000 3ad3b564f8ae456f286446d091709f5a09fa4a93 0\taaaaaa
+    160000 956df937508b65b5e72a4cf02696255be3631b78 0\ta.a.a/a
+    160000 b9f77763f0fab67eeeb6371492166567a8b7a3d2 0\ta_b/c
+    160000 b9f77763f0fab67eeeb6371492166567a8b7a3d2 0\ta b/c"""
 
     git_config = """submodule.foo/bar.path=foo/bar
     submodule.foo/bar.url=http://example.com/foo/bar
@@ -1530,7 +1537,8 @@ class GclientTest(trial_dir.TestCase):
 
     os_path_isfile_mock = mock.MagicMock(return_value=True)
     subprocess2_check_output_mock = mock.MagicMock(
-        side_effect=[ls_tree.encode(), git_config.encode()])
+        side_effect=[git_config.encode(),
+                     ls_files.encode()])
 
     options, _ = gclient.OptionParser().parse_args([])
     client = gclient.GClient.LoadCurrentConfig(options)
@@ -1574,6 +1582,14 @@ class GclientTest(trial_dir.TestCase):
                           'b9f77763f0fab67eeeb6371492166567a8b7a3d2'),
               }
           })
+      subprocess2_check_output_mock.assert_has_calls([
+          mock.call(['git', 'config', '--file', mock.ANY, '-l']),
+          mock.call([
+              'git', 'ls-files', '-s', 'foo/bar', 'aaaaaa', 'a.a.a/a', 'a_b/c',
+              'a b/c'
+          ],
+                    cwd=mock.ANY)
+      ])
 
   def testSameDirAllowMultipleCipdDeps(self):
     """Verifies gclient allow multiple cipd deps under same directory."""