From 840318dd2246aa56380bbb0e44730ee777e6398f Mon Sep 17 00:00:00 2001
From: Georgy Dyuldin <gdyuldin@users.noreply.github.com>
Date: Tue, 16 Jul 2019 16:38:42 +0300
Subject: [PATCH] git: Fix submodule names issue (#2274)

GitPython creates submodules nodes during tree iterate without explicit
passing name attribute. This path replaces such submodules with new
instances with filled name attribute.

Fixes: #1898
---
 dvc/scm/git/tree.py     | 19 +++++++++++++++++--
 tests/func/test_tree.py | 24 +++++++++++++++++-------
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/dvc/scm/git/tree.py b/dvc/scm/git/tree.py
index 5716984d2..1493f4030 100644
--- a/dvc/scm/git/tree.py
+++ b/dvc/scm/git/tree.py
@@ -13,6 +13,21 @@ GIT_MODE_DIR = 0o40000
 GIT_MODE_FILE = 0o644
 
 
+def _iter_tree(tree):
+    # During usual tree iteration with ``for .. in ..`` GitPython returns each
+    # submodule instance without correctly filled ``name`` property. It raises
+    # an exception during accessing such submodule ``name`` later.
+    # The same time repo's submodules property contains correctly initialized
+    # submodules list.
+    # Here we just replace submodule instance from iterator with the
+    # corresponding instance from ``repo.submodules.``
+    submodules = {x.hexsha: x for x in tree.repo.submodules}
+    for node in tree:
+        if node.type == "submodule":
+            node = submodules[node.hexsha]
+        yield node
+
+
 class GitTree(BaseTree):
     """Proxies the repo file access methods to Git objects"""
 
@@ -72,7 +87,7 @@ class GitTree(BaseTree):
             return False
         # see https://github.com/gitpython-developers/GitPython/issues/851
         # `return (i in tree)` doesn't work so here is a workaround:
-        for i in obj:
+        for i in _iter_tree(obj):
             if i.name == path:
                 return True
         return False
@@ -104,7 +119,7 @@ class GitTree(BaseTree):
 
     def _walk(self, tree, topdown=True):
         dirs, nondirs = [], []
-        for i in tree:
+        for i in _iter_tree(tree):
             if i.mode == GIT_MODE_DIR:
                 dirs.append(i.name)
             else:
diff --git a/tests/func/test_tree.py b/tests/func/test_tree.py
index 7bec4ce71..9cc771480 100644
--- a/tests/func/test_tree.py
+++ b/tests/func/test_tree.py
@@ -9,7 +9,7 @@ from dvc.scm import SCM
 from dvc.scm.git import GitTree
 from dvc.scm.tree import WorkingTree
 
-from tests.basic_env import TestDir, TestGit
+from tests.basic_env import TestDir, TestGit, TestGitSubmodule
 
 
 class TestWorkingTree(TestDir):
@@ -39,12 +39,7 @@ class TestWorkingTree(TestDir):
         self.assertFalse(self.tree.isfile("not-existing-file"))
 
 
-class TestGitTree(TestGit):
-    def setUp(self):
-        super(TestGitTree, self).setUp()
-        self.scm = SCM(self._root_dir)
-        self.tree = GitTree(self.git, "master")
-
+class GitTreeTests(object):
     def test_open(self):
         self.scm.add([self.FOO, self.UNICODE, self.DATA_DIR])
         self.scm.commit("add")
@@ -83,6 +78,21 @@ class TestGitTree(TestGit):
         self.assertFalse(self.tree.isfile("not-existing-file"))
 
 
+class TestGitTree(TestGit, GitTreeTests):
+    def setUp(self):
+        super(TestGitTree, self).setUp()
+        self.scm = SCM(self._root_dir)
+        self.tree = GitTree(self.git, "master")
+
+
+class TestGitSubmoduleTree(TestGitSubmodule, GitTreeTests):
+    def setUp(self):
+        super(TestGitSubmoduleTree, self).setUp()
+        self.scm = SCM(self._root_dir)
+        self.tree = GitTree(self.git, "master")
+        self._pushd(self._root_dir)
+
+
 class AssertWalkEqualMixin(object):
     def assertWalkEqual(self, actual, expected, msg=None):
         def convert_to_sets(walk_results):