From 867a297158494e43709e09bd57fbbcab7082982d Mon Sep 17 00:00:00 2001 From: Carles Cufi Date: Fri, 29 Nov 2024 12:37:25 +0100 Subject: [PATCH 1/2] action: Document errors properly The action currently fails with very little explanation as to why it does so. Add help text that describes the causes of failure for most of the common cases. Signed-off-by: Carles Cufi --- action.py | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/action.py b/action.py index d860e03..568cb8b 100755 --- a/action.py +++ b/action.py @@ -56,6 +56,8 @@ def stdout(msg): def die(s): print(f'ERROR: {s}', file=sys.stderr) + print(f'\nMore information about the process can ben found in: ' + f'\nhttps://nordicsemi.atlassian.net/wiki/spaces/NCS/pages/108201225/Pull+Requests') if die_switch: # Switch back stdout('die: switch') @@ -156,14 +158,14 @@ def fetch_pr(repo, prn, target): if pr.is_merged(): die(f'PR #{prn} is merged, please use [nrf fromtree] instead') if pr.state == 'closed': - die(f'PR #{prn} is closed and not merged') + die(f'PR #{prn} is closed and not merged, please open a new PR') revs = dict() for rev in pr.get_reviews(): revs[rev.user.login] = rev.state for k,v in revs.items(): if "CHANGES_REQUESTED" in v: - die(f'PR #{prn} has requested changes') + die(f'PR #{prn} has requested changes, please resolve those') shas = [c.sha for c in pr.get_commits()] ref = f'nrf/pull/{prn}' @@ -211,19 +213,21 @@ def check_commit(urepo, ubranch, target, sha, merge): m = re.match(r'^(Revert\s+\")?\[nrf (mergeup|fromtree|fromlist|noup)\]\s+', title) if not m: - die(f'{sha}: Title does not contain a sauce tag') + die(f'{sha}: Title does not contain a sauce tag ' + f'(mergeup|fromtree|fromlist|noup)') revert = m.group(1) tag = m.group(2) if not tag: - die(f'{sha}: Title does not contain a sauce tag') + die(f'{sha}: Title does not contain a sauce tag ' + f'(mergeup|fromtree|fromlist|noup)') if revert: if tag == 'mergeup': - die('Mergeup cannot be reverted') + die('Mergeup commits cannot be reverted') regex = r'^This reverts commit \b([a-f0-9]{40})\b\.' match = re.search(regex, body, re.MULTILINE) if not match: - die(f'{sha}: revert commit missing reverted SHA') + die(f'{sha}: revert commit message missing reverted SHA') stdout(f'revert: {match.group(1)}') # The SHA to replay is the revert commmit's usha = sha @@ -244,7 +248,7 @@ def check_commit(urepo, ubranch, target, sha, merge): match = re.search(regex, body, re.MULTILINE) if not match: - die(f'{sha}: fromlist commit missing an Upstream PR reference') + die(f'{sha}: fromlist commit missing an "Upstream PR #:" reference') upr = match.group(1) stdout(f'fromlist: {upr}') @@ -263,13 +267,14 @@ def check_commit(urepo, ubranch, target, sha, merge): break if not usha: - die(f'{sha}: unable to match any commit from PR {upr}') + die(f'{sha}: unable to match any commit from upstream PR #{upr}') elif tag == 'fromtree': regex = r'^\(cherry picked from commit \b([a-f0-9]{40})\b\)' match = re.search(regex, body, re.MULTILINE) if not match: - die(f'{sha}: fromtree commit missing cherry-pick reference') + die(f'{sha}: fromtree commit missing cherry-pick reference. ' + f'Please use "git cherry-pick -x" when cherry-picking') #stdout(f'fromtree: {match.group(0)}') usha = match.group(1) @@ -285,7 +290,8 @@ def check_commit(urepo, ubranch, target, sha, merge): contains = runc_out(f'git -C {target} branch {ref} --contains {usha}') if not re.match(rf'^.*{ref}.*$', contains): - die(f'Branch {ref} does not contain commit {usha}') + die(f'fromtree: upstream branch {ref} does not contain commit {usha}. ' + f'Please check that the commit is merged upstream') elif tag == 'noup': stdout('noup') @@ -308,14 +314,22 @@ def check_commit(urepo, ubranch, target, sha, merge): except subprocess.CalledProcessError as e: pass # Ignore it and exit forcefully - die(f'ERROR: unable to cherry-pick {usha}') + die(f'Unable to cherry-pick commit {usha}. This means that the upstream ' + f'commit does not apply cleanly into the NCS fork. This can happen ' + f'if you modified an upstream commit in order to resolve conflicts, ' + f'but this is now allowed. Instead, revert any [nrf noup] commits that ' + f'may be causing the conflict and cherry-pick any additional prior ' + f'commits from upstream that may be needed in order to avoid a merge ' + f'conflict') # Execute a diff between the replay branch and the sha to make sure the # commit has not been modified diff = runc_out(f'git -C {target} diff {sha}') if diff: - die(f'SHA {sha} non-empty diff:\n{diff}') + die(f'SHA {sha} non-empty diff between fork and upstream. This likely ' + f'means that you modified an upstream commit when cherry-picking. ' + f'This is not allowed. Full diff:\n{diff}') return False From a5baaf62540df6cec069f22dde2911e7d3967be0 Mon Sep 17 00:00:00 2001 From: Ingar Kulbrandstad <58302197+Balaklaka@users.noreply.github.com> Date: Fri, 29 Nov 2024 13:32:05 +0100 Subject: [PATCH 2/2] Update action.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If [ ] is used, then nrf should be added. And it should be the same in L217 and L222. Co-authored-by: Kristoffer Rist Skøien --- action.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/action.py b/action.py index 568cb8b..90687e2 100755 --- a/action.py +++ b/action.py @@ -219,7 +219,7 @@ def check_commit(urepo, ubranch, target, sha, merge): tag = m.group(2) if not tag: die(f'{sha}: Title does not contain a sauce tag ' - f'(mergeup|fromtree|fromlist|noup)') + f'([mergeup] |[fromtree] |[fromlist] |[noup]) ') if revert: if tag == 'mergeup':