From: Naveen Naidu Date: Sun, 8 Jun 2025 13:55:24 +0000 (+0530) Subject: src/script/config_diff.py: add support for `ref-commit-sha` and `cmp-commit-sha`... X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=bf2aa9220a1ff747d8dc943ea1ebe8b171d4c07c;p=ceph.git 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 --- diff --git a/src/script/config-diff/README.md b/src/script/config-diff/README.md index 2ab954975f5fb..8274f7b30626b 100644 --- a/src/script/config-diff/README.md +++ b/src/script/config-diff/README.md @@ -52,6 +52,8 @@ python3 config_diff.py [options] - `--cmp-branch`: The branch to compare. - `--remote-repo`: The remote repository URL for the branch to compare. - `--ref-repo`: (Optional) The repository URL for the reference branch. Defaults to the Ceph upstream repository. + - `--ref-commit-sha`: (Optional) The commit sha for the reference branch that is used for reference + - `--cmp-commit-sha`: (Optional) The commit sha of the comparing branch - `--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. - `--format`: (Optional) Specify the output format for the configuration diff. Options are `json` or `posix-diff`. Default is `json`. diff --git a/src/script/config-diff/config_diff.py b/src/script/config-diff/config_diff.py index 1202dee661f8b..7d6dd9943fa43 100644 --- a/src/script/config-diff/config_diff.py +++ b/src/script/config-diff/config_diff.py @@ -61,7 +61,7 @@ def git_show_yaml_files(hexsha: str, repo: Repo): def sparse_branch_checkout_remote_repo_skip_clone( - remote_repo, remote_branch_name, local_branch_name + remote_repo: str, remote_branch_name: str, local_branch_name: str, commit_sha: str ) -> Repo: repo = Repo(".", search_parent_directories=True) git_cmd = repo.git @@ -80,6 +80,13 @@ def sparse_branch_checkout_remote_repo_skip_clone( "--depth=1", ) + if commit_sha: + git_cmd.fetch( + REMOTE_REPO_GIT_REMOTE_NAME, + commit_sha, + "--depth=1", + ) + if not folder_exists_in_branch(local_branch_name, git_cmd, CEPH_CONFIG_OPTIONS_FOLDER_PATH): git_cmd.sparse_checkout("add", CEPH_CONFIG_OPTIONS_FOLDER_PATH) git_cmd.checkout() @@ -87,7 +94,7 @@ def sparse_branch_checkout_remote_repo_skip_clone( return repo -def sparse_branch_checkout_skip_clone(branch_name) -> Repo: +def sparse_branch_checkout_skip_clone(branch_name: str, commit_sha: str) -> Repo: repo = Repo(".", search_parent_directories=True) git_cmd = repo.git @@ -105,6 +112,13 @@ def sparse_branch_checkout_skip_clone(branch_name) -> Repo: "--depth=1", ) + if commit_sha: + git_cmd.fetch( + "origin", + commit_sha, + "--depth=1", + ) + if not folder_exists_in_branch(branch_name, git_cmd, CEPH_CONFIG_OPTIONS_FOLDER_PATH): git_cmd.sparse_checkout("add", CEPH_CONFIG_OPTIONS_FOLDER_PATH) git_cmd.checkout() @@ -112,7 +126,9 @@ def sparse_branch_checkout_skip_clone(branch_name) -> Repo: return repo -def sparse_branch_checkout(repo_url: str, branch_name: str) -> tempfile.TemporaryDirectory[str]: +def sparse_branch_checkout( + repo_url: str, branch_name: str, commit_sha: str = None +) -> tempfile.TemporaryDirectory[str]: """ Clone a sparse branch and checkout the required folder. @@ -135,9 +151,19 @@ def sparse_branch_checkout(repo_url: str, branch_name: str) -> tempfile.Temporar "--depth=1", ], ) + git_cmd = repo.git + if commit_sha: + git_cmd.fetch( + "origin", + commit_sha, + "--depth=1", + ) git_cmd.sparse_checkout("add", CEPH_CONFIG_OPTIONS_FOLDER_PATH) - git_cmd.checkout() + if commit_sha: + git_cmd.checkout("FETCH_HEAD") + else: + git_cmd.checkout() repo.close() return config_tmp_dir @@ -450,6 +476,8 @@ def diff_branch_remote_repo( ref_branch: str, remote_repo: str, cmp_branch: str, + ref_commit_sha: str, + cmp_commit_sha: str, skip_clone: bool, format_type: str, ): @@ -465,17 +493,25 @@ def diff_branch_remote_repo( format_type (str): How should the results be printed. """ final_result = {} + ref_config_dict = {} + config_dict = {} if skip_clone: cmp_branch_local_branch_name = REMOTE_REPO_GIT_REMOTE_NAME + "/" + cmp_branch - ref_git_repo = sparse_branch_checkout_skip_clone(ref_branch) + ref_git_repo = sparse_branch_checkout_skip_clone(ref_branch, ref_commit_sha) remote_git_repo = sparse_branch_checkout_remote_repo_skip_clone( - remote_repo, cmp_branch, cmp_branch_local_branch_name + remote_repo, cmp_branch, cmp_branch_local_branch_name, cmp_commit_sha ) - ref_config_dict = git_show_yaml_files(ref_branch, ref_git_repo) + if ref_commit_sha: + ref_config_dict = git_show_yaml_files(ref_commit_sha, ref_git_repo) + else: + ref_config_dict = git_show_yaml_files(ref_branch, ref_git_repo) # To show the files from remote repo, you need to append the remote name # before the branch - config_dict = git_show_yaml_files(cmp_branch_local_branch_name, remote_git_repo) + if cmp_commit_sha: + config_dict = git_show_yaml_files(cmp_commit_sha, remote_git_repo) + else: + config_dict = git_show_yaml_files(cmp_branch_local_branch_name, remote_git_repo) final_result = diff_config(ref_config_dict, config_dict) @@ -483,8 +519,18 @@ def diff_branch_remote_repo( ref_git_repo.close() remote_git_repo.close() else: - ref_repo_tmp_dir = sparse_branch_checkout(ref_repo, ref_branch) - cmp_repo_tmp_dir = sparse_branch_checkout(remote_repo, cmp_branch) + if ref_commit_sha: + ref_repo_tmp_dir = sparse_branch_checkout( + ref_repo, ref_branch, commit_sha=ref_commit_sha + ) + else: + ref_repo_tmp_dir = sparse_branch_checkout(ref_repo, ref_branch) + if cmp_commit_sha: + cmp_repo_tmp_dir = sparse_branch_checkout( + remote_repo, cmp_branch, commit_sha=cmp_commit_sha + ) + else: + cmp_repo_tmp_dir = sparse_branch_checkout(remote_repo, cmp_branch) ref_config_dict = load_config_yaml_files(Path(ref_repo_tmp_dir.name)) config_dict = load_config_yaml_files(Path(cmp_repo_tmp_dir.name)) final_result = diff_config(ref_config_dict, config_dict) @@ -573,6 +619,12 @@ def main(): parser_diff_branch_remote_repo.add_argument( "--cmp-branch", required=True, help="the branch to compare against" ) + parser_diff_branch_remote_repo.add_argument( + "--ref-commit-sha", required=False, help="the reference commit" + ) + parser_diff_branch_remote_repo.add_argument( + "--cmp-commit-sha", required=False, help="the commit to compare against" + ) parser_diff_branch_remote_repo.add_argument( "--skip-clone", action="store_true", @@ -590,6 +642,12 @@ def main(): if args.skip_clone and args.ref_repo != CEPH_UPSTREAM_REMOTE_URL: parser.error("--ref-repo cannot be set if --skip-clone is used.") + if args.ref_commit_sha and not args.ref_branch: + parser.error("--ref-commit-sha needs --ref-branch to be set.") + + if args.cmp_commit_sha and not args.cmp_branch: + parser.error("--cmp-commit-sha needs --cmp-branch to be set.") + if args.mode == "diff-branch": diff_branch(args.ref_repo, args.ref_branch, args.cmp_branch, args.skip_clone, args.format) @@ -602,6 +660,8 @@ def main(): args.ref_branch, args.remote_repo, args.cmp_branch, + args.ref_commit_sha, + args.cmp_commit_sha, args.skip_clone, args.format, )