diff --git a/git_review/cmd.py b/git_review/cmd.py index 6b414b3e..93512427 100755 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -57,7 +57,7 @@ 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", - track="0") + track="0", usepushurl="0") _branch_name = None _has_color = None @@ -369,7 +369,7 @@ def make_remote_url(scheme, username, hostname, port, project): return "%s://%s@%s/%s" % (scheme, username, hostport, project) -def add_remote(scheme, hostname, port, project, remote): +def add_remote(scheme, hostname, port, project, remote, usepushurl): """Adds a gerrit remote.""" asked_for_username = False @@ -392,11 +392,15 @@ def add_remote(scheme, hostname, port, project, remote): "%s" % remote_url) asked_for_username = True - print("Creating a git remote called \"%s\" that maps to:" % remote) + if usepushurl: + cmd = "git remote set-url --push %s %s" % (remote, remote_url) + print("Adding a git push url to '%s' that maps to:" % remote) + else: + cmd = "git remote add -f %s %s" % (remote, remote_url) + print("Creating a git remote called '%s' that maps to:" % remote) print("\t%s" % remote_url) - cmd = "git remote add -f %s %s" % (remote, remote_url) - (status, remote_output) = run_command_status(cmd) + (status, remote_output) = run_command_status(cmd) if status != 0: raise CommandFailed(status, remote_output, cmd, {}) @@ -678,6 +682,7 @@ def load_config_file(config_file): 'remote': 'defaultremote', 'rebase': 'defaultrebase', 'track': 'track', + 'usepushurl': 'usepushurl', } config = {} for config_key, option_name in options.items(): @@ -732,42 +737,58 @@ def resolve_tracking(remote, branch): return remote, branch -def check_remote(branch, remote, scheme, hostname, port, project): +def check_remote(branch, remote, scheme, hostname, port, project, + usepushurl=False): """Check that a Gerrit Git remote repo exists, if not, set one.""" - has_color = check_color_support() - if has_color: - color_never = "--color=never" + if usepushurl: + push_url = git_config_get_value('remote.%s' % remote, 'pushurl', None) + if push_url: + return else: - color_never = "" + has_color = check_color_support() + if has_color: + color_never = "--color=never" + else: + color_never = "" - if remote in run_command("git remote").split("\n"): + if remote in run_command("git remote").split("\n"): - remotes = run_command("git branch -a %s" % color_never).split("\n") - for current_remote in remotes: - if (current_remote.strip() == "remotes/%s/%s" % (remote, branch) - and not UPDATE): - return - # We have the remote, but aren't set up to fetch. Fix it - if VERBOSE: - print("Setting up gerrit branch tracking for better rebasing") - update_remote(remote) - return + remotes = run_command("git branch -a %s" % color_never).split("\n") + for current_remote in remotes: + remote_string = "remotes/%s/%s" % (remote, branch) + if (current_remote.strip() == remote_string and not UPDATE): + return + # We have the remote, but aren't set up to fetch. Fix it + if VERBOSE: + print("Setting up gerrit branch tracking for better rebasing") + update_remote(remote) + return if hostname is False or project is False: # This means there was no .gitreview file printwrap("No '.gitreview' file found in this repository. We don't " - "know where your gerrit is. Please manually create a remote " - "named \"%s\" and try again." % remote) + "know where your gerrit is.") + if usepushurl: + printwrap("Please set the push-url on your origin remote to the " + "location of your gerrit server and try again") + else: + printwrap("Please manually create a remote " + "named \"%s\" and try again." % remote) sys.exit(1) # Gerrit remote not present, try to add it try: - add_remote(scheme, hostname, port, project, remote) + add_remote(scheme, hostname, port, project, remote, usepushurl) except Exception: print(sys.exc_info()[2]) - printwrap("We don't know where your gerrit is. Please manually create " - "a remote named \"%s\" and try again." % remote) + if usepushurl: + printwrap("We don't know where your gerrit is. Please manually" + " add a push-url to the '%s' remote and try again." + % remote) + else: + printwrap("We don't know where your gerrit is. Please manually" + " create a remote named '%s' and try again." % remote) raise @@ -1294,6 +1315,10 @@ def _main(): help="Regenerate Change-id before submitting") parser.add_argument("-r", "--remote", dest="remote", help="git remote to use for gerrit") + parser.add_argument("--use-pushurl", dest="usepushurl", + action="store_true", + help="Use remote push-url logic instead of separate" + " remotes") rebase_group = parser.add_mutually_exclusive_group() rebase_group.add_argument("-R", "--no-rebase", dest="rebase", @@ -1400,7 +1425,8 @@ def _main(): config = Config(os.path.join(top_dir, ".gitreview")) parser.set_defaults(rebase=convert_bool(config['rebase']), track=convert_bool(config['track']), - remote=config['remote']) + remote=None, + usepushurl=convert_bool(config['usepushurl'])) options = parser.parse_args() if no_git_dir: raise no_git_dir @@ -1421,6 +1447,11 @@ def _main(): VERBOSE = options.verbose UPDATE = options.update remote = options.remote + if not remote: + if options.usepushurl: + remote = 'origin' + else: + remote = config['remote'] yes = options.yes status = 0 @@ -1428,7 +1459,8 @@ def _main(): remote, branch = resolve_tracking(remote, branch) check_remote(branch, remote, config['scheme'], - config['hostname'], config['port'], config['project']) + config['hostname'], config['port'], config['project'], + usepushurl=options.usepushurl) if options.color: set_color_output(options.color) diff --git a/git_review/tests/__init__.py b/git_review/tests/__init__.py index 0accecd7..ef1fb0d3 100644 --- a/git_review/tests/__init__.py +++ b/git_review/tests/__init__.py @@ -131,6 +131,7 @@ class BaseGitReviewTestCase(testtools.TestCase, GerritHelpers): """Base class for the git-review tests.""" _test_counter = 0 + _remote = 'gerrit' @property def project_uri(self): @@ -201,12 +202,18 @@ class BaseGitReviewTestCase(testtools.TestCase, GerritHelpers): # go to the just cloned test Git repository self._run_git('clone', self.project_uri) - self._run_git('remote', 'add', 'gerrit', self.project_uri) + self.configure_gerrit_remote() self.addCleanup(shutil.rmtree, self.test_dir) # ensure user is configured for all tests self._configure_gitreview_username() + def set_remote(self, uri): + self._run_git('remote', 'set-url', self._remote, uri) + + def reset_remote(self): + self._run_git('remote', 'rm', self._remote) + def attach_on_exception(self, filename): @self.addOnException def attach_file(exc_info): @@ -290,6 +297,9 @@ class BaseGitReviewTestCase(testtools.TestCase, GerritHelpers): os.environ['PATH'] = self.ssh_dir + os.pathsep + os.environ['PATH'] os.environ['GIT_SSH'] = self._dir('ssh', 'ssh') + def configure_gerrit_remote(self): + self._run_git('remote', 'add', self._remote, self.project_uri) + def _configure_gitreview_username(self): self._run_git('config', 'gitreview.username', 'test_user') diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py index ffaa34a5..259aec14 100644 --- a/git_review/tests/test_git_review.py +++ b/git_review/tests/test_git_review.py @@ -23,6 +23,39 @@ from git_review import tests from git_review.tests import utils +class ConfigTestCase(tests.BaseGitReviewTestCase): + """Class for config tests.""" + + def test_get_config_from_cli(self): + self.reset_remote() + self._run_git('remote', 'rm', 'origin') + self._create_gitreview_file(defaultremote='remote-file') + self._run_git('config', 'gitreview.remote', 'remote-gitconfig') + self._run_git_review('-s', '-r', 'remote-cli') + + remote = self._run_git('remote').strip() + self.assertEqual('remote-cli', remote) + + def test_get_config_from_gitconfig(self): + self.reset_remote() + self._run_git('remote', 'rm', 'origin') + self._create_gitreview_file(defaultremote='remote-file') + self._run_git('config', 'gitreview.remote', 'remote-gitconfig') + self._run_git_review('-s') + + remote = self._run_git('remote').strip() + self.assertEqual('remote-gitconfig', remote) + + def test_get_config_from_file(self): + self.reset_remote() + self._run_git('remote', 'rm', 'origin') + self._create_gitreview_file(defaultremote='remote-file') + self._run_git_review('-s') + + remote = self._run_git('remote').strip() + self.assertEqual('remote-file', remote) + + class GitReviewTestCase(tests.BaseGitReviewTestCase): """Class for the git-review tests.""" @@ -35,14 +68,14 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): def test_git_review_s(self): """Test git-review -s.""" - self._run_git('remote', 'rm', 'gerrit') + self.reset_remote() self._run_git_review('-s') self._simple_change('test file modified', 'test commit message') self.assertIn('Change-Id:', self._run_git('log', '-1')) def test_git_review_s_in_detached_head(self): """Test git-review -s in detached HEAD state.""" - self._run_git('remote', 'rm', 'gerrit') + self.reset_remote() master_sha1 = self._run_git('rev-parse', 'master') self._run_git('checkout', master_sha1) self._run_git_review('-s') @@ -56,14 +89,14 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): self._run_git('reset', '--hard', 'HEAD^') # Review setup with an outdated repo - self._run_git('remote', 'rm', 'gerrit') + self.reset_remote() self._run_git_review('-s') self._simple_change('test file modified', 'test commit message 2') self.assertIn('Change-Id:', self._run_git('log', '-1')) def test_git_review_s_from_subdirectory(self): """Test git-review -s from subdirectory.""" - self._run_git('remote', 'rm', 'gerrit') + self.reset_remote() utils.run_cmd('mkdir', 'subdirectory', chdir=self.test_dir) self._run_git_review( '-s', chdir=os.path.join(self.test_dir, 'subdirectory')) @@ -81,7 +114,7 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): # download clean Git repository and fresh change from Gerrit to it self._run_git('clone', self.project_uri) - self._run_git('remote', 'add', 'gerrit', self.project_uri) + self.configure_gerrit_remote() self._run_git_review('-d', change_id) self.assertIn('test commit message', self._run_git('log', '-1')) @@ -94,7 +127,7 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): # and branch is tracking head = self._run_git('symbolic-ref', '-q', 'HEAD') self.assertIn( - 'refs/remotes/gerrit/master', + 'refs/remotes/%s/master' % self._remote, self._run_git("for-each-ref", "--format='%(upstream)'", head)) def test_multiple_changes(self): @@ -164,9 +197,11 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): """Test message displayed where no remote branch exists.""" self._run_git_review('-s') self._run_git('checkout', '-b', 'new_branch') + self._simple_change('simple message', + 'need to avoid noop message') exc = self.assertRaises(Exception, self._run_git_review, 'new_branch') self.assertIn("The branch 'new_branch' does not exist on the given " - "remote 'gerrit'", exc.args[0]) + "remote '%s'" % self._remote, exc.args[0]) def test_need_rebase_no_upload(self): """Test change needing a rebase does not upload.""" @@ -179,8 +214,9 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): 'create conflict with master') exc = self.assertRaises(Exception, self._run_git_review) - self.assertIn("Errors running git rebase -p -i remotes/gerrit/master", - exc.args[0]) + self.assertIn( + "Errors running git rebase -p -i remotes/%s/master" % self._remote, + exc.args[0]) self.assertIn("It is likely that your change has a merge conflict.", exc.args[0]) @@ -196,8 +232,9 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): self._dir('test', 'new_test_file.txt')) review_res = self._run_git_review('-v') - self.assertIn("Running: git rebase -p -i remotes/gerrit/master", - review_res) + self.assertIn( + "Running: git rebase -p -i remotes/%s/master" % self._remote, + review_res) self.assertEqual(self._run_git('rev-parse', 'HEAD^1'), head_1) def test_uploads_with_nondefault_rebase(self): @@ -473,53 +510,43 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): def test_git_review_F_R(self): self.assertRaises(Exception, self._run_git_review, '-F', '-R') - def test_get_config_from_cli(self): - self._run_git('remote', 'rm', 'origin') - self._run_git('remote', 'rm', 'gerrit') - self._create_gitreview_file(defaultremote='remote-file') - self._run_git('config', 'gitreview.remote', 'remote-gitconfig') - self._run_git_review('-s', '-r', 'remote-cli') - - remote = self._run_git('remote').strip() - self.assertEqual('remote-cli', remote) - - def test_get_config_from_gitconfig(self): - self._run_git('remote', 'rm', 'origin') - self._run_git('remote', 'rm', 'gerrit') - self._create_gitreview_file(defaultremote='remote-file') - self._run_git('config', 'gitreview.remote', 'remote-gitconfig') - self._run_git_review('-s') - - remote = self._run_git('remote').strip() - self.assertEqual('remote-gitconfig', remote) - - def test_get_config_from_file(self): - self._run_git('remote', 'rm', 'origin') - self._run_git('remote', 'rm', 'gerrit') - self._create_gitreview_file(defaultremote='remote-file') - self._run_git_review('-s') - - remote = self._run_git('remote').strip() - self.assertEqual('remote-file', remote) - def test_config_instead_of_honored(self): - self._run_git('remote', 'add', 'alias', 'test_project_url') + self.set_remote('test_project_url') - exc = self.assertRaises(Exception, self._run_git_review, - '-l', '-r', 'alias') - self.assertIn("'test_project_url' does not appear to be a git " - "repository", exc.args[0]) + self.assertRaises(Exception, self._run_git_review, '-l') self._run_git('config', '--add', 'url.%s.insteadof' % self.project_uri, 'test_project_url') - self._run_git_review('-l', '-r', 'alias') + self._run_git_review('-l') + + def test_config_pushinsteadof_honored(self): + self.set_remote('test_project_url') + + self.assertRaises(Exception, self._run_git_review, '-l') - self._run_git('config', '--unset', - 'url.%s.insteadof' % self.project_uri) self._run_git('config', '--add', 'url.%s.pushinsteadof' % self.project_uri, 'test_project_url') - self._run_git_review('-l', '-r', 'alias') + self._run_git_review('-l') + + +class PushUrlTestCase(GitReviewTestCase): + """Class for the git-review tests using origin push-url.""" + + _remote = 'origin' + + def set_remote(self, uri): + self._run_git('remote', 'set-url', '--push', self._remote, uri) + + def reset_remote(self): + self._run_git('config', '--unset', 'remote.%s.pushurl' % self._remote) + + def configure_gerrit_remote(self): + self.set_remote(self.project_uri) + self._run_git('config', 'gitreview.usepushurl', '1') + + def test_config_pushinsteadof_honored(self): + self.skipTest("pushinsteadof doesn't rewrite pushurls") class HttpGitReviewTestCase(tests.HttpMixin, GitReviewTestCase):