From 95226d4cd09790c3628ab8474cfdb21bf3fe99ad Mon Sep 17 00:00:00 2001 From: Diana Picus Date: Wed, 1 Nov 2017 13:16:54 +0100 Subject: llvm push: Print remote branch name after pushing Get llvm.py push to print the name of the remote branch that it has pushed to. This is helpful so developers don't need to guess what namespaces have been prepended to the branch name, and also serves as quick visual confirmation. This required a bit of refactoring, since the name of the branch was computed when needed. We now compute it separately and pass it in to the function that pushes the branch. We rely on the fact that the branch name will be the same for all the subprojects involved. Most of the changes are mechanical, to account for this refactoring (including new names for some of the existing functions, which make more sense in this new context). Change-Id: Id7e496bcded60080803e3a850d73329428bc3bac --- modules/llvm.py | 17 ++-- scripts/llvm.py | 21 +++-- tests/cli/testllvmpush.py | 5 +- tests/unittests/testpush.py | 190 ++++++++++++++++++++++++++------------------ 4 files changed, 139 insertions(+), 94 deletions(-) diff --git a/modules/llvm.py b/modules/llvm.py index 2e7c44f..2ce4994 100644 --- a/modules/llvm.py +++ b/modules/llvm.py @@ -268,25 +268,26 @@ def get_user_from_remote(url): return match.group('user') -def push_current_branch(proj, pathToRepo): - """Push the current branch to origin. - - It will be pushed into linaro-local/$user/$branch, where $branch is the - current branch and $user is the username used as part of the remote URL. +def get_remote_branch(repo, local_branch): + """ + Get the name of the remote branch corresponding to the given local branch. """ - repo = Worktree(proj, pathToRepo) - with cd(repo.repodir): remote = git("remote", "get-url", "--push", "origin").strip() user = get_user_from_remote(remote) if not user: raise EnvironmentError("Couldn't parse user from {}.".format(remote)) - local_branch = repo.getbranch() remote_branch = "linaro-local/{}/{}".format(user, local_branch) if not repo.is_valid_branch_name(remote_branch): raise EnvironmentError( "{} is not a valid branch name.".format(remote_branch)) + return remote_branch + + +def push_branch(proj, local_branch, remote_branch, pathToRepo): + """Push the given local branch into origin's given remote branch.""" + repo = Worktree(proj, pathToRepo) with cd(repo.repodir): git("push", "-u", "origin", "+{}:{}".format(local_branch, remote_branch)) diff --git a/scripts/llvm.py b/scripts/llvm.py index 2252ab2..3244bde 100644 --- a/scripts/llvm.py +++ b/scripts/llvm.py @@ -3,7 +3,11 @@ import os from sys import exit -from modules.llvm import LLVMSubproject, LLVMSourceConfig, push_current_branch +from modules.llvm import LLVMSubproject +from modules.llvm import LLVMSourceConfig +from modules.llvm import get_remote_branch +from modules.llvm import push_branch + from linaropy.git.clone import Clone from linaropy.proj import Proj @@ -66,7 +70,7 @@ def projects(args): dump_config(config) -def push_branch(args): +def push_current_branch(args): """Push current branch to origin.""" proj = Proj() @@ -74,13 +78,18 @@ def push_branch(args): llvm_worktree_root = get_worktree_root(args.env) config = LLVMSourceConfig(proj, llvm_worktree_root) - llvmBranch = Clone(proj, llvm_worktree_root).getbranch() + llvm_worktree = Clone(proj, llvm_worktree_root) + local_branch = llvm_worktree.getbranch() try: - config.for_each_enabled(partial(push_current_branch, proj)) - except RuntimeError as exc: + remote_branch = get_remote_branch(llvm_worktree, local_branch) + config.for_each_enabled(partial(push_branch, proj, local_branch, + remote_branch)) + print("Pushed to {}".format(remote_branch)) + except (EnvironmentError, RuntimeError) as exc: die("Failed to push branch because: " + str(exc) + str(exc.__cause__)) + ########################################################################## # Command line parsing # ########################################################################## @@ -127,7 +136,7 @@ projs.add_argument( push = subcommands.add_parser( "push", help="Push current branch to origin linaro-local//, for all linked subprojects.") -push.set_defaults(run_command=push_branch) +push.set_defaults(run_command=push_current_branch) args = options.parse_args() args.run_command(args) diff --git a/tests/cli/testllvmpush.py b/tests/cli/testllvmpush.py index 769aa18..1d9b8ab 100644 --- a/tests/cli/testllvmpush.py +++ b/tests/cli/testllvmpush.py @@ -101,9 +101,12 @@ class Testllvmpush(LLVMTestCase): with cd(worktreePath): self.create_dummy_commit("Test {} push".format(subproj)) - self.run_quietly(self.llvm_push()) + pushed = self.run_with_output(self.llvm_push()) remote_branch = "linaro-local/{}/{}".format(self.user, self.branch) + self.assertRegex( + pushed, "(.*\n)*Pushed to {}(.*\n)*".format(remote_branch)) + for subproj in self.all_repos: origin = self.get_origin_path(subproj) diff --git a/tests/unittests/testpush.py b/tests/unittests/testpush.py index 0046e2b..3eb8e35 100644 --- a/tests/unittests/testpush.py +++ b/tests/unittests/testpush.py @@ -9,7 +9,30 @@ from linaropy.proj import Proj from linaropy.git.clone import Clone from linaropy.git.worktree import Worktree -from modules.llvm import get_user_from_remote, push_current_branch +from modules.llvm import get_user_from_remote, get_remote_branch, push_branch + +# FIXME: Maybe move these helpers somewhere more public, or use LLVMTestCase for +# these tests as well... + + +def create_dummy_commit(message="Branches without commits confuse git"): + filename = "file" + str(uuid.uuid4()) + open(filename, "a").close() + git("add", filename) + git("commit", "-m", message) + + +def get_last_commit_message(branch=""): + return str(git("rev-list", "-1", "--oneline", branch)).strip() + + +def create_dummy_repo(path): + if not os.path.exists(path): + os.makedirs(path) + + with cd(path): + git("init") + create_dummy_commit() class TestGetUser(unittest.TestCase): @@ -31,27 +54,69 @@ class TestGetUser(unittest.TestCase): self.assertEqual(user, None) -class TestPush(unittest.TestCase): - testdirprefix = "PushUT" +class TestGetRemoteBranch(unittest.TestCase): + testdirprefix = "GetRemoteBranchUT" + + def setUp(self): + # Create a git repo so we can play with its remotes. + self.proj = Proj(prefix=self.testdirprefix) + + path = os.path.join(self.proj.projdir, "repo") + create_dummy_repo(path) + self.repo = Clone(self.proj, path) + + def tearDown(self): + self.proj.cleanup() + + def test_valid_branch(self): + """ + Test that we return the correct remote branch name if everything is + valid. + """ + user = "a.user" + remote = "file://{}@{}".format(user, self.repo.repodir) + with cd(self.repo.repodir): + git("remote", "add", "origin", remote) + + local_branch = "a-branch" + remote_branch = get_remote_branch(self.repo, local_branch) + self.assertEqual( + remote_branch, "linaro-local/{}/{}".format(user, local_branch)) + + def test_no_user(self): + """Test that we error out when we can't parse a user in the remote name.""" + remote = "file://" + self.repo.repodir + with cd(self.repo.repodir): + git("remote", "add", "origin", remote) + + with self.assertRaises(EnvironmentError) as context: + get_remote_branch(self.repo, "a-branch") + + self.assertEqual(str(context.exception), + "Couldn't parse user from {}.".format(remote)) + + def test_invalid_user(self): + """ + Test that we error out when the value of the user parsed from the remote + wouldn't look good in a branch name (e.g. if it contains spaces). + """ + + badUser = "LLVM Developer" + with cd(self.repo.repodir): + git("remote", "add", "origin", + "file://{}@{}".format(badUser, self.repo.repodir)) - def __create_dummy_commit( - self, - message="Branches without commits confuse git"): - filename = "file" + str(uuid.uuid4()) - open(filename, "a").close() - git("add", filename) - git("commit", "-m", message) + branch = "a-branch" + with self.assertRaises(EnvironmentError) as context: + get_remote_branch(self.repo, branch) - def __get_last_commit_message(self, branch=""): - return str(git("rev-list", "-1", "--oneline", branch)).strip() + self.assertEqual(str(context.exception), + "linaro-local/{}/{} is not a valid branch name.".format(badUser, + branch)) - def __create_dummy_repo(self, path): - if not os.path.exists(path): - os.makedirs(path) - with cd(path): - git("init") - self.__create_dummy_commit() +class TestPushBranch(unittest.TestCase): + testdirprefix = "PushBranchUT" def setUp(self): # We're going to create a hierarchy with an origin repo, a clone repo @@ -61,7 +126,7 @@ class TestPush(unittest.TestCase): self.user = "llvm-developer" path = os.path.join(self.proj.projdir, "origin") - self.__create_dummy_repo(path) + create_dummy_repo(path) self.origin = Clone(self.proj, path) path = os.path.join(self.proj.projdir, "clone") @@ -69,6 +134,7 @@ class TestPush(unittest.TestCase): self.clone = Clone(self.proj, path) self.worktreeBranch = "a-branch" + self.remoteBranch = "remotely/a-branch" self.worktree = Worktree.create( self.proj, self.clone, os.path.join( self.proj.projdir, "worktree"), self.worktreeBranch) @@ -80,35 +146,30 @@ class TestPush(unittest.TestCase): """Test that we can push a new branch to origin.""" with cd(self.worktree.repodir): - self.__create_dummy_commit("This should make it to origin") + create_dummy_commit("This should make it to origin") - push_current_branch(self.proj, self.worktree.repodir) + push_branch(self.proj, self.worktreeBranch, self.remoteBranch, + self.worktree.repodir) with cd(self.origin.repodir): self.assertRegex( - self.__get_last_commit_message( - "linaro-local/{}/{}".format( - self.user, - self.worktreeBranch)), + get_last_commit_message(self.remoteBranch), ".*This should make it to origin") def test_push_existing_branch(self): """Test that we can push to a branch that already exists in origin.""" - - remoteBranch = "linaro-local/{}/{}".format( - self.user, self.worktreeBranch) - with cd(self.worktree.repodir): - self.__create_dummy_commit("This already exists in origin") + create_dummy_commit("This already exists in origin") git("push", "origin", "{}:{}".format( - self.worktreeBranch, remoteBranch)) + self.worktreeBranch, self.remoteBranch)) - self.__create_dummy_commit("This should make it to origin too") + create_dummy_commit("This should make it to origin too") - push_current_branch(self.proj, self.worktree.repodir) + push_branch(self.proj, self.worktreeBranch, self.remoteBranch, + self.worktree.repodir) with cd(self.origin.repodir): - self.assertRegex(self.__get_last_commit_message(remoteBranch), + self.assertRegex(get_last_commit_message(self.remoteBranch), ".*This should make it to origin too") def test_push_squashed_update(self): @@ -117,84 +178,55 @@ class TestPush(unittest.TestCase): that has already been pushed to origin. This isn't a nice thing to do, but we need to support it because Gerrit requires squashes. """ - remoteBranch = "linaro-local/{}/{}".format(self.user, - self.worktreeBranch) with cd(self.worktree.repodir): - self.__create_dummy_commit("First version of the patch") + create_dummy_commit("First version of the patch") - push_current_branch(self.proj, self.worktree.repodir) + push_branch(self.proj, self.worktreeBranch, self.remoteBranch, + self.worktree.repodir) with cd(self.origin.repodir): self.assertRegex( - self.__get_last_commit_message(remoteBranch), + get_last_commit_message(self.remoteBranch), ".*First version of the patch") with cd(self.worktree.repodir): git("commit", "--amend", "-m", "Second version of the patch") - push_current_branch(self.proj, self.worktree.repodir) + push_branch(self.proj, self.worktreeBranch, self.remoteBranch, + self.worktree.repodir) with cd(self.origin.repodir): self.assertRegex( - self.__get_last_commit_message(remoteBranch), + get_last_commit_message(self.remoteBranch), "/*Second version of the patch") def test_push_rebased_branch(self): """ Test that we can push again with new updates after a rebase.""" - remoteBranch = "linaro-local/{}/{}".format(self.user, - self.worktreeBranch) with cd(self.worktree.repodir): - self.__create_dummy_commit("First change") + create_dummy_commit("First change") - push_current_branch(self.proj, self.worktree.repodir) + push_branch(self.proj, self.worktreeBranch, self.remoteBranch, + self.worktree.repodir) with cd(self.origin.repodir): self.assertRegex( - self.__get_last_commit_message(remoteBranch), + get_last_commit_message(self.remoteBranch), ".*First change") with cd(self.origin.repodir): - self.__create_dummy_commit("Master moving forward") + create_dummy_commit("Master moving forward") with cd(self.worktree.repodir): - self.__create_dummy_commit("Second change") + create_dummy_commit("Second change") git("fetch", "origin", "master") git("rebase", "origin/master") - push_current_branch(self.proj, self.worktree.repodir) + push_branch(self.proj, self.worktreeBranch, self.remoteBranch, + self.worktree.repodir) with cd(self.origin.repodir): - self.assertRegex(self.__get_last_commit_message(remoteBranch), + self.assertRegex(get_last_commit_message(self.remoteBranch), ".*Second change") - def test_push_no_user(self): - """Test that we error out when we can't parse a user in the remote name.""" - remote = "file://" + self.origin.repodir - with cd(self.worktree.repodir): - git("remote", "set-url", "origin", remote) - self.__create_dummy_commit("Make it look like a real branch") - - with self.assertRaises(EnvironmentError) as context: - push_current_branch(self.proj, self.worktree.repodir) - - self.assertEqual(str(context.exception), - "Couldn't parse user from {}.".format(remote)) - - def test_push_invalid_user(self): - """Test that we error out when the value of LLVM_GITUSER wouldn't look good in a branch name (e.g. if it contains spaces).""" - - badUser = "LLVM Developer" - with cd(self.worktree.repodir): - git("remote", "set-url", "origin", - "file://{}@{}".format(badUser, self.origin.repodir)) - self.__create_dummy_commit("Make it look like a real branch") - - with self.assertRaises(EnvironmentError) as context: - push_current_branch(self.proj, self.worktree.repodir) - - self.assertEqual(str(context.exception), - "linaro-local/{}/{} is not a valid branch name.".format(badUser, - self.worktreeBranch)) - if __name__ == "__main__": unittest.main() -- cgit v1.2.3