diff --git a/git-review.1 b/git-review.1
index 237212b..8515835 100644
--- a/git-review.1
+++ b/git-review.1
@@ -160,6 +160,18 @@ 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 \-track
+Choose the branch to submit the change against (and, if
+rebasing, to rebase against) from the branch being tracked
+(if a branch is being tracked), and set the tracking branch
+when downloading a change to point to the remote and branch
+against which patches should be submitted.
+See gitreview.track configuration.
+.It Fl \-no\-track
+Ignore any branch being tracked by the current branch,
+overriding gitreview.track.
+This option is implied by providing a specific branch name
+on the command line.
.It Fl \-version
Print the version number and exit.
.El
@@ -199,6 +211,37 @@ This setting determines the default name to use for gerrit remote
.It gitreview.branch
This setting determines the default branch
.Ed
+.It gitreview.track
+Determines whether to prefer the currently-tracked branch (if any)
+and the branch against which the changeset was submitted to Gerrit
+(if there is exactly one such branch) to the defaultremote and
+defaultbranch for submitting and rebasing against.
+If the local topic branch is tracking a remote branch, the remote
+and branch that the local topic branch is tracking should be used
+for submit and rebase operations, rather than the defaultremote
+and defaultbranch.
+.Pp
+When downloading a patch, creates the local branch to track the
+appropriate remote and branch in order to choose that branch by
+default when submitting modifications to that changeset.
+.Pp
+A value of 'true' or 'false' should be specified.
+.Bl -tag
+.It true
+Do prefer the currently-tracked branch (if any) \- equivalent
+to setting
+.Fl \-track
+when submitting changes.
+.It false
+Ignore tracking branches \- equivalent to setting
+.Fl \-no\-track
+(the default) or providing an explicit branch name when submitting
+changes. This is the default value unless overridden by
+.Pa .gitreview
+file, and is implied by providing a specific branch name on the
+command line.
+.El
+.Ed
.It gitreview.rebase
This setting determines whether changes submitted will
be rebased to the newest state of the branch.
@@ -278,6 +321,7 @@ project=department/project.git
defaultbranch=master
defaultremote=review
defaultrebase=0
+track=0
.Ed
.Pp
When the same option is provided through FILES and CONFIGURATION, the
diff --git a/git_review/cmd.py b/git_review/cmd.py
index 3abf327..17d4d98 100755
--- a/git_review/cmd.py
+++ b/git_review/cmd.py
@@ -56,7 +56,8 @@ CONFIGDIR = os.path.expanduser("~/.config/git-review")
GLOBAL_CONFIG = "/etc/git-review/git-review.conf"
USER_CONFIG = os.path.join(CONFIGDIR, "git-review.conf")
DEFAULTS = dict(scheme='ssh', hostname=False, port=None, project=False,
- branch='master', remote="gerrit", rebase="1")
+ branch='master', remote="gerrit", rebase="1",
+ track="0")
_branch_name = None
_has_color = None
@@ -653,6 +654,7 @@ def load_config_file(config_file):
'branch': 'defaultbranch',
'remote': 'defaultremote',
'rebase': 'defaultrebase',
+ 'track': 'track',
}
config = {}
for config_key, option_name in options.items():
@@ -674,6 +676,39 @@ def update_remote(remote):
return True
+def parse_tracking(ref=None):
+ """Return tracked (remote, branch) of current HEAD or other named
+ branch if tracking remote.
+ """
+ if ref is None:
+ ref = run_command_exc(
+ SymbolicRefFailed,
+ "git", "symbolic-ref", "-q", "HEAD")
+ tracked = run_command_exc(
+ ForEachRefFailed,
+ "git", "for-each-ref", "--format=%(upstream)", ref)
+
+ # Only on explicitly tracked remote branch do we diverge from default
+ if tracked and tracked.startswith('refs/remotes/'):
+ return tracked[13:].partition('/')[::2]
+
+ return None, None
+
+
+def resolve_tracking(remote, branch):
+ """Resolve tracked upstream remote/branch if current branch is tracked."""
+ tracked_remote, tracked_branch = parse_tracking()
+ # tracked_branch will be empty when tracking a local branch
+ if tracked_branch:
+ if VERBOSE:
+ print('Following tracked %s/%s rather than default %s/%s' % (
+ tracked_remote, tracked_branch,
+ remote, branch))
+ return tracked_remote, tracked_branch
+
+ return remote, branch
+
+
def check_remote(branch, remote, scheme, hostname, port, project):
"""Check that a Gerrit Git remote repo exists, if not, set one."""
@@ -982,6 +1017,26 @@ class ResetHardFailed(CommandFailed):
EXIT_CODE = 66
+class SetUpstreamBranchFailed(CommandFailed):
+ "Cannot set upstream to remote branch"
+ EXIT_CODE = 67
+
+
+class SymbolicRefFailed(CommandFailed):
+ "Cannot find symbolic reference"
+ EXIT_CODE = 68
+
+
+class ForEachRefFailed(CommandFailed):
+ "Cannot process symbolic reference"
+ EXIT_CODE = 69
+
+
+class BranchTrackingMismatch(GitReviewException):
+ "Branch exists but is tracking unexpected branch"
+ EXIT_CODE = 70
+
+
def fetch_review(review, masterbranch, remote):
remote_url = get_remote_url(remote)
@@ -1020,6 +1075,7 @@ def fetch_review(review, masterbranch, remote):
author = re.sub('\W+', '_', review_info['owner']['name']).lower()
except KeyError:
author = 'unknown'
+ remote_branch = review_info['branch']
if patchset_number is None:
branch_name = "review/%s/%s" % (author, topic)
@@ -1029,10 +1085,10 @@ def fetch_review(review, masterbranch, remote):
print("Downloading %s from gerrit" % refspec)
run_command_exc(PatchSetGitFetchFailed,
"git", "fetch", remote, refspec)
- return branch_name
+ return branch_name, remote_branch
-def checkout_review(branch_name):
+def checkout_review(branch_name, remote, remote_branch):
"""Checkout a newly fetched (FETCH_HEAD) change
into a branch
"""
@@ -1041,10 +1097,24 @@ def checkout_review(branch_name):
run_command_exc(CheckoutNewBranchFailed,
"git", "checkout", "-b",
branch_name, "FETCH_HEAD")
+ # --set-upstream-to is not supported in git 1.7
+ run_command_exc(SetUpstreamBranchFailed,
+ "git", "branch", "--set-upstream",
+ branch_name,
+ '%s/%s' % (remote, remote_branch))
except CheckoutNewBranchFailed as e:
if re.search("already exists\.?", e.output):
- print("Branch already exists - reusing")
+ print("Branch %s already exists - reusing" % branch_name)
+ track_remote, track_branch = parse_tracking(
+ ref='refs/heads/' + branch_name)
+ if track_remote and not (track_remote == remote and
+ track_branch == remote_branch):
+ print("Branch %s incorrectly tracking %s/%s instead of %s/%s"
+ % (branch_name,
+ track_remote, track_branch,
+ remote, remote_branch))
+ raise BranchTrackingMismatch
run_command_exc(CheckoutExistingBranchFailed,
"git", "checkout", branch_name)
run_command_exc(ResetHardFailed,
@@ -1101,8 +1171,8 @@ def compare_review(review_spec, branch, remote, rebase=False):
old_review = build_review_number(review, old_ps)
new_review = build_review_number(review, new_ps)
- old_branch = fetch_review(old_review, branch, remote)
- checkout_review(old_branch)
+ old_branch, _ = fetch_review(old_review, branch, remote)
+ checkout_review(old_branch, None, None)
if rebase:
print('Rebasing %s' % old_branch)
@@ -1111,8 +1181,8 @@ def compare_review(review_spec, branch, remote, rebase=False):
print('Skipping rebase because of conflicts')
run_command_exc(CommandFailed, 'git', 'rebase', '--abort')
- new_branch = fetch_review(new_review, branch, remote)
- checkout_review(new_branch)
+ new_branch, remote_branch = fetch_review(new_review, branch, remote)
+ checkout_review(new_branch, remote, remote_branch)
if rebase:
print('Rebasing also %s' % new_branch)
@@ -1186,6 +1256,14 @@ def _main():
action="store_true",
help="Force rebase even when not needed.")
+ track_group = parser.add_mutually_exclusive_group()
+ track_group.add_argument("--track", dest="track",
+ action="store_true",
+ help="Use tracked branch as default.")
+ track_group.add_argument("--no-track", dest="track",
+ action="store_false",
+ help="Ignore tracked branch.")
+
fetch = parser.add_mutually_exclusive_group()
fetch.set_defaults(download=False, compare=False, cherrypickcommit=False,
cherrypickindicate=False, cherrypickonly=False)
@@ -1273,8 +1351,8 @@ def _main():
else:
no_git_dir = False
config = Config(os.path.join(top_dir, ".gitreview"))
- parser.set_defaults(branch=config['branch'],
- rebase=convert_bool(config['rebase']),
+ parser.set_defaults(rebase=convert_bool(config['rebase']),
+ track=convert_bool(config['track']),
remote=config['remote'])
options = parser.parse_args()
if no_git_dir:
@@ -1284,7 +1362,13 @@ def _main():
print(COPYRIGHT)
sys.exit(0)
- branch = options.branch
+ if options.branch is None:
+ branch = config['branch']
+ else:
+ # explicitly-specified branch on command line overrides options.track
+ branch = options.branch
+ options.track = False
+
global VERBOSE
global UPDATE
VERBOSE = options.verbose
@@ -1293,6 +1377,9 @@ def _main():
yes = options.yes
status = 0
+ if options.track:
+ remote, branch = resolve_tracking(remote, branch)
+
check_remote(branch, remote, config['scheme'],
config['hostname'], config['port'], config['project'])
@@ -1304,9 +1391,10 @@ def _main():
compare_review(options.changeidentifier,
branch, remote, options.rebase)
return
- local_branch = fetch_review(options.changeidentifier, branch, remote)
+ local_branch, remote_branch = fetch_review(options.changeidentifier,
+ branch, remote)
if options.download:
- checkout_review(local_branch)
+ checkout_review(local_branch, remote, remote_branch)
else:
if options.cherrypickcommit:
cherrypick_review()
diff --git a/git_review/tests/__init__.py b/git_review/tests/__init__.py
index bcd7c73..dc2d894 100644
--- a/git_review/tests/__init__.py
+++ b/git_review/tests/__init__.py
@@ -239,6 +239,16 @@ class BaseGitReviewTestCase(testtools.TestCase, GerritHelpers):
self._run_git('add', file_)
self._run_git('commit', '-m', commit_message)
+ def _simple_amend(self, change_text, file_=None):
+ """Helper method to amend existing commit with change."""
+ if file_ is None:
+ file_ = self._dir('test', 'test_file_new.txt')
+ utils.write_to_file(file_, change_text.encode())
+ self._run_git('add', file_)
+ # cannot use --no-edit because it does not exist in older git
+ message = self._run_git('log', '-1', '--format=%s\n\n%b')
+ self._run_git('commit', '--amend', '-m', 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 9044911..94bd39b 100644
--- a/git_review/tests/test_git_review.py
+++ b/git_review/tests/test_git_review.py
@@ -90,6 +90,12 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
self.assertNotIn('test commit message',
self._run_git('show', 'HEAD^1'))
+ # and branch is tracking
+ head = self._run_git('symbolic-ref', '-q', 'HEAD')
+ self.assertIn(
+ 'refs/remotes/gerrit/master',
+ self._run_git("for-each-ref", "--format='%(upstream)'", head))
+
def test_multiple_changes(self):
"""Test git-review asks about multiple changes.
@@ -160,6 +166,73 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
review_res)
self.assertEqual(self._run_git('rev-parse', 'HEAD^1'), head_1)
+ def test_uploads_with_nondefault_rebase(self):
+ """Test changes rebase against correct branches."""
+ # prepare maintenance branch that is behind master
+ self._create_gitreview_file(track='true',
+ defaultremote='origin')
+ self._run_git('add', '.gitreview')
+ self._run_git('commit', '-m', 'track=true.')
+ self._simple_change('diverge master from maint',
+ 'no conflict',
+ self._dir('test', 'test_file_to_diverge.txt'))
+ self._run_git('push', 'origin', 'master')
+ self._run_git('push', 'origin', 'master', 'master:other')
+ self._run_git_review('-s')
+ head_1 = self._run_git('rev-parse', 'HEAD^1')
+ self._run_gerrit_cli('create-branch',
+ 'test/test_project',
+ 'maint', head_1)
+ self._run_git('fetch')
+
+ br_out = self._run_git('checkout',
+ '-b', 'test_branch', 'origin/maint')
+ expected_track = 'Branch test_branch set up to track remote' + \
+ ' branch maint from origin.'
+ self.assertIn(expected_track, br_out)
+ branches = self._run_git('branch', '-a')
+ expected_branch = '* test_branch'
+ observed = branches.split('\n')
+ self.assertIn(expected_branch, observed)
+
+ self._simple_change('some new message',
+ 'just another file (no conflict)',
+ self._dir('test', 'new_tracked_test_file.txt'))
+ change_id = self._run_git('log', '-1').split()[-1]
+
+ review_res = self._run_git_review('-v')
+ # no rebase needed; if it breaks it would try to rebase to master
+ self.assertNotIn("Running: git rebase -p -i remotes/origin/master",
+ review_res)
+ # Don't need to query gerrit for the branch as the second half
+ # of this test will work only if the branch was correctly
+ # stored in gerrit
+
+ # delete branch locally
+ self._run_git('checkout', 'master')
+ self._run_git('branch', '-D', 'test_branch')
+
+ # download, amend, submit
+ self._run_git_review('-d', change_id)
+ self._simple_amend('just another file (no conflict)',
+ self._dir('test', 'new_tracked_test_file_2.txt'))
+ new_change_id = self._run_git('log', '-1').split()[-1]
+ self.assertEqual(change_id, new_change_id)
+ review_res = self._run_git_review('-v')
+ # caused the right thing to happen
+ self.assertIn("Running: git rebase -p -i remotes/origin/maint",
+ review_res)
+
+ # track different branch than expected in changeset
+ branch = self._run_git('rev-parse', '--abbrev-ref', 'HEAD')
+ self._run_git('branch',
+ '--set-upstream',
+ branch,
+ 'remotes/origin/other')
+ self.assertRaises(
+ Exception, # cmd.BranchTrackingMismatch inside
+ self._run_git_review, '-d', change_id)
+
def test_no_rebase_check(self):
"""Test -R causes a change to be uploaded without rebase checking."""
self._run_git_review('-s')
diff --git a/git_review/tests/test_unit.py b/git_review/tests/test_unit.py
index 30c4fc4..05113fe 100644
--- a/git_review/tests/test_unit.py
+++ b/git_review/tests/test_unit.py
@@ -176,6 +176,48 @@ password=pass
"""
+class ResolveTrackingUnitTest(testtools.TestCase):
+ """Class for testing resolve_tracking."""
+ def setUp(self):
+ testtools.TestCase.setUp(self)
+ patcher = mock.patch('git_review.cmd.run_command_exc')
+ self.addCleanup(patcher.stop)
+ self.run_command_exc = patcher.start()
+
+ def test_track_local_branch(self):
+ 'Test that local tracked branch is not followed.'
+ self.run_command_exc.side_effect = [
+ '',
+ 'refs/heads/other/branch',
+ ]
+ self.assertEqual(cmd.resolve_tracking(u'remote', u'rbranch'),
+ (u'remote', u'rbranch'))
+
+ def test_track_untracked_branch(self):
+ 'Test that local untracked branch is not followed.'
+ self.run_command_exc.side_effect = [
+ '',
+ '',
+ ]
+ self.assertEqual(cmd.resolve_tracking(u'remote', u'rbranch'),
+ (u'remote', u'rbranch'))
+
+ def test_track_remote_branch(self):
+ 'Test that remote tracked branch is followed.'
+ self.run_command_exc.side_effect = [
+ '',
+ 'refs/remotes/other/branch',
+ ]
+ self.assertEqual(cmd.resolve_tracking(u'remote', u'rbranch'),
+ (u'other', u'branch'))
+
+ def test_track_git_error(self):
+ 'Test that local tracked branch is not followed.'
+ self.run_command_exc.side_effect = [cmd.CommandFailed(1, '', [], {})]
+ self.assertRaises(cmd.CommandFailed,
+ cmd.resolve_tracking, u'remote', u'rbranch')
+
+
class GitReviewUnitTest(testtools.TestCase):
"""Class for misc unit tests."""
@@ -236,3 +278,19 @@ class GitReviewUnitTest(testtools.TestCase):
stdin='url=%s' % url)
calls = [mock.call(url), mock.call(url, auth=('user', 'pass'))]
mock_get.assert_has_calls(calls)
+
+ @mock.patch('sys.argv', ['argv0', '--track', 'branch'])
+ @mock.patch('git_review.cmd.check_remote')
+ @mock.patch('git_review.cmd.resolve_tracking')
+ def test_command_line_no_track(self, resolve_tracking, check_remote):
+ check_remote.side_effect = Exception()
+ self.assertRaises(Exception, cmd._main)
+ self.assertFalse(resolve_tracking.called)
+
+ @mock.patch('sys.argv', ['argv0', '--track'])
+ @mock.patch('git_review.cmd.check_remote')
+ @mock.patch('git_review.cmd.resolve_tracking')
+ def test_track(self, resolve_tracking, check_remote):
+ check_remote.side_effect = Exception()
+ self.assertRaises(Exception, cmd._main)
+ self.assertTrue(resolve_tracking.called)