Bug 1542625 - worktree should be clean if untracked files are present when applying a patch; r=zalun!

Exclude untracked files when determing current state, and use `--index`
to add new files while applying the patch.

Differential Revision: https://phabricator.services.mozilla.com/D31193

Author: globau

moz-phab

@@ -1332,7 +1332,9 @@ class Git(Repository):
         self.branch = None
 
     def is_worktree_clean(self):
-        return self.git_out(["status", "--short"], split=False) == ""
+        return all(
+            [l.startswith("?? ") for l in self.git_out(["status", "--porcelain"])]
+        )
 
     def before_submit(self):
         # Store current branch (fails if HEAD in detached state)
@@ -1698,10 +1700,6 @@ class Git(Repository):
     def checkout(self, node):
         self.git(["checkout", "--quiet", node])
 
-    def add(self):
-        """Add new files in the working directory."""
-        self.git(["add", "."])
-
     def commit(self, body, author=None, author_date=None):
         """Commit the changes in the working directory."""
         commands = ["commit", "-a"]
@@ -1759,8 +1757,7 @@ class Git(Repository):
 
     def apply_patch(self, diff, body, author, author_date):
         with temporary_file(diff) as patch_file:
-            self.git(["apply", patch_file])
-        self.add()
+            self.git(["apply", "--index", patch_file])
         self.commit(body, author, author_date)
 
     def _get_current_head(self):

tests/test_git.py

@@ -257,14 +257,14 @@ def test_worktree_clean(m_git_out, git):
     m_git_out.return_value = ""
     assert git.is_worktree_clean()
 
-    m_git_out.return_value = "xxx"
+    m_git_out.return_value = ["xxx"]
     assert not git.is_worktree_clean()
 
+    m_git_out.return_value = ["?? one", "?? two"]
+    assert git.is_worktree_clean()
 
-@mock.patch("mozphab.Git.git")
-def test_add(m_git, git):
-    git.add()
-    assert m_git.called_once_with(["add", "."])
+    m_git_out.return_value = ["?? one", "?? two", " M xxx"]
+    assert not git.is_worktree_clean()
 
 
 @mock.patch("mozphab.Git.git")
@@ -375,13 +375,11 @@ def __init__(
 
 @mock.patch("mozphab.temporary_file")
 @mock.patch("mozphab.Git.git")
-@mock.patch("mozphab.Git.add")
 @mock.patch("mozphab.Git.commit")
-def test_apply_patch(m_commit, m_add, m_git, m_temp_fn, git):
+def test_apply_patch(m_commit, m_git, m_temp_fn, git):
     m_temp_fn.return_value = create_temp_fn("filename")
     git.apply_patch("diff", "commit message", "user", 1)
-    m_git.assert_called_once_with(["apply", "filename"])
-    m_add.assert_called_once()
+    m_git.assert_called_once_with(["apply", "--index", "filename"])
     m_commit.assert_called_with("commit message", "user", 1)
     m_temp_fn.assert_called_once_with("diff")
 

tests/test_integration-git.py

@@ -36,6 +36,8 @@ def test_submit_create(in_process, git_repo_path, init_sha):
     testfile.write_text(u"a")
     git_out("add", ".")
     git_out("commit", "--message", "A r?alice")
+    testfile = git_repo_path / "untracked"
+    testfile.write_text(u"a")
 
     mozphab.main(["submit", "--yes", "--bug", "1", init_sha])
 

tests/test_integration-patch.py

@@ -138,7 +138,7 @@ def test_git_patch_with_commit(
     result = git_out("branch")
     assert "* D1" in result
 
-    time.sleep(1)
+    time.sleep(1)  # to ensure the patch is applied with a different timestamp
     mozphab.main(["patch", "D1"])
     assert [".arcconfig", ".git", "X"] == sorted(os.listdir(str(git_repo_path)))
     test_file = git_repo_path / "X"