From ff733a6496a4ff67618f4b131bb18b66769f9a62 Mon Sep 17 00:00:00 2001 From: Devon Loehr Date: Fri, 7 Feb 2025 09:47:10 -0800 Subject: [PATCH] `git cl split`: Include dirname as part of branch names This CL changes the way `git cl split` generates branch names during upload. Specifically, it attempts to derive a common directory for all the files in each CL, and includes that dirname as part of the branch name, in addition to a hash of the files in the CL. This makes it a bit easier to guess what's in a branch, given its name. Since the hash is still included, branch names should remain unique. Finding a common directory should be deterministic, so we can still rely on branches having the same name if the script is re-run with the same splitting. Bug: 389069356 Change-Id: I70490258755f13962f3db5c835619caa24176af9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6243377 Commit-Queue: Devon Loehr Reviewed-by: Josip Sokcevic --- split_cl.py | 25 +++++++++++++++++++++++-- tests/split_cl_test.py | 5 +++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/split_cl.py b/split_cl.py index 58b9ebafd..e669260b6 100644 --- a/split_cl.py +++ b/split_cl.py @@ -99,14 +99,35 @@ def EnsureInGitRepository(): git.run('rev-parse') -def CreateBranchForOneCL(prefix, files, upstream): +def CreateBranchName(prefix: str, files: List[Tuple[str, str]]) -> str: + """ + Given a sub-CL as a list of (action, file) pairs, create a unique and + deterministic branch name for it. + The name has the format ___split. + """ + file_names = [file for _, file in files] + if len(file_names) == 1: + # Only one file, just use its directory as the common path + common_path = os.path.dirname(file_names[0]) + else: + common_path = os.path.commonpath(file_names) + if not common_path: + # Files have nothing in common at all. Unlikely but possible. + common_path = "None" + # Replace path delimiter with underscore in common_path. + common_path = common_path.replace(os.path.sep, '_') + return f"{prefix}_{HashList(files):020}_{common_path}_split" + + +def CreateBranchForOneCL(prefix: str, files: List[Tuple[str, str]], + upstream: str) -> bool: """Creates a branch named |prefix| + "_" + |hash(files)| + "_split". Return false if the branch already exists. |upstream| is used as upstream for the created branch. """ existing_branches = set(git.branches(use_limit=False)) - branch_name = f'{prefix}_{HashList(files)}_split' + branch_name = CreateBranchName(prefix, files) if branch_name in existing_branches: return False git.run('checkout', '-t', upstream, '-b', branch_name) diff --git a/tests/split_cl_test.py b/tests/split_cl_test.py index c6a73c73c..ceafe62ed 100755 --- a/tests/split_cl_test.py +++ b/tests/split_cl_test.py @@ -143,7 +143,7 @@ class SplitClTest(unittest.TestCase): self.assertEqual(mock_git_run.call_count, 4) mock_git_run.assert_has_calls([ mock.call("checkout", "-t", "upstream_branch", "-b", - f"branch_to_upload_{split_cl.HashList(files)}_split"), + split_cl.CreateBranchName("branch_to_upload", files)), mock.call("rm", os.path.join(abs_repository_path, "foo", "b.cc")), mock.call("checkout", "branch_to_upload", "--", os.path.join(abs_repository_path, "bar", "a.cc")), @@ -168,7 +168,8 @@ class SplitClTest(unittest.TestCase): reviewers = {"reviewer1@gmail.com"} mock_cmd_upload = mock.Mock() upload_cl_tester.mock_git_branches.return_value = [ - "branch0", f"branch_to_upload_{split_cl.HashList(files)}_split" + "branch0", + split_cl.CreateBranchName("branch_to_upload", files) ] upload_cl_tester.DoUploadCl(directories, files, reviewers, mock_cmd_upload)