Skip to content

Commit bf2aa92

Browse files
committed
src/script/config_diff.py: add support for ref-commit-sha and cmp-commit-sha arguments
Introduced `ref-commit-sha` and `cmp-commit-sha` arguments to the `diff-branch-remote-repo` mode, enabling comparison of remote branches against specific commits. This enhancement is crucial for comparing configuration changes between a pull request (PR) and the Ceph upstream main branch. It allows for precise comparison by focusing on files changed in the PR, rather than simply comparing the PR's head with its latest commit. The approach mirrors GitHub's three-dot diff [1], where the PR is compared against the common ancestor of the Ceph upstream repository , i.e., the point where the PR was forked. [1]: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-comparing-branches-in-pull-requests Signed-off-by: Naveen Naidu <[email protected]>
1 parent 37d296b commit bf2aa92

File tree

2 files changed

+72
-10
lines changed

2 files changed

+72
-10
lines changed

src/script/config-diff/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ python3 config_diff.py <mode> [options]
5252
- `--cmp-branch`: The branch to compare.
5353
- `--remote-repo`: The remote repository URL for the branch to compare.
5454
- `--ref-repo`: (Optional) The repository URL for the reference branch. Defaults to the Ceph upstream repository.
55+
- `--ref-commit-sha`: (Optional) The commit sha for the reference branch that is used for reference
56+
- `--cmp-commit-sha`: (Optional) The commit sha of the comparing branch
5557
- `--skip-clone`: (Optional) Skips cloning repositories for diff. **Note**: When using this flag, the script must be run from a valid Ceph upstream repository or a forked repository that has access to the branches present in the upstream repository or already contains those branches.
5658
- `--format`: (Optional) Specify the output format for the configuration diff. Options are `json` or `posix-diff`. Default is `json`.
5759

src/script/config-diff/config_diff.py

Lines changed: 70 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ def git_show_yaml_files(hexsha: str, repo: Repo):
6161

6262

6363
def sparse_branch_checkout_remote_repo_skip_clone(
64-
remote_repo, remote_branch_name, local_branch_name
64+
remote_repo: str, remote_branch_name: str, local_branch_name: str, commit_sha: str
6565
) -> Repo:
6666
repo = Repo(".", search_parent_directories=True)
6767
git_cmd = repo.git
@@ -80,14 +80,21 @@ def sparse_branch_checkout_remote_repo_skip_clone(
8080
"--depth=1",
8181
)
8282

83+
if commit_sha:
84+
git_cmd.fetch(
85+
REMOTE_REPO_GIT_REMOTE_NAME,
86+
commit_sha,
87+
"--depth=1",
88+
)
89+
8390
if not folder_exists_in_branch(local_branch_name, git_cmd, CEPH_CONFIG_OPTIONS_FOLDER_PATH):
8491
git_cmd.sparse_checkout("add", CEPH_CONFIG_OPTIONS_FOLDER_PATH)
8592
git_cmd.checkout()
8693

8794
return repo
8895

8996

90-
def sparse_branch_checkout_skip_clone(branch_name) -> Repo:
97+
def sparse_branch_checkout_skip_clone(branch_name: str, commit_sha: str) -> Repo:
9198
repo = Repo(".", search_parent_directories=True)
9299
git_cmd = repo.git
93100

@@ -105,14 +112,23 @@ def sparse_branch_checkout_skip_clone(branch_name) -> Repo:
105112
"--depth=1",
106113
)
107114

115+
if commit_sha:
116+
git_cmd.fetch(
117+
"origin",
118+
commit_sha,
119+
"--depth=1",
120+
)
121+
108122
if not folder_exists_in_branch(branch_name, git_cmd, CEPH_CONFIG_OPTIONS_FOLDER_PATH):
109123
git_cmd.sparse_checkout("add", CEPH_CONFIG_OPTIONS_FOLDER_PATH)
110124
git_cmd.checkout()
111125

112126
return repo
113127

114128

115-
def sparse_branch_checkout(repo_url: str, branch_name: str) -> tempfile.TemporaryDirectory[str]:
129+
def sparse_branch_checkout(
130+
repo_url: str, branch_name: str, commit_sha: str = None
131+
) -> tempfile.TemporaryDirectory[str]:
116132
"""
117133
Clone a sparse branch and checkout the required folder.
118134
@@ -135,9 +151,19 @@ def sparse_branch_checkout(repo_url: str, branch_name: str) -> tempfile.Temporar
135151
"--depth=1",
136152
],
137153
)
154+
138155
git_cmd = repo.git
156+
if commit_sha:
157+
git_cmd.fetch(
158+
"origin",
159+
commit_sha,
160+
"--depth=1",
161+
)
139162
git_cmd.sparse_checkout("add", CEPH_CONFIG_OPTIONS_FOLDER_PATH)
140-
git_cmd.checkout()
163+
if commit_sha:
164+
git_cmd.checkout("FETCH_HEAD")
165+
else:
166+
git_cmd.checkout()
141167
repo.close()
142168

143169
return config_tmp_dir
@@ -450,6 +476,8 @@ def diff_branch_remote_repo(
450476
ref_branch: str,
451477
remote_repo: str,
452478
cmp_branch: str,
479+
ref_commit_sha: str,
480+
cmp_commit_sha: str,
453481
skip_clone: bool,
454482
format_type: str,
455483
):
@@ -465,26 +493,44 @@ def diff_branch_remote_repo(
465493
format_type (str): How should the results be printed.
466494
"""
467495
final_result = {}
496+
ref_config_dict = {}
497+
config_dict = {}
468498
if skip_clone:
469499
cmp_branch_local_branch_name = REMOTE_REPO_GIT_REMOTE_NAME + "/" + cmp_branch
470-
ref_git_repo = sparse_branch_checkout_skip_clone(ref_branch)
500+
ref_git_repo = sparse_branch_checkout_skip_clone(ref_branch, ref_commit_sha)
471501
remote_git_repo = sparse_branch_checkout_remote_repo_skip_clone(
472-
remote_repo, cmp_branch, cmp_branch_local_branch_name
502+
remote_repo, cmp_branch, cmp_branch_local_branch_name, cmp_commit_sha
473503
)
474-
ref_config_dict = git_show_yaml_files(ref_branch, ref_git_repo)
504+
if ref_commit_sha:
505+
ref_config_dict = git_show_yaml_files(ref_commit_sha, ref_git_repo)
506+
else:
507+
ref_config_dict = git_show_yaml_files(ref_branch, ref_git_repo)
475508

476509
# To show the files from remote repo, you need to append the remote name
477510
# before the branch
478-
config_dict = git_show_yaml_files(cmp_branch_local_branch_name, remote_git_repo)
511+
if cmp_commit_sha:
512+
config_dict = git_show_yaml_files(cmp_commit_sha, remote_git_repo)
513+
else:
514+
config_dict = git_show_yaml_files(cmp_branch_local_branch_name, remote_git_repo)
479515

480516
final_result = diff_config(ref_config_dict, config_dict)
481517

482518
ref_git_repo.delete_remote(REMOTE_REPO_GIT_REMOTE_NAME)
483519
ref_git_repo.close()
484520
remote_git_repo.close()
485521
else:
486-
ref_repo_tmp_dir = sparse_branch_checkout(ref_repo, ref_branch)
487-
cmp_repo_tmp_dir = sparse_branch_checkout(remote_repo, cmp_branch)
522+
if ref_commit_sha:
523+
ref_repo_tmp_dir = sparse_branch_checkout(
524+
ref_repo, ref_branch, commit_sha=ref_commit_sha
525+
)
526+
else:
527+
ref_repo_tmp_dir = sparse_branch_checkout(ref_repo, ref_branch)
528+
if cmp_commit_sha:
529+
cmp_repo_tmp_dir = sparse_branch_checkout(
530+
remote_repo, cmp_branch, commit_sha=cmp_commit_sha
531+
)
532+
else:
533+
cmp_repo_tmp_dir = sparse_branch_checkout(remote_repo, cmp_branch)
488534
ref_config_dict = load_config_yaml_files(Path(ref_repo_tmp_dir.name))
489535
config_dict = load_config_yaml_files(Path(cmp_repo_tmp_dir.name))
490536
final_result = diff_config(ref_config_dict, config_dict)
@@ -573,6 +619,12 @@ def main():
573619
parser_diff_branch_remote_repo.add_argument(
574620
"--cmp-branch", required=True, help="the branch to compare against"
575621
)
622+
parser_diff_branch_remote_repo.add_argument(
623+
"--ref-commit-sha", required=False, help="the reference commit"
624+
)
625+
parser_diff_branch_remote_repo.add_argument(
626+
"--cmp-commit-sha", required=False, help="the commit to compare against"
627+
)
576628
parser_diff_branch_remote_repo.add_argument(
577629
"--skip-clone",
578630
action="store_true",
@@ -590,6 +642,12 @@ def main():
590642
if args.skip_clone and args.ref_repo != CEPH_UPSTREAM_REMOTE_URL:
591643
parser.error("--ref-repo cannot be set if --skip-clone is used.")
592644

645+
if args.ref_commit_sha and not args.ref_branch:
646+
parser.error("--ref-commit-sha needs --ref-branch to be set.")
647+
648+
if args.cmp_commit_sha and not args.cmp_branch:
649+
parser.error("--cmp-commit-sha needs --cmp-branch to be set.")
650+
593651
if args.mode == "diff-branch":
594652
diff_branch(args.ref_repo, args.ref_branch, args.cmp_branch, args.skip_clone, args.format)
595653

@@ -602,6 +660,8 @@ def main():
602660
args.ref_branch,
603661
args.remote_repo,
604662
args.cmp_branch,
663+
args.ref_commit_sha,
664+
args.cmp_commit_sha,
605665
args.skip_clone,
606666
args.format,
607667
)

0 commit comments

Comments
 (0)