Ignore unstaged/uncommitted submodule changes

When checking for unstaged or uncommitted changes to avoid the test
rebase (which could cause data loss for users of git.autostash),
it's still fine if there are unstaged or uncommitted changes in
submodules since those won't be rebased. Have the git diff
invocations explicitly ignore submodules, and also add regression
tests which demonstrate it's working.

This fixes a regression originally introduced by change
Iabb8387c9db59a7d02ebfd43b688e7bb93d3159f.

Change-Id: I20d602e86537b573ac1f9788221215047a594f83

4 files changed, 56 insertions, 2 deletions

diff --git a/git_review/cmd.py b/git_review/cmd.py
index ccf4c09..16b613a 100644
--- a/git_review/cmd.py
+++ b/git_review/cmd.py
@@ -888,13 +888,13 @@ def rebase_changes(branch, remote, interactive=True):
     # Either the rebase will fail with a similar message, or if the user
     # has turned on rebase.autostash then the subsequent reset will
     # silently discard those changes.
-    cmd = "git diff --quiet"
+    cmd = "git diff --ignore-submodules --quiet"
     (status, output) = run_command_status(cmd)
     if status != 0:
         printwrap("You have unstaged changes. Please commit or stash them "
                   "first, and then try again.")
         sys.exit(1)
-    cmd = "git diff --cached --quiet"
+    cmd = "git diff --cached --ignore-submodules --quiet"
     (status, output) = run_command_status(cmd)
     if status != 0:
         printwrap("You have uncommitted changes. Please commit or stash them "
diff --git a/git_review/tests/__init__.py b/git_review/tests/__init__.py
index 379ac86..545495e 100644
--- a/git_review/tests/__init__.py
+++ b/git_review/tests/__init__.py
@@ -291,6 +291,18 @@ class BaseGitReviewTestCase(testtools.TestCase, GerritHelpers):
                              '--work-tree=' + self._dir('test'),
                              command, *args)
 
+    def _run_git_sub(self, command, *args):
+        """Run git command using submodule of test git directory."""
+        if command == 'init':
+            utils.run_git('init', self._dir('test', 'sub'))
+            self._simple_change_sub('submodule content', 'initial commit')
+            utils.run_git('submodule', 'add', os.path.join('.', 'sub'),
+                          chdir=self._dir('test'))
+            return self._run_git('commit', '-m', 'add submodule')
+        return utils.run_git('--git-dir=' + self._dir('test', 'sub', '.git'),
+                             '--work-tree=' + self._dir('test', 'sub'),
+                             command, *args)
+
     def _run_gerrit(self, ssh_addr, ssh_port, http_addr, http_port):
         # create a copy of site dir
         if os.path.exists(self.site_dir):
@@ -344,6 +356,26 @@ class BaseGitReviewTestCase(testtools.TestCase, GerritHelpers):
         message = self._run_git('log', '-1', '--format=%s\n\n%b')
         self._run_git('commit', '--amend', '-m', message)
 
+    def _unstaged_change_sub(self, change_text, file_=None):
+        """Helper method to create small submodule changes and not stage."""
+        if file_ is None:
+            file_ = self._dir('test', 'sub', 'test_file.txt')
+        utils.write_to_file(file_, ''.encode())
+        self._run_git_sub('add', file_)
+        utils.write_to_file(file_, change_text.encode())
+
+    def _uncommitted_change_sub(self, change_text, file_=None):
+        """Helper method to create small submodule changes and not commit."""
+        if file_ is None:
+            file_ = self._dir('test', 'sub', 'test_file.txt')
+        self._unstaged_change_sub(change_text, file_)
+        self._run_git_sub('add', file_)
+
+    def _simple_change_sub(self, change_text, commit_message, file_=None):
+        """Helper method to create small submodule changes and commit them."""
+        self._uncommitted_change_sub(change_text, file_)
+        self._run_git_sub('commit', '-m', commit_message)
+
     def _configure_ssh(self, ssh_addr, ssh_port):
         """Setup ssh and scp to run with special options."""
 
diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py
index ac02bb4..4717bdd 100644
--- a/git_review/tests/test_git_review.py
+++ b/git_review/tests/test_git_review.py
@@ -286,6 +286,22 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
         exc = self.assertRaises(Exception, self._run_git_review)
         self.assertIn("You have uncommitted changes. Please", exc.args[0])
 
+    def test_ignore_unstaged_submodule_changes(self):
+        """Test message displayed when unstaged changes are present."""
+        self._run_git_review('-s')
+        self._run_git('checkout', '-b', 'test_branch')
+        self._run_git_sub('init')
+        self._unstaged_change_sub(change_text='simple message')
+        self._run_git_review()
+
+    def test_ignore_uncommitted_submodule_changes(self):
+        """Test message displayed when staged changes are present."""
+        self._run_git_review('-s')
+        self._run_git('checkout', '-b', 'test_branch')
+        self._run_git_sub('init')
+        self._uncommitted_change_sub(change_text='simple message')
+        self._run_git_review()
+
     def test_rebase_no_remote_branch_msg(self):
         """Test message displayed where no remote branch exists."""
         self._run_git_review('-s')
diff --git a/releasenotes/notes/ignore-unstaged-uncommitted-submodule-changes-99dbf054f5617d08.yaml b/releasenotes/notes/ignore-unstaged-uncommitted-submodule-changes-99dbf054f5617d08.yaml
new file mode 100644
index 0000000..0523b3a
--- /dev/null
+++ b/releasenotes/notes/ignore-unstaged-uncommitted-submodule-changes-99dbf054f5617d08.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    When checking for unstaged or uncommitted changes to avoid performing a
+    test rebase, unstaged and uncommitted changes in Git submodules are now
+    ignored since those won't be rebased anyway.