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
This commit is contained in:
Clark Boylan 2021-04-09 13:16:52 -07:00
parent 18189abf59
commit 39cd763d5d
4 changed files with 31 additions and 2 deletions

View File

@ -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 Also can be used for
.Fl \-compare .Fl \-compare
to skip automatic rebase of fetched reviews. 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 .It Fl \-color Ar always|never|auto
Enable or disable a color output. Default is "auto". Enable or disable a color output. Default is "auto".
.It Fl \-no\-color .It Fl \-no\-color

View File

@ -1538,6 +1538,9 @@ additional information:
parser.add_argument("-l", "--list", dest="list", action="count", parser.add_argument("-l", "--list", dest="list", action="count",
help="List available reviews for the current project, " help="List available reviews for the current project, "
"if passed more than once, will show more information") "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", parser.add_argument("-y", "--yes", dest="yes", action="store_true",
help="Indicate that you do, in fact, understand if " help="Indicate that you do, in fact, understand if "
"you are submitting more than one patch") "you are submitting more than one patch")
@ -1701,10 +1704,14 @@ additional information:
sys.exit(1) sys.exit(1)
assert_one_change(remote, branch, yes, have_hook) assert_one_change(remote, branch, yes, have_hook)
no_thin = ''
if options.no_thin:
no_thin = '--no-thin'
ref = "for" ref = "for"
cmd = ("git push --no-follow-tags %s HEAD:refs/%s/%s" % cmd = ("git push --no-follow-tags %s %s HEAD:refs/%s/%s" %
(remote, ref, branch)) (no_thin, remote, ref, branch))
push_options = [] push_options = []
if options.topic is not None: if options.topic is not None:
topic = options.topic topic = options.topic

View File

@ -501,6 +501,15 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
def test_git_review_F_R(self): def test_git_review_F_R(self):
self.assertRaises(Exception, self._run_git_review, '-F', '-R') 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): def test_config_instead_of_honored(self):
self.set_remote('test_project_url') self.set_remote('test_project_url')

View File

@ -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.