From: Patrick Donnelly Date: Thu, 30 Apr 2026 18:36:30 +0000 (-0700) Subject: script/ptl-tool: introduce interactive backport parity and conflict verification X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=922f0c62f5e3cb7a45c0fd1130389da78dbb712f;p=ceph.git script/ptl-tool: introduce interactive backport parity and conflict verification This patch significantly expands `ptl-tool.py` to automate and improve the backport review process. It adds robust interactive checks to verify that all commits from an original PR are correctly represented in a backport PR. Key additions: * Commit Parity Verification: Analyzes the local Git DAG to ensure all cherry-picked commits map properly to the original main PRs, generating a visual map of the commits. * Conflict Simulation: Creates temporary, detached worktrees to dry-run the cherry-pick sequence, verifying conflict resolutions dynamically. * Automated GitHub Reviews: Enables maintainers to open an editor, preview markdown drafts, and post `REQUEST_CHANGES` reviews detailing missing commits or backport deviations directly to the pull request. * Interactive Diffing: Provides file-specific 3-pane patch comparisons (range-diffs, original patch, backport patch) within the terminal editor during conflict investigations. * New CLI Flags: Introduces `--audit`, `--always-fetch`, `--skip-conflict-check`, and `--release-merge` for greater control over the script's behavior. Assisted-by: Gemini Signed-off-by: Patrick Donnelly --- diff --git a/src/script/ptl-tool.py b/src/script/ptl-tool.py index 56303c98a738..cd9a9e745a67 100755 --- a/src/script/ptl-tool.py +++ b/src/script/ptl-tool.py @@ -144,6 +144,8 @@ import argparse import datetime +import difflib +import hashlib from getpass import getuser import itertools import json @@ -151,8 +153,12 @@ import logging import os import re import signal +import subprocess import sys +import tempfile import textwrap +import threading +import webbrowser MISSING_DEPS = [] @@ -220,6 +226,8 @@ SPECIAL_BRANCHES = ('main', 'luminous', 'jewel', 'HEAD') TEST_BRANCH = os.getenv("PTL_TOOL_TEST_BRANCH", "wip-{user}-testing-%Y%m%d.%H%M%S") USER = os.getenv("PTL_TOOL_USER", getuser()) +SANDBOX_CFG = ['rerere.enabled=false', 'commit.gpgSign=false', 'core.hooksPath=/dev/null'] + log = logging.getLogger(__name__) log.addHandler(logging.StreamHandler()) log.setLevel(logging.INFO) @@ -274,6 +282,38 @@ def get(session, url, params=None, paging=True): link = response.headers.get('link', None) page += 1 +def resolve_ref(G, ref, remote_url, always_fetch=False): + """ + Attempts to resolve a git reference locally. If it fails, or if + always_fetch is True, it fetches the ref from the remote. + Returns the git.Commit object. + """ + if not always_fetch: + try: + c = G.commit(ref) + log.debug(f"Resolved {ref} locally to {c.hexsha[:8]}.") + return c + except (git.exc.BadName, ValueError): + pass + + # Check typical remote prefix fallbacks for branches + if not re.match(r'^[0-9a-f]{40}$', ref) and not ref.startswith('refs/'): + for local_alias in [f"upstream/{ref}", f"origin/{ref}"]: + try: + c = G.commit(local_alias) + log.debug(f"Resolved {ref} locally via {local_alias} to {c.hexsha[:8]}.") + return c + except (git.exc.BadName, ValueError): + pass + + log.info(f"Fetching {ref} from {remote_url}") + try: + G.git.fetch(remote_url, ref) + return G.commit("FETCH_HEAD") + except git.exc.GitCommandError as e: + log.error(f"Could not fetch {ref} from {remote_url}: {e}") + sys.exit(1) + def get_credits(session, pr, pr_req): comments = [pr_req] @@ -330,6 +370,500 @@ def get_credits(session, pr, pr_req): return "\n".join(credits), new_new_contributors +def format_parity_row(left_sha, left_msg, right_sha, right_msg, is_missing=False, is_extra=False, right_prefix=""): + """Helper to format visualizer rows.""" + if is_missing: + left_col = "\033[91m<<<< MISSING IN BACKPORT >>>>\033[0m" + visible_left_len = 29 + else: + l_msg = (left_msg[:25] + '...') if len(left_msg) > 28 else left_msg + left_col = f"[ {left_sha[:8]} ] {l_msg}" + visible_left_len = len(left_col) + + if is_extra: + right_col = "\033[93m<<<< EXTRA IN BACKPORT >>>>\033[0m" + else: + r_msg = (right_msg[:20] + '...') if len(right_msg) > 23 else right_msg + right_col = f"{right_prefix}[ {right_sha[:8]} ] {r_msg}" + + padding = max(1, 47 - visible_left_len) + return f"{left_col}{' ' * padding}{right_col}" + +def make_pipe(content, fds_to_close, threads): + """ + Helper to create a pipe, write content to it in a background thread, + and return the path to the file descriptor. + """ + r, w = os.pipe() + fds_to_close.append(r) + + def writer(): + try: + with os.fdopen(w, 'wb') as pipe_w: + pipe_w.write(content.encode('utf-8')) + except Exception: + pass + + t = threading.Thread(target=writer) + t.start() + threads.append(t) + return f"/dev/fd/{r}", r + +def post_draft_review(session, pr, initial_text, base=None): + """ + Opens an editor with the draft text, previews it, and prompts the user to + post it as a REQUEST_CHANGES review. + """ + if base: + rfa_msg = f"\n\nWhen you are ready for this to be reviewed and QA'd again, please add the `pdonnell-{base}-RFA` label to this PR." + if rfa_msg not in initial_text: + initial_text += rfa_msg + + editor = os.environ.get('EDITOR', 'vim') + with tempfile.NamedTemporaryFile(mode='w+', suffix='.md', delete=False) as tf: + tf.write(initial_text) + tf_path = tf.name + + try: + while True: + subprocess.run([editor, tf_path]) + with open(tf_path, 'r', encoding='utf-8') as f_read: + final_text = f_read.read().strip() + + print("\n" + "="*80) + print("DRAFT REVIEW PREVIEW (REQUEST_CHANGES):") + print("-" * 80) + print(final_text) + print("="*80 + "\n") + + confirm = input(f"Post this review requesting changes to PR #{pr}? [y/e/N] (y=post, e=edit again, n=cancel): ").strip().lower() + if confirm == 'y': + endpoint = f"https://api.github.com/repos/{BASE_PROJECT}/{BASE_REPO}/pulls/{pr}/reviews" + r = session.post(endpoint, auth=gitauth(), json={'body': final_text, 'event': 'REQUEST_CHANGES'}) + if r.status_code in (200, 201): + log.info(f"Successfully posted review to PR #{pr}") + return True + else: + log.error(f"Failed to post review: {r.status_code} {r.text}") + return False + elif confirm == 'e': + continue + else: + print("Review cancelled.") + return False + finally: + os.unlink(tf_path) + +def verify_commit_parity(G, session, pr, pr_commits, base): + """ + Analyzes the local Git DAG to ensure all commits from the original main PRs + are present in the backport PR. + """ + log.info("Verifying commit parity with original PR(s) locally...") + bp_cherry_picks = [] + invalid_format_commits = [] + cp_regex = re.compile(r"\(cherry picked from commit ([0-9a-f]{40})\)") + for commit in pr_commits: + m = cp_regex.search(commit.message) + is_cherry_pick = bool(m) + if not is_cherry_pick and not commit.summary.startswith(f"{base}:"): + invalid_format_commits.append(commit) + + if is_cherry_pick: + bp_cherry_picks.append((commit, m.group(1))) + + missing_commits = [] + found_prs = set() + bp_orig_shas = set(orig_sha for _, orig_sha in bp_cherry_picks) + analyzed_merges = set() + pr_mapping = {} + bp_commits_mapped = set() + + valid_ref = None + for ref in ['main', 'origin/main', 'upstream/main']: + try: + G.git.rev_parse('--verify', ref) + valid_ref = ref + break + except git.exc.GitCommandError: + pass + + if valid_ref: + for commit, orig_sha in bp_cherry_picks: + try: + # Find merge commit using intersection of ancestry-path and first-parent + ancestry = G.git.rev_list(f"{orig_sha}..{valid_ref}", '--ancestry-path').splitlines() + first_parent = G.git.rev_list(f"{orig_sha}..{valid_ref}", '--first-parent').splitlines() + + first_parent_set = set(first_parent) + merge_sha = None + for c in reversed(ancestry): + if c in first_parent_set: + merge_sha = c + break + + if merge_sha and merge_sha not in analyzed_merges: + analyzed_merges.add(merge_sha) + # Extract the original PR commits using merge parents (merge^1..merge^2) + orig_pr_commits = G.git.rev_list(f"{merge_sha}^1..{merge_sha}^2").splitlines() + orig_pr_commits.reverse() # chronological + + merge_msg = G.commit(merge_sha).summary + m_pr = re.search(r'(?:Merge PR|Merge pull request) #(\d+)', merge_msg, re.IGNORECASE) + pr_name = f"PR #{m_pr.group(1)}" if m_pr else f"Merge {merge_sha[:8]}" + found_prs.add(pr_name) + pr_mapping[pr_name] = [] + + for o_commit_sha in orig_pr_commits: + o_summary = G.commit(o_commit_sha).summary + bp_match = next((c for c, o_sha in bp_cherry_picks if o_sha == o_commit_sha), None) + + pr_mapping[pr_name].append({ + 'o_sha': o_commit_sha, + 'o_summary': o_summary, + 'bp_commit': bp_match, + 'm_sha': merge_sha + }) + + if bp_match: + bp_commits_mapped.add(bp_match.hexsha) + else: + missing_commits.append((pr_name, o_commit_sha, o_summary, merge_sha)) + except git.exc.GitCommandError: + log.debug(f"Local DAG traversal skipped/failed for {orig_sha[:8]}") + else: + log.warning("Could not find local main/upstream ref to perform DAG parity check.") + + if found_prs: + log.info(f"Original PRs identified in this backport: {', '.join(found_prs)}") + if len(found_prs) > 1: + print("\033[91m" + "="*80) + print("WARNING: Multiple original PRs detected in this backport!") + print("Normally we expect exactly one main PR per backport.") + print("Detected: " + ", ".join(found_prs)) + print("="*80 + "\033[0m") + + # Print Visualizer UI + visualizer_text = "" + visualizer_lines = [] + if found_prs or pr_commits: + visualizer_lines.append("=" * 80) + visualizer_lines.append("COMMIT PARITY VISUALIZER") + visualizer_lines.append("=" * 80) + + bp_to_source = {} + for pr_name, commit_list in pr_mapping.items(): + for item in commit_list: + if item['bp_commit']: + bp_to_source[item['bp_commit'].hexsha] = (pr_name, item) + + unprinted_missing = {} + for pr_name, o_sha, o_summary, m_sha in missing_commits: + if pr_name not in unprinted_missing: + unprinted_missing[pr_name] = [] + unprinted_missing[pr_name].append((o_sha, o_summary)) + + visualizer_lines.append(f"BACKPORT PR #{pr}".ljust(47) + "SOURCE PR / STATUS") + visualizer_lines.append("-" * 80) + + # Print all backport commits exactly in chronological order + current_pr = None + for bp_c in pr_commits: + if bp_c.hexsha in bp_to_source: + pr_name, item = bp_to_source[bp_c.hexsha] + + # If PR block changes, flush the missing commits for the previous block + if current_pr is not None and pr_name != current_pr: + if current_pr in unprinted_missing: + for m_sha, m_summary in unprinted_missing[current_pr]: + prefix = " " * (len(current_pr) + 1) + visualizer_lines.append(format_parity_row(None, None, m_sha, m_summary, is_missing=True, right_prefix=prefix)) + del unprinted_missing[current_pr] + visualizer_lines.append("") # Add visual grouping space between different PRs + + prefix = f"{pr_name} " if pr_name != current_pr else " " * (len(pr_name) + 1) + current_pr = pr_name + visualizer_lines.append(format_parity_row(bp_c.hexsha, bp_c.summary, item['o_sha'], item['o_summary'], right_prefix=prefix)) + else: + if current_pr is not None: + if current_pr in unprinted_missing: + for m_sha, m_summary in unprinted_missing[current_pr]: + prefix = " " * (len(current_pr) + 1) + visualizer_lines.append(format_parity_row(None, None, m_sha, m_summary, is_missing=True, right_prefix=prefix)) + del unprinted_missing[current_pr] + visualizer_lines.append("") + current_pr = None # Reset so the next PR prints its name + visualizer_lines.append(format_parity_row(bp_c.hexsha, bp_c.summary, None, None, is_extra=True)) + + # Flush any remaining missing commits for the last processed PR block + if current_pr is not None and current_pr in unprinted_missing: + for m_sha, m_summary in unprinted_missing[current_pr]: + prefix = " " * (len(current_pr) + 1) + visualizer_lines.append(format_parity_row(None, None, m_sha, m_summary, is_missing=True, right_prefix=prefix)) + del unprinted_missing[current_pr] + + # Flush any missing commits for PRs that had NO commits successfully mapped + for pr_name, missing_list in unprinted_missing.items(): + visualizer_lines.append("") + first = True + for m_sha, m_summary in missing_list: + prefix = f"{pr_name} " if first else " " * (len(pr_name) + 1) + visualizer_lines.append(format_parity_row(None, None, m_sha, m_summary, is_missing=True, right_prefix=prefix)) + first = False + + visualizer_lines.append("=" * 80) + + visualizer_text = "\n".join(visualizer_lines) + print(visualizer_text) + + for commit in invalid_format_commits: + log.error(f"Commit {commit.hexsha[:8]} invalid format. Must be cherry-pick or start with '{base}:'") + ans = input("Do you want to allow this commit anyway? [y/N] ") + if ans.lower() != 'y': + sys.exit(1) + + visualizer_clean = re.sub(r'\033\[[0-9;]*m', '', visualizer_text) if visualizer_text else "" + if missing_commits: + # Generate markdown comment draft + md_text = f"**Automated Backport Parity Review**\n\n" + if len(found_prs) > 1: + md_text += f":warning: **Multiple Original PRs Detected:** {', '.join(found_prs)}\n\n" + md_text += "Discrepancies detected between the backport PR and the original source PR(s). Please review the mapping below:\n\n" + md_text += f"```text\n{visualizer_clean}\n```" + + while True: + ans = input("Parity mismatch! [p]roceed, [o]pen browser to investigate, [r]eview PR (request changes), [q]uit: ").strip().lower() + if ans == 'p': + break + elif ans == 'q': + log.error("Rejecting PR due to incomplete backport.") + sys.exit(1) + elif ans == 'r': + if post_draft_review(session, pr, md_text, base=base): + log.error("Rejecting PR due to incomplete backport after posting review.") + sys.exit(1) + elif ans == 'o': + first_url = True + for pr_name, o_sha, o_summary, m_sha in missing_commits: + urls_to_open = [] + m_pr = re.search(r'#(\d+)', pr_name) + if m_pr: + urls_to_open.append(f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{m_pr.group(1)}") + urls_to_open.append(f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{m_sha}") + urls_to_open.append(f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{o_sha}") + + for url in urls_to_open: + if first_url: + webbrowser.open_new(url) + first_url = False + else: + webbrowser.open_new_tab(url) + print("Opened URLs in a new browser window.") + else: + print("Invalid choice. Please enter p, o, r, or q.") + elif analyzed_merges: + print("\033[92mCommit parity check passed! All upstream commits from identified PRs are present.\033[0m") + + if len(found_prs) > 1: + while True: + ans = input("Multiple original PRs detected! Do you want to post a review requesting documentation? [y/N/o] (y=yes, n=no, o=open PRs in browser): ").strip().lower() + if ans == 'o': + url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{pr}" + webbrowser.open_new(url) + print(f"Opened {url} in browser.") + for pr_str in found_prs: + m_pr = re.search(r'#(\d+)', pr_str) + if m_pr: + orig_url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{m_pr.group(1)}" + webbrowser.open_new_tab(orig_url) + print(f"Opened {orig_url} in browser.") + elif ans == 'y': + md_text = f"**Automated Backport Parity Review - Multiple PRs Detected**\n\n" + md_text += f"This backport appears to pull commits from multiple `main` PRs including: {', '.join(found_prs)}.\n\n" + md_text += "This must be explicitly documented in the backport PR description. Furthermore, each backport tracker ticket associated with these `main` PRs must be linked to this PR.\n\n" + md_text += f"**Commit Parity Visualizer:**\n```text\n{visualizer_clean}\n```\n" + if post_draft_review(session, pr, md_text, base=base): + log.error("Rejecting PR pending documentation of multiple original PRs.") + sys.exit(1) + break + else: + break + + return visualizer_text + +def simulate_conflict_resolution(G, session, pr, pr_commits, base, always_fetch, visualizer_text): + """ + Creates a temporary worktree to dry-run the cherry-pick sequence, verifying + conflict resolutions dynamically. + """ + wt_dir = tempfile.mkdtemp(prefix="ptl-worktree-") + try: + G.git.worktree('add', '--detach', wt_dir, base) + wt_repo = git.Repo(wt_dir) + + cp_regex = re.compile(r"\(cherry picked from commit ([0-9a-f]{40})\)") + auto_approve_conflicts = False + first_conflict = True + + for commit in pr_commits: + if len(commit.parents) > 1: + log.error(f"Commit {commit.hexsha[:8]} is a merge commit. Not allowed.") + sys.exit(1) + + m = cp_regex.search(commit.message) + is_cherry_pick = bool(m) + + if is_cherry_pick: + orig_sha = m.group(1) + log.info(f"Simulating cherry-pick of {orig_sha[:8]} for {commit.hexsha[:8]} ...") + c = resolve_ref(wt_repo, orig_sha, BASE_REMOTE_URL, always_fetch) + + has_conflict = False + clean_tree = None + try: + wt_repo.git(c=SANDBOX_CFG).cherry_pick(c.hexsha) + clean_tree = wt_repo.head.commit.tree.hexsha + wt_repo.git.reset('--hard', 'HEAD~1') + except git.exc.GitCommandError: + has_conflict = True + unmerged = wt_repo.git.diff('--name-only', '--diff-filter=U').splitlines() + wt_repo.git.cherry_pick('--abort') + + # Always apply the backport commit so the worktree matches the PR sequence + try: + wt_repo.git(c=SANDBOX_CFG).cherry_pick("--allow-empty", commit.hexsha) + except git.exc.GitCommandError: + log.error(f"Failed to apply backport commit {commit.hexsha[:8]}! The PR likely has conflicts with the base branch and needs a rebase.") + wt_repo.git.cherry_pick('--abort') + sys.exit(1) + + backport_tree = wt_repo.head.commit.tree.hexsha + + is_sneaky = (not has_conflict) and (clean_tree != backport_tree) + + if has_conflict or is_sneaky: + if auto_approve_conflicts: + log.info(f"Skipping approval and taking backport commit for {orig_sha[:8]}.") + continue + + if first_conflict: + ans = input(f"Conflict or unapproved deviation detected in {c.hexsha[:8]}. Do you want to interactively review this PR? [Y/n/m]\n(y = yes, n = auto-approve remaining, m = skip to merge) ") + first_conflict = False + if ans.lower() == 'm': + log.info("Skipping ahead to merge.") + return + elif ans.lower() == 'n': + log.info("Auto-approving and skipping interactive checks for this PR.") + auto_approve_conflicts = True + continue + + editor = os.environ.get('EDITOR', 'vim') + if has_conflict: + log.warning(f"Opening {editor} to inspect conflict in {c.hexsha[:8]} ...") + else: + log.warning(f"Opening {editor} to inspect unexplained deviation in clean cherry-pick {c.hexsha[:8]} ...") + unmerged = wt_repo.git.diff('--name-only', clean_tree, backport_tree).splitlines() + for f in unmerged: + log.debug("Unmerged: %s", f) + if unmerged: + # Generate a diff of the commit messages + orig_msg = c.message.splitlines(keepends=True) + bp_msg = commit.message.splitlines(keepends=True) + msg_diff = "".join(difflib.unified_diff(orig_msg, bp_msg, + fromfile=f'Original ({c.hexsha[:8]})', + tofile=f'Backport ({commit.hexsha[:8]})')) + + # Generate git range-diff to highlight changes between the original patch and backport + try: + diff = wt_repo.git.range_diff(f"{c.hexsha}^!", f"{commit.hexsha}^!") + except git.exc.GitCommandError as e: + log.warning(f"Could not generate range-diff: {e}") + diff = f"Could not generate range-diff:\n{e}" + + # Concatenate with a marker + diff = f"{msg_diff}\n" + "="*80 + "\nRANGE DIFF\n" + "="*80 + "\n\n" + diff + + # Present file-specific 3-pane view (range-diff, orig patch, backport patch) + for f in unmerged: + try: + file_diff = f"Conflict in {f}:\n\n" + diff + orig_patch = wt_repo.git.show(c.hexsha, "--", f) + bp_patch = wt_repo.git.show(commit.hexsha, "--", f) + + if orig_patch == "": + orig_patch = "(file not found or empty)" + if bp_patch == "": + bp_patch = "(file not found or empty)" + + fds_to_close = [] + threads = [] + + cmd = [editor] + if any(ed in editor for ed in ['vim', 'nvim', 'vi']): + cmd.append('-O') + + pass_fds = [] + if file_diff: + path, r_fd = make_pipe(file_diff, fds_to_close, threads) + cmd.append(path) + pass_fds.append(r_fd) + + path1, r_fd1 = make_pipe(orig_patch, fds_to_close, threads) + cmd.append(path1) + pass_fds.append(r_fd1) + + path2, r_fd2 = make_pipe(bp_patch, fds_to_close, threads) + cmd.append(path2) + pass_fds.append(r_fd2) + + subprocess.run(cmd, pass_fds=pass_fds) + + for t in threads: + t.join() + for fd in fds_to_close: + os.close(fd) + except git.exc.GitCommandError as e: + log.warning(f"Could not generate patch comparison for {f}: {e}") + + prompt_text = "Does the PR properly explain and resolve this conflict/deviation? [y/N/s/m]\n(y = next check, n = abort, s = skip remaining checks, m = skip to merge) " + ans = input(prompt_text) + if ans.lower() == 'm': + log.info("Skipping ahead to merge.") + return + elif ans.lower() == 's': + log.info("Skipping remaining checks for this PR.") + auto_approve_conflicts = True + elif ans.lower() == 'y': + pass # Already applied backport commit + else: + log.error("Rejecting PR due to undocumented/unapproved change. Preparing draft review...") + visualizer_clean = re.sub(r'\033\[[0-9;]*m', '', visualizer_text) if visualizer_text else "" + diff_text = diff if 'diff' in locals() else "No range diff available." + + md_text = "**Automated Backport Parity Review - Backport Deviation Alert**\n\n" + md_text += "A conflict or unapproved deviation was detected during the simulation of this backport, and the changes appear to be incorrect or undocumented. Please review the highlighted files.\n\n" + md_text += "**Affected File(s):**\n" + for f_name in unmerged: + f_hash = hashlib.sha256(f_name.encode('utf-8')).hexdigest() + md_text += f"* `{f_name}`\n" + md_text += f" * Original: https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{c.hexsha}#diff-{f_hash}\n" + md_text += f" * Backport: https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{commit.hexsha}#diff-{f_hash}\n" + + if visualizer_clean: + md_text += f"\n**Commit Parity Visualizer:**\n```text\n{visualizer_clean}\n```\n\n" + + md_text += f"**Range Diff:**\n
Click to expand\n\n```diff\n{diff_text}\n```\n
\n\n" + + post_draft_review(session, pr, md_text) + sys.exit(1) + else: + log.info(f"Applying branch-specific commit {commit.hexsha[:8]} ...") + wt_repo.git(c=SANDBOX_CFG).cherry_pick("--allow-empty", commit.hexsha) + log.info("Verification of backport cherry-pick for PR #%d is complete.", pr) + finally: + log.info("Removing temporary worktree `%s'", wt_dir) + G.git.worktree('remove', '--force', wt_dir) + def build_branch(args): base = args.base branch = datetime.datetime.utcnow().strftime(args.branch).format(user=USER) @@ -357,9 +891,8 @@ def build_branch(args): G = git.Repo(args.git) try: - log.info("Fetching .githubmap from %s main branch", BASE_REMOTE_URL) - G.git.fetch(BASE_REMOTE_URL, "main") - githubmap_content = G.git.show("FETCH_HEAD:.githubmap") + c = resolve_ref(G, 'main', BASE_REMOTE_URL, args.always_fetch) + githubmap_content = G.git.show(f"{c.hexsha}:.githubmap") comment = re.compile(r"\s*#") patt = re.compile(r"([\w-]+)\s+(.*)") for line in githubmap_content.splitlines(): @@ -396,19 +929,39 @@ def build_branch(args): prs.append(n) log.info("Will merge PRs: {}".format(prs)) + # PRE-FLIGHT: Auto-detect base from the first PR if necessary and cache responses + pr_responses = {} + for pr in prs: + log.info("Fetching information for PR #%d", pr) + endpoint = f"https://api.github.com/repos/{BASE_PROJECT}/{BASE_REPO}/pulls/{pr}" + pr_responses[pr] = next(get(session, endpoint, paging=False)) + + if prs and base is None: + for pr in prs: + detected_base = pr_responses[pr].get("base", {}).get("ref") + log.debug("Detected base for PR #%d as `%s'", pr, detected_base) + if detected_base: + log.info(f"Auto-detected target base from PR #{pr}: {detected_base}") + base = detected_base + if args.merge_branch_name is False: + merge_branch_name = detected_base + else: + if base != detected_base: + raise SystemExit("base of each PR is not equal, use hard-coded --base") + if base == 'HEAD': log.info("Branch base is HEAD; not checking out!") else: log.info("Detaching HEAD onto base: {}".format(base)) try: - G.git.fetch(BASE_REMOTE_URL, base) - # So we know that we're not on an old test branch, detach HEAD onto ref: - c = G.commit('FETCH_HEAD') + c = resolve_ref(G, base, BASE_REMOTE_URL, args.always_fetch) + G.git.checkout(c) except git.exc.GitCommandError: - log.debug("could not fetch %s from %s", base, BASE_REMOTE_URL) log.info(f"Trying to checkout uninterpreted base {base}") c = G.commit(base) - G.git.checkout(c) + G.git.checkout(c) + + # So we know that we're not on an old test branch, detach HEAD onto ref: assert G.head.is_detached qa_tracker_description = [] @@ -418,22 +971,68 @@ def build_branch(args): log.info("Merging PR #{pr}".format(pr=pr)) remote_ref = "refs/pull/{pr}/head".format(pr=pr) - try: - G.git.fetch(BASE_REMOTE_URL, remote_ref) - except git.exc.GitCommandError: - log.error("could not fetch %s from %s", remote_ref, BASE_REMOTE_URL) - sys.exit(1) - tip = G.commit("FETCH_HEAD") + remote_sha1 = pr_responses[pr].get('head', {}).get('sha') + + ref_to_fetch = remote_sha1 if remote_sha1 else remote_ref + tip = resolve_ref(G, ref_to_fetch, BASE_REMOTE_URL, args.always_fetch) + log.info("Now have head for PR #%d: %s", pr, str(tip)) - endpoint = f"https://api.github.com/repos/{BASE_PROJECT}/{BASE_REPO}/pulls/{pr}" - response = next(get(session, endpoint, paging=False)) + response = pr_responses[pr] + + log.info("Performing trivial merge check for PR #%d...", pr) + wt_dir = tempfile.mkdtemp(prefix="ptl-merge-check-") + has_base_conflicts = False + try: + G.git.worktree('add', '--detach', wt_dir, G.head.commit) + wt_repo = git.Repo(wt_dir) + try: + wt_repo.git(c=SANDBOX_CFG).merge(tip.hexsha, '--no-commit', '--no-ff') + except git.exc.GitCommandError: + has_base_conflicts = True + finally: + try: + wt_repo.git.merge('--abort') + except git.exc.GitCommandError: + pass + finally: + G.git.worktree('remove', '--force', wt_dir) + + if has_base_conflicts: + log.error(f"PR #{pr} has conflicts with the target base branch.") + while True: + ans = input(f"PR #{pr} needs a rebase! Do you want to post a review requesting a rebase? [y/N/o] (y=yes, n=abort script, o=open PR in browser): ").strip().lower() + if ans == 'o': + url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{pr}" + webbrowser.open_new(url) + print(f"Opened {url} in browser.") + elif ans == 'y': + md_text = "**Automated PR Review - Rebase Required**\n\n" + md_text += f"This PR currently has merge conflicts with the target base branch. Please rebase and resolve the conflicts." + if post_draft_review(session, pr, md_text): + log.error("Rejecting PR pending rebase.") + sys.exit(1) + elif ans == 'n' or ans == '': + log.error(f"Aborting script due to unmergeable PR #{pr}.") + sys.exit(1) qa_tracker_description.append(f'* "PR #{pr}":{response["html_url"]} -- {response["title"].strip()}') message = "Merge PR #%d into %s\n\n* %s:\n" % (pr, merge_branch_name, remote_ref) - for commit in G.iter_commits(rev="HEAD.."+str(tip)): - message = message + ("\t%s\n" % commit.message.split('\n', 1)[0]) + pr_commits = list(G.iter_commits(rev="HEAD.."+str(tip))) + pr_commits.reverse() # chronological order for simulation + + if base != 'main': + visualizer_text = verify_commit_parity(G, session, pr, pr_commits, base) + if not args.skip_conflict_check: + simulate_conflict_resolution(G, session, pr, pr_commits, base, args.always_fetch, visualizer_text) + + if args.audit: + log.info(f"Audit of PR #{pr} complete. Skipping merge.") + continue + + for commit in reversed(pr_commits): # back to reverse-chronological for the message + message = message + ("\t%s\n" % commit.summary) # Get tracker issues / bzs cited so the PTL can do updates short = commit.hexsha[:8] for m in BZ_MATCH.finditer(commit.message): @@ -442,6 +1041,7 @@ def build_branch(args): log.info("[ {sha1} ] Ceph tracker cited: {cite}".format(sha1=short, cite=m.group(1))) message = message + "\n" + if args.credits: (addendum, new_contributors) = get_credits(session, pr, response) message += addendum @@ -466,6 +1066,10 @@ def build_branch(args): sys.exit(1) log.info("Labeled PR #{pr} {label}".format(pr=pr, label=label)) + if args.audit: + log.info("Audit complete. Exiting without modifying branches or Redmine.") + return + message = """ ceph-dev-pipeline: configure @@ -596,7 +1200,10 @@ def build_branch(args): endpoint = f"https://api.github.com/repos/{BASE_PROJECT}/{BASE_REPO}/issues/{pr}/comments" body = f"This PR is under test in [{issue.url}]({issue.url})." r = session.post(endpoint, auth=gitauth(), data=json.dumps({'body':body})) - log.debug(f"= {r}") + if r.status_code == 201: + log.info(f"Successfully posted comment to PR #{pr}") + else: + log.error(f"Failed to post comment: {r.status_code} {r.text}") class SplitCommaAppendAction(argparse.Action): """ @@ -623,16 +1230,9 @@ class SplitCommaAppendAction(argparse.Action): def main(): parser = argparse.ArgumentParser(description="Ceph PTL tool") - default_base = 'main' default_branch = TEST_BRANCH default_label = '' - if len(sys.argv) > 1 and sys.argv[1] in SPECIAL_BRANCHES: - argv = sys.argv[2:] - default_branch = 'HEAD' # Leave HEAD detached - default_base = default_branch - default_label = False - else: - argv = sys.argv[1:] + argv = sys.argv[1:] group = parser.add_argument_group('General Options') group.add_argument('--debug', dest='debug', action='store_true', help='turn debugging on') @@ -643,8 +1243,10 @@ def main(): group.add_argument('--pr-label', dest='pr_label', action='store', help='source PRs to merge via label') group = parser.add_argument_group('Branch Control Options') - group.add_argument('--base', dest='base', action='store', default=default_base, help='base for branch') + group.add_argument('--always-fetch', dest='always_fetch', action='store_true', help='always fetch commits from remote (bypass local cache)') + group.add_argument('--base', dest='base', action='store', help='base for branch') group.add_argument('--branch', dest='branch', action='store', default=default_branch, help='branch to create ("HEAD" leaves HEAD detached; i.e. no branch is made)') + group.add_argument('--release-merge', dest='release_merge', action='store_true', help='enable release merge behavior (implies --branch HEAD)') group.add_argument('--branch-name-append', dest='branch_append', action='store', help='append string to branch name') group.add_argument('--branch-release', dest='branch_release', action='store', help='release name to embed in branch (for shaman)') group.add_argument('--merge-branch-name', dest='merge_branch_name', action='store', default=False, help='name of the branch for merge messages') @@ -670,6 +1272,10 @@ def main(): group.add_argument('--no-push-ci', dest='no_push_ci', action='store_true', help='don\'t push branch to ceph-ci repo (when making QA tickets)') group.add_argument('--push-ci', dest='push_ci', action='store_true', help='push branch and tag to CI repository (even when not making QA tickets)') + group = parser.add_argument_group('Backport Verification') + group.add_argument('--audit', dest='audit', action='store_true', help='run parity and conflict simulations without merging or modifying branches') + group.add_argument('--skip-conflict-check', dest='skip_conflict_check', action='store_true', help='skip conflict resolution simulation') + def parse_pr(value): m = re.search(r'/pull/(\d+)', value) if m: @@ -694,6 +1300,9 @@ def main(): if args.debug_build: args.flavors.add('debug') + if args.release_merge: + args.branch = 'HEAD' + if not GITHUB_TOKEN: log.error("Missing GitHub Personal Access Token.") log.error("Please create a file at ~/.github_token containing your token,")