Skip to content

Commit bc616fa

Browse files
committed
ciq-cherry-pick.py: Improve error handling
All exceptions are propapagated to main and handled there, except for CherryPickException that is raised only if cherry pick failed due to conflicts. This way user is not overwhelmed with a stacktrace when the error is obvious. For unexpected errors, the full stacktrace is shown. Also added some comments to full_cherry_pick function to explain that if one of the cherry pick fails, the previous successful ones are left intact. This can be improved in the future by making it interactive. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
1 parent a8712fe commit bc616fa

2 files changed

Lines changed: 42 additions & 10 deletions

File tree

ciq-cherry-pick.py

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import os
44
import re
55
import subprocess
6+
import traceback
67

78
import git
89

@@ -13,13 +14,18 @@
1314
CIQ_fixes_references,
1415
CIQ_get_full_hash,
1516
CIQ_original_commit_author_to_tag_string,
17+
CIQ_reset_HEAD,
1618
CIQ_run_git,
1719
)
1820

1921
MERGE_MSG = git.Repo(os.getcwd()).git_dir + "/MERGE_MSG"
2022
MERGE_MSG_BAK = f"{MERGE_MSG}.bak"
2123

2224

25+
class CherryPickException(Exception):
26+
pass
27+
28+
2329
def check_fixes(sha):
2430
"""
2531
Checks if commit has "Fixes:" references and if so, it checks if the
@@ -57,8 +63,11 @@ def manage_commit_message(full_sha, ciq_tags, jira_ticket, commit_successful):
5763
raise RuntimeError(f"Could not find author of commit {full_sha}")
5864

5965
new_tags.append(author)
60-
with open(MERGE_MSG, "r") as file:
61-
original_msg = file.readlines()
66+
try:
67+
with open(MERGE_MSG, "r") as file:
68+
original_msg = file.readlines()
69+
except IOError as e:
70+
raise RuntimeError(f"Failed to read commit message from {MERGE_MSG}: {e}") from e
6271

6372
optional_msg = "" if commit_successful else "upstream-diff |"
6473
new_msg = CIQ_cherry_pick_commit_standardization(
@@ -68,8 +77,11 @@ def manage_commit_message(full_sha, ciq_tags, jira_ticket, commit_successful):
6877
print(f"Cherry Pick New Message for {full_sha}")
6978
print(f"\n Original Message located here: {MERGE_MSG_BAK}")
7079

71-
with open(MERGE_MSG, "w") as file:
72-
file.writelines(new_msg)
80+
try:
81+
with open(MERGE_MSG, "w") as file:
82+
file.writelines(new_msg)
83+
except IOError as e:
84+
raise RuntimeError(f"Failed to write commit message to {MERGE_MSG}: {e}") from e
7385

7486

7587
def cherry_pick(sha, ciq_tags, jira_ticket):
@@ -79,6 +91,8 @@ def cherry_pick(sha, ciq_tags, jira_ticket):
7991
- MERGE_MSG.bak contains the original commit message
8092
- MERGE_MSG contains the standardized commit message
8193
- Conflict has to be solved manually
94+
In case of runtime errors that are not cherry pick conflicts, the cherry
95+
pick changes are reverted. (git reset --hard HEAD)
8296
8397
In case of success:
8498
- the commit is cherry picked
@@ -87,7 +101,10 @@ def cherry_pick(sha, ciq_tags, jira_ticket):
87101
"""
88102

89103
# Expand the provided SHA1 to the full SHA1 in case it's either abbreviated or an expression
90-
full_sha = CIQ_get_full_hash(repo=os.getcwd(), short_hash=sha)
104+
try:
105+
full_sha = CIQ_get_full_hash(repo=os.getcwd(), short_hash=sha)
106+
except RuntimeError as e:
107+
raise RuntimeError(f"Invalid commit SHA {sha}: {e}") from e
91108

92109
check_fixes(sha=full_sha)
93110

@@ -98,16 +115,20 @@ def cherry_pick(sha, ciq_tags, jira_ticket):
98115
except RuntimeError:
99116
commit_successful = False
100117

101-
manage_commit_message(
102-
full_sha=full_sha, ciq_tags=ciq_tags, jira_ticket=jira_ticket, commit_successful=commit_successful
103-
)
118+
try:
119+
manage_commit_message(
120+
full_sha=full_sha, ciq_tags=ciq_tags, jira_ticket=jira_ticket, commit_successful=commit_successful
121+
)
122+
except RuntimeError as e:
123+
CIQ_reset_HEAD(repo=os.getcwd())
124+
raise RuntimeError(f"Could not create proper commit message: {e}") from e
104125

105126
if not commit_successful:
106127
error_str = (
107128
f"[FAILED] git cherry-pick -nsx {full_sha}\n"
108129
"Manually resolve conflict and add explanation under `upstream-diff` tag in commit message\n"
109130
)
110-
raise RuntimeError(error_str)
131+
raise CherryPickException(error_str)
111132

112133
CIQ_run_git(repo_path=os.getcwd(), args=["commit", "-F", MERGE_MSG])
113134

@@ -131,6 +152,9 @@ def cherry_pick_fixes(sha, ciq_tags, jira_ticket, upstream_ref):
131152
def full_cherry_pick(sha, ciq_tags, jira_ticket, upstream_ref):
132153
"""
133154
Cherry picks a commit from upstream-ref along with its Fixes: references.
155+
If cherry-pick or cherry_pick_fixes fail, the exception is propagated
156+
If one of the cherry picks fails, an exception is returned and the previous
157+
successful cherry picks are left as they are.
134158
"""
135159
# Cherry pick the commit
136160
cherry_pick(sha=sha, ciq_tags=ciq_tags, jira_ticket=jira_ticket)
@@ -168,6 +192,10 @@ def full_cherry_pick(sha, ciq_tags, jira_ticket, upstream_ref):
168192

169193
try:
170194
full_cherry_pick(sha=args.sha, ciq_tags=tags, jira_ticket=args.ticket, upstream_ref=args.upstream_ref)
171-
except Exception as e:
195+
except CherryPickException as e:
172196
print(e)
173197
exit(1)
198+
except Exception as e:
199+
print(f"full_cherry_pick failed {e}")
200+
traceback.print_exc()
201+
exit(1)

ciq_helpers.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,10 @@ def CIQ_find_fixes_in_mainline_current_branch(repo, upstream_ref, hash_):
305305
return CIQ_find_fixes_in_mainline(repo, current_branch, upstream_ref, hash_)
306306

307307

308+
def CIQ_reset_HEAD(repo):
309+
return CIQ_run_git(repo=repo, args=["reset", "--hard", "HEAD"])
310+
311+
308312
def repo_init(repo):
309313
"""Initialize a git repo object.
310314

0 commit comments

Comments
 (0)