Skip to content

Commit 44039bf

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 bf3f3ac commit 44039bf

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
@@ -4,6 +4,7 @@
44
import re
55
import subprocess
66
import sys
7+
import traceback
78

89
import git
910

@@ -14,13 +15,18 @@
1415
CIQ_fixes_references,
1516
CIQ_get_full_hash,
1617
CIQ_original_commit_author_to_tag_string,
18+
CIQ_reset_HEAD,
1719
CIQ_run_git,
1820
)
1921

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

2325

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

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

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

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

7587

7688
def cherry_pick(sha, ciq_tags, jira_ticket):
@@ -80,6 +92,8 @@ def cherry_pick(sha, ciq_tags, jira_ticket):
8092
- MERGE_MSG.bak contains the original commit message
8193
- MERGE_MSG contains the standardized commit message
8294
- Conflict has to be solved manually
95+
In case of runtime errors that are not cherry pick conflicts, the cherry
96+
pick changes are reverted. (git reset --hard HEAD)
8397
8498
In case of success:
8599
- the commit is cherry picked
@@ -88,7 +102,10 @@ def cherry_pick(sha, ciq_tags, jira_ticket):
88102
"""
89103

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

93110
check_fixes(sha=full_sha)
94111

@@ -99,16 +116,20 @@ def cherry_pick(sha, ciq_tags, jira_ticket):
99116
except RuntimeError:
100117
commit_successful = False
101118

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

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

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

@@ -132,6 +153,9 @@ def cherry_pick_fixes(sha, ciq_tags, jira_ticket, upstream_ref):
132153
def full_cherry_pick(sha, ciq_tags, jira_ticket, upstream_ref):
133154
"""
134155
Cherry picks a commit from upstream-ref along with its Fixes: references.
156+
If cherry-pick or cherry_pick_fixes fail, the exception is propagated
157+
If one of the cherry picks fails, an exception is returned and the previous
158+
successful cherry picks are left as they are.
135159
"""
136160
# Cherry pick the commit
137161
cherry_pick(sha=sha, ciq_tags=ciq_tags, jira_ticket=jira_ticket)
@@ -180,6 +204,10 @@ def full_cherry_pick(sha, ciq_tags, jira_ticket, upstream_ref):
180204

181205
try:
182206
full_cherry_pick(sha=args.sha, ciq_tags=tags, jira_ticket=args.ticket, upstream_ref=args.upstream_ref)
183-
except Exception as e:
207+
except CherryPickException as e:
184208
logging.error(e)
185209
sys.exit(1)
210+
except Exception as e:
211+
logging.error(f"full_cherry_pick failed {e}")
212+
traceback.print_exc()
213+
sys.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_path=repo, args=["reset", "--hard", "HEAD"])
310+
311+
308312
def repo_init(repo):
309313
"""Initialize a git repo object.
310314

0 commit comments

Comments
 (0)