src/script/config_diff.py: add support for `ref-commit-sha` and `cmp-commit-sha`...

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 <naveennaidu479@gmail.com>

diff --git a/src/script/config-diff/README.md b/src/script/config-diff/README.md
index 2ab954975f5fbf5b114f36a7876e01ce635e0de3..8274f7b30626b6209f468a6b1f680b9102134a30 100644 (file)
--- a/src/script/config-diff/README.md
+++ b/src/script/config-diff/README.md
@@ -52,6 +52,8 @@ python3 config_diff.py <mode> [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 1202dee661f8b68fbf57ab31e581d4981f1296a7..7d6dd9943fa4321d43f7a2292caf65238e70c8b6 100644 (file)
--- 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,
         )