From 39cd763d5d3ae310b00e54f6fc24eba993ed57f5 Mon Sep 17 00:00:00 2001
From: Clark Boylan <clark.boylan@gmail.com>
Date: Fri, 9 Apr 2021 13:16:52 -0700
Subject: [PATCH] Add option for disabling thin pushes

There is a long standing issue with C Git pushing to Gerrit and Jgit
where the occasional push will fail because the negotiated packs are
missing a tree object. This happens very occasionally but when it does
it would be nice to be able to point users at an easy workaround.
Pushing with --no-thin is that workaround.

Note that --no-thin is much less efficient so shouldn't be used by
default.

This old bug, https://bugs.launchpad.net/git-review/+bug/1332549, has
details but it seems to affect current C git and Gerrit+Jgit.

Change-Id: Id6ba52a656a14c921acab1b14ef668e6251245da
---
 git-review.1                                          |  4 ++++
 git_review/cmd.py                                     | 11 +++++++++--
 git_review/tests/test_git_review.py                   |  9 +++++++++
 .../notes/add-no-thin-option-ea7cb50e22e7017d.yaml    |  9 +++++++++
 4 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/add-no-thin-option-ea7cb50e22e7017d.yaml

diff --git a/git-review.1 b/git-review.1
index 5b2218ac..c20e06c4 100644
--- a/git-review.1
+++ b/git-review.1
@@ -179,6 +179,10 @@ When submitting a change for review, you will usually want it to be based on the
 Also can be used for
 .Fl \-compare
 to skip automatic rebase of fetched reviews.
+.It Fl \-no-thin
+Disable thin pushes when pushing to Gerrit. This should only be used if you
+are currently experiencing unpack failures due to missing trees. It should
+not be required in typical day to day use.
 .It Fl \-color Ar always|never|auto
 Enable or disable a color output. Default is "auto".
 .It Fl \-no\-color
diff --git a/git_review/cmd.py b/git_review/cmd.py
index b3e7e38e..a97a20c1 100644
--- a/git_review/cmd.py
+++ b/git_review/cmd.py
@@ -1538,6 +1538,9 @@ additional information:
     parser.add_argument("-l", "--list", dest="list", action="count",
                         help="List available reviews for the current project, "
                         "if passed more than once, will show more information")
+    parser.add_argument("--no-thin", dest="no_thin", action="store_true",
+                        help="git push with --no-thin. This may workaround "
+                        "issues with pushing in some circumstances.")
     parser.add_argument("-y", "--yes", dest="yes", action="store_true",
                         help="Indicate that you do, in fact, understand if "
                              "you are submitting more than one patch")
@@ -1701,10 +1704,14 @@ additional information:
             sys.exit(1)
     assert_one_change(remote, branch, yes, have_hook)
 
+    no_thin = ''
+    if options.no_thin:
+        no_thin = '--no-thin'
+
     ref = "for"
 
-    cmd = ("git push --no-follow-tags %s HEAD:refs/%s/%s" %
-           (remote, ref, branch))
+    cmd = ("git push --no-follow-tags %s %s HEAD:refs/%s/%s" %
+           (no_thin, remote, ref, branch))
     push_options = []
     if options.topic is not None:
         topic = options.topic
diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py
index 7bf89af0..d988e1d0 100644
--- a/git_review/tests/test_git_review.py
+++ b/git_review/tests/test_git_review.py
@@ -501,6 +501,15 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
     def test_git_review_F_R(self):
         self.assertRaises(Exception, self._run_git_review, '-F', '-R')
 
+    def test_git_review_no_thin(self):
+        """Test git-review --no-thin."""
+        self._run_git_review('-s')
+
+        # Push with --no-thin set. We can't really introspect the packs
+        # that were sent so we infer success as the command not failing.
+        self._simple_change('test file modified', 'test commit message')
+        self._run_git_review('--no-thin')
+
     def test_config_instead_of_honored(self):
         self.set_remote('test_project_url')
 
diff --git a/releasenotes/notes/add-no-thin-option-ea7cb50e22e7017d.yaml b/releasenotes/notes/add-no-thin-option-ea7cb50e22e7017d.yaml
new file mode 100644
index 00000000..1f6d79ca
--- /dev/null
+++ b/releasenotes/notes/add-no-thin-option-ea7cb50e22e7017d.yaml
@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    Add support for --no-thin which is passed to git push. This is useful to
+    work around situations where Gerrit's JGit and git-review's C git
+    implementations cannot agree on the pack file contents used to transfer
+    a push. When they disagree you see this as unpack failures due to missing
+    trees. Using --no-thin avoids complicated negotiations and works around
+    this problem.