-
Notifications
You must be signed in to change notification settings - Fork 1
[RR] Add Interactive settings and Minor Release Rebase #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mainline
Are you sure you want to change the base?
Changes from all commits
94bc41e
c28c53e
943ed08
c4ded0c
8c9baa6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,8 @@ | |||||||
|
|
||||||||
| import git | ||||||||
|
|
||||||||
| from ciq_helpers import get_backport_commit_data | ||||||||
|
|
||||||||
| FIPS_PROTECTED_DIRECTORIES = [ | ||||||||
| b"arch/x86/crypto/", | ||||||||
| b"crypto/asymmetric_keys/", | ||||||||
|
|
@@ -25,7 +27,53 @@ def find_common_tag(old_tags, new_tags): | |||||||
| return None | ||||||||
|
|
||||||||
|
|
||||||||
| def get_branch_tag_sha_list(repo, branch): | ||||||||
| def get_commit_maps_from_backport_data(repo_path, branch, common_tag): | ||||||||
| """Get commit maps using get_backport_commit_data from ciq_helpers. | ||||||||
|
|
||||||||
| This function properly parses all commits and extracts upstream references | ||||||||
| from 'commit <sha>' lines in commit bodies. This ensures we correctly identify | ||||||||
| duplicates even when different CIQ commits reference the same upstream commit. | ||||||||
|
|
||||||||
| Returns: | ||||||||
| commit_map: dict mapping CIQ commit SHA -> upstream commit SHA (or "" if no upstream) | ||||||||
| commit_map_rev: dict mapping upstream commit SHA -> CIQ commit SHA | ||||||||
| """ | ||||||||
| # get_backport_commit_data returns: | ||||||||
| # { "upstream_sha": { "repo_commit": "ciq_sha", "upstream_subject": "...", ... } } | ||||||||
| backport_data, success = get_backport_commit_data( | ||||||||
| repo_path, branch, common_tag.decode() if isinstance(common_tag, bytes) else common_tag, allow_duplicates=True | ||||||||
| ) | ||||||||
|
|
||||||||
| if not success: | ||||||||
| print("[rolling release update] WARNING: Duplicate upstream commits detected in backport data") | ||||||||
| print("[rolling release update] Continuing with allow_duplicates=True") | ||||||||
|
|
||||||||
| # Transform to the format expected by the rest of the script | ||||||||
| commit_map = {} | ||||||||
| commit_map_rev = {} | ||||||||
|
|
||||||||
| for upstream_sha, data in backport_data.items(): | ||||||||
| ciq_sha = data["repo_commit"] | ||||||||
| commit_map[ciq_sha] = upstream_sha | ||||||||
| commit_map_rev[upstream_sha] = ciq_sha | ||||||||
|
|
||||||||
| # Also get all commits (including those without upstream references) | ||||||||
| # to ensure we have a complete list | ||||||||
| repo = git.Repo(repo_path) | ||||||||
| repo.git.checkout(branch) | ||||||||
| common_tag_str = common_tag.decode() if isinstance(common_tag, bytes) else common_tag | ||||||||
| all_commits = repo.git.log("--pretty=%H", f"{common_tag_str}..HEAD").split("\n") | ||||||||
|
|
||||||||
| # Add commits without upstream references (CIQ-specific commits) | ||||||||
| for commit_sha in all_commits: | ||||||||
| commit_sha = commit_sha.strip() | ||||||||
| if commit_sha and commit_sha not in commit_map: | ||||||||
| commit_map[commit_sha] = "" | ||||||||
|
|
||||||||
| return commit_map, commit_map_rev | ||||||||
|
Comment on lines
+43
to
+73
|
||||||||
|
|
||||||||
|
|
||||||||
| def get_branch_tag_sha_list(repo, branch, minor_version=False): | ||||||||
| print("[rolling release update] Checking out branch: ", branch) | ||||||||
| repo.git.checkout(branch) | ||||||||
| results = subprocess.run( | ||||||||
|
|
@@ -37,8 +85,17 @@ def get_branch_tag_sha_list(repo, branch): | |||||||
|
|
||||||||
| print("[rolling release update] Gathering all the RESF kernel Tags") | ||||||||
| tags = [] | ||||||||
| last_resf_tag = b"" | ||||||||
| for line in results.stdout.split(b"\n"): | ||||||||
| if b"tag: resf_kernel" in line: | ||||||||
| if DEBUG: | ||||||||
| print(line) | ||||||||
| tags.append(line.split(b" ")[0]) | ||||||||
| if last_resf_tag == b"": | ||||||||
| last_resf_tag = line.split(b" ")[0] | ||||||||
| if minor_version and b"tag: kernel-" in line: | ||||||||
| if DEBUG: | ||||||||
| print(line) | ||||||||
| tags.append(line.split(b" ")[0]) | ||||||||
|
|
||||||||
| # Print summary instead of all tags | ||||||||
|
|
@@ -48,7 +105,7 @@ def get_branch_tag_sha_list(repo, branch): | |||||||
| for line_tag in tags: | ||||||||
| print(f" {line_tag.decode()}") | ||||||||
|
|
||||||||
| return tags | ||||||||
| return tags, last_resf_tag | ||||||||
|
|
||||||||
|
|
||||||||
| def check_for_fips_protected_changes(repo, branch, common_tag): | ||||||||
|
|
@@ -142,10 +199,19 @@ def check_for_fips_protected_changes(repo, branch, common_tag): | |||||||
| parser.add_argument( | ||||||||
| "--verbose-git-show", help="When SHAs are detected for removal do the full git show <sha>", action="store_true" | ||||||||
| ) | ||||||||
| parser.add_argument( | ||||||||
| "--new-minor-version", | ||||||||
|
||||||||
| help="Do not stop at the RESF tags, continue down the CENTOS / ROCKY MAIN branch." | ||||||||
| " This is used for the new minor version releases", | ||||||||
| action="store_true", | ||||||||
| ) | ||||||||
| parser.add_argument( | ||||||||
| "--demo", help="DEMO mode, will make a new set of branches with demo_ prepended", action="store_true" | ||||||||
| ) | ||||||||
| parser.add_argument("--debug", help="Enable debug output", action="store_true") | ||||||||
| parser.add_argument( | ||||||||
| "--interactive", help="Interactive mode - pause on merge conflicts for user resolution", action="store_true" | ||||||||
| ) | ||||||||
| args = parser.parse_args() | ||||||||
|
|
||||||||
| if args.demo: | ||||||||
|
|
@@ -164,52 +230,53 @@ def check_for_fips_protected_changes(repo, branch, common_tag): | |||||||
| rolling_product = args.old_rolling_branch.split("/")[0] | ||||||||
| print("[rolling release update] Rolling Product: ", rolling_product) | ||||||||
|
|
||||||||
| old_rolling_branch_tags = get_branch_tag_sha_list(repo, args.old_rolling_branch) | ||||||||
| if args.new_minor_version: | ||||||||
| print("[rolling release update] New Minor Version: ", args.new_minor_version) | ||||||||
|
|
||||||||
| old_rolling_branch_tags, old_rolling_resf_tag_sha = get_branch_tag_sha_list( | ||||||||
| repo, args.old_rolling_branch, args.new_minor_version | ||||||||
| ) | ||||||||
| if DEBUG: | ||||||||
| print("[rolling release update] Old Rolling Branch Tags: ", old_rolling_branch_tags) | ||||||||
|
|
||||||||
| new_base_branch_tags = get_branch_tag_sha_list(repo, args.new_base_branch) | ||||||||
| new_base_branch_tags, new_base_resf_tag_sha = get_branch_tag_sha_list( | ||||||||
| repo, args.new_base_branch, args.new_minor_version | ||||||||
| ) | ||||||||
| if DEBUG: | ||||||||
| print("[rolling release update] New Base Branch Tags: ", new_base_branch_tags) | ||||||||
|
|
||||||||
| latest_resf_sha = find_common_tag(old_rolling_branch_tags, new_base_branch_tags) | ||||||||
| print("[rolling release update] Latest RESF tag sha: ", latest_resf_sha) | ||||||||
| print(repo.git.show('--pretty="%H %s"', "-s", latest_resf_sha.decode())) | ||||||||
|
|
||||||||
| print("[rolling release update] Checking for FIPS protected changes between the common tag and HEAD") | ||||||||
| shas_to_check = check_for_fips_protected_changes(repo, args.new_base_branch, latest_resf_sha) | ||||||||
| if shas_to_check and args.fips_override is False: | ||||||||
| for sha, dir in shas_to_check.items(): | ||||||||
| print(f"## Commit {sha.decode()}") | ||||||||
| print("'''") | ||||||||
| dir_list = [] | ||||||||
| for d in dir: | ||||||||
| dir_list.append(d.decode()) | ||||||||
| print(repo.git.show(sha.decode(), dir_list)) | ||||||||
| print("'''") | ||||||||
| print("[rolling release update] FIPS protected changes found between the common tag and HEAD") | ||||||||
| print("[rolling release update] Please Contact the CIQ FIPS / Security team for further instructions") | ||||||||
| print("[rolling release update] Exiting") | ||||||||
| exit(1) | ||||||||
| common_sha = find_common_tag(old_rolling_branch_tags, new_base_branch_tags) | ||||||||
| print("[rolling release update] Common tag sha: ", common_sha) | ||||||||
| print(repo.git.show('--pretty="%H %s"', "-s", common_sha.decode())) | ||||||||
|
|
||||||||
| if "fips" in rolling_product: | ||||||||
| print("[rolling release update] Checking for FIPS protected changes between the common tag and HEAD") | ||||||||
| shas_to_check = check_for_fips_protected_changes(repo, args.new_base_branch, common_sha) | ||||||||
| if shas_to_check and args.fips_override is False: | ||||||||
| for sha, dir in shas_to_check.items(): | ||||||||
| print(f"## Commit {sha.decode()}") | ||||||||
| print("'''") | ||||||||
| dir_list = [] | ||||||||
| for d in dir: | ||||||||
| dir_list.append(d.decode()) | ||||||||
| print(repo.git.show(sha.decode(), dir_list)) | ||||||||
| print("'''") | ||||||||
| print("[rolling release update] FIPS protected changes found between the common tag and HEAD") | ||||||||
| print("[rolling release update] Please Contact the CIQ FIPS / Security team for further instructions") | ||||||||
| print("[rolling release update] Exiting") | ||||||||
| exit(1) | ||||||||
|
|
||||||||
| print("[rolling release update] Checking out old rolling branch: ", args.old_rolling_branch) | ||||||||
| repo.git.checkout(args.old_rolling_branch) | ||||||||
| print( | ||||||||
| "[rolling release update] Finding the CIQ Kernel and Associated Upstream commits between the last resf tag and HEAD" | ||||||||
| ) | ||||||||
| rolling_commit_map = {} | ||||||||
| rollint_commit_map_rev = {} | ||||||||
| rolling_commits = repo.git.log(f"{latest_resf_sha.decode()}..HEAD") | ||||||||
| for line in rolling_commits.split("\n"): | ||||||||
| if line.startswith("commit "): | ||||||||
| ciq_commit = line.split("commit ")[1] | ||||||||
| rolling_commit_map[ciq_commit] = "" | ||||||||
| if line.startswith(" commit "): | ||||||||
| upstream_commit = line.split(" commit ")[1] | ||||||||
| rolling_commit_map[ciq_commit] = upstream_commit | ||||||||
| rollint_commit_map_rev[upstream_commit] = ciq_commit | ||||||||
|
|
||||||||
| print("[rolling release update] Last RESF tag sha: ", latest_resf_sha) | ||||||||
| print(f"[rolling release update] Getting SHAS {old_rolling_resf_tag_sha.decode()}..HEAD") | ||||||||
| rolling_commit_map, rolling_commit_map_rev = get_commit_maps_from_backport_data( | ||||||||
| args.repo, args.old_rolling_branch, old_rolling_resf_tag_sha | ||||||||
| ) | ||||||||
|
|
||||||||
| print("[rolling release update] Last RESF tag sha: ", common_sha) | ||||||||
|
|
||||||||
| print(f"[rolling release update] Total commits in old branch: {len(rolling_commit_map)}") | ||||||||
| if DEBUG: | ||||||||
|
|
@@ -282,17 +349,9 @@ def check_for_fips_protected_changes(repo, branch, common_tag): | |||||||
| exit(1) | ||||||||
|
|
||||||||
| print("[rolling release update] Creating Map of all new commits from last rolling release fork") | ||||||||
| new_base_commit_map = {} | ||||||||
| new_base_commit_map_rev = {} | ||||||||
| new_base_commits = repo.git.log(f"{latest_resf_sha.decode()}..HEAD") | ||||||||
| for line in new_base_commits.split("\n"): | ||||||||
| if line.startswith("commit "): | ||||||||
| ciq_commit = line.split("commit ")[1] | ||||||||
| new_base_commit_map[ciq_commit] = "" | ||||||||
| if line.startswith(" commit "): | ||||||||
| upstream_commit = line.split(" commit ")[1] | ||||||||
| new_base_commit_map[ciq_commit] = upstream_commit | ||||||||
| new_base_commit_map_rev[upstream_commit] = ciq_commit | ||||||||
| new_base_commit_map, new_base_commit_map_rev = get_commit_maps_from_backport_data( | ||||||||
| args.repo, f"{os.getlogin()}_{new_rolling_branch_kernel}", common_sha | ||||||||
| ) | ||||||||
|
|
||||||||
| print(f"[rolling release update] Total commits in new branch: {len(new_base_commit_map)}") | ||||||||
| if DEBUG: | ||||||||
|
|
@@ -309,14 +368,16 @@ def check_for_fips_protected_changes(repo, branch, common_tag): | |||||||
| ) | ||||||||
| commits_to_remove = {} | ||||||||
| for ciq_commit, upstream_commit in rolling_commit_map.items(): | ||||||||
| if upstream_commit in new_base_commit_map_rev: | ||||||||
| if upstream_commit and upstream_commit in new_base_commit_map_rev: | ||||||||
| new_base_ciq_commit = new_base_commit_map_rev[upstream_commit] | ||||||||
| print(f"- Old commit {ciq_commit[:12]} backported upstream {upstream_commit[:12]}") | ||||||||
| print( | ||||||||
| f"- Commit {ciq_commit} already present in new base branch: {repo.git.show('--pretty=oneline', '-s', ciq_commit)}" | ||||||||
| f" Already in new base as {new_base_ciq_commit[:12]}: {repo.git.show('--pretty=%s', '-s', new_base_ciq_commit)}" | ||||||||
| ) | ||||||||
| commits_to_remove[ciq_commit] = upstream_commit | ||||||||
| if ciq_commit in new_base_commit_map: | ||||||||
| elif ciq_commit in new_base_commit_map: | ||||||||
| print( | ||||||||
| f"- CIQ Commit {ciq_commit} already present in new base branch: {repo.git.show('--pretty=oneline', '-s', ciq_commit)}" | ||||||||
| f"- CIQ Commit {ciq_commit[:12]} already present in new base branch: {repo.git.show('--pretty=oneline', '-s', ciq_commit)}" | ||||||||
|
||||||||
| f"- CIQ Commit {ciq_commit[:12]} already present in new base branch: {repo.git.show('--pretty=oneline', '-s', ciq_commit)}" | |
| f"- CIQ Commit {ciq_commit[:12]} already present in new base branch: {repo.git.show('--pretty=%s', '-s', ciq_commit)}" |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Potential logic issue with empty string handling: At line 371, the code checks if upstream_commit and upstream_commit in new_base_commit_map_rev:. The first condition checks for truthiness, which will be False for empty strings. However, earlier in the code (line 71), empty strings are explicitly used to indicate commits without upstream references. This means CIQ-specific commits (with upstream_commit == "") will never match the first condition, which is correct.
However, at line 378, there's an elif ciq_commit in new_base_commit_map: which could match CIQ-specific commits. If a CIQ-specific commit exists in both maps, it will be added to commits_to_remove with upstream_commit value of "" (line 382). This seems intentional, but the logic could be clearer. Consider adding a comment explaining this behavior.
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Parenthetical note may cause confusion: The text "(or commit manually if needed)" at line 416 might confuse users about the correct workflow. If a user commits manually without using git cherry-pick --continue, the cherry-pick state might not be properly cleared. Consider removing this parenthetical or clarifying that manual commits should still be followed by git cherry-pick --continue or appropriate cleanup.
| print("[rolling release update] (or commit manually if needed)") |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Ambiguous help text: The instruction at line 418 says "To skip this commit: git cherry-pick --skip", but users might be confused about what happens if they skip. Consider clarifying that skipping will cause the commit to be dropped from the rolling release, and that this might not be desirable in most cases. Adding a warning about the implications of skipping would improve user experience.
| print("[rolling release update] 1. To skip this commit: git cherry-pick --skip") | |
| print("[rolling release update] 1. To skip this commit: git cherry-pick --skip") | |
| print("[rolling release update] WARNING: Skipping will drop this commit from the rolling release. This may not be desirable, as the changes in this commit will not be included. Consider carefully before skipping.") |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential bug with dictionary iteration: The code iterates over rolling_commit_map.items() with reversed() at line 396, but then attempts to find (ciq_commit, upstream_commit) using list(reversed(rolling_commit_map.items())) at line 437. Since dictionaries in Python 3.7+ maintain insertion order, this should work, but there's a subtle issue: if commits are removed from rolling_commit_map during the loop (which happens when duplicate commits are found), the remaining_commits list won't accurately reflect what's left to process.
The issue is that remaining_commits is created from the original rolling_commit_map at line 437, but this happens AFTER we've already been iterating through it. The list should be created before the loop starts to accurately track remaining commits.
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Potential race condition with CHERRY_PICK_HEAD check: Between checking for the existence of .git/CHERRY_PICK_HEAD at line 458 and checking git status at line 464, the user could manually complete the cherry-pick in another terminal, leaving the file system in an intermediate state. While this is unlikely in normal usage, consider combining these checks or re-checking CHERRY_PICK_HEAD after the status check for robustness.
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential infinite loop risk: The while loop at line 424 could potentially become an infinite loop if the repository enters an unexpected state where neither the CHERRY_PICK_HEAD file exists nor are there uncommitted changes, but the cherry-pick actually failed. Consider adding a maximum retry count or timeout to prevent indefinite blocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Unclear documentation: The comment at line 41-42 says "get_backport_commit_data returns: { 'upstream_sha': { 'repo_commit': 'ciq_sha', ... } }", but this comment duplicates information from the ciq_helpers.py docstring and may become outdated. Consider removing this comment or making it a reference to the ciq_helpers function's documentation instead.