From 73d2ac1e39f86806b3d7be7ce09e39690127f5ec Mon Sep 17 00:00:00 2001 From: Evan Wilde Date: Mon, 25 Jul 2022 14:46:52 -0700 Subject: [PATCH 1/2] Fix update-checkout error reporting In the event of a failure, update-checkout wouldn't always fail very gracefully. As a result the actual failure wasn't always printed, which makes it really difficult to figure out what's going on. In my case, I'm jumping between 5.6 and main branches, so the `swift-cmark-gfm` is causing update-checkout to hiccup since it's not in the `main` configuration file scheme for `main`, but is in the 5.6 repository. You wouldn't know that though, since it would complain about `repo_path` not being in the error message before crashing. I've gone ahead and done some cleanups, reporting what the problem is at the point of failure beyond the "key error 'swift-cmark-cfg'. Modernized the error reporter a bit, so it should be a smidge easier to read. --- .../update_checkout/update_checkout.py | 71 ++++++++++++------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/utils/update_checkout/update_checkout/update_checkout.py b/utils/update_checkout/update_checkout/update_checkout.py index 83610b6330025..fd4ffe8319688 100755 --- a/utils/update_checkout/update_checkout/update_checkout.py +++ b/utils/update_checkout/update_checkout/update_checkout.py @@ -60,18 +60,33 @@ def check_parallel_results(results, op): parallel implementation. """ - fail_count = 0 if results is None: return 0 + + # Remove non-errors + results = [r for r in results if r] + + if results: + print(f"======{op} FAILURES======", file=sys.stderr) + + messages = [] for r in results: - if r is not None: - if fail_count == 0: - print("======%s FAILURES======" % op) - print("%s failed (ret=%d): %s" % (r.repo_path, r.ret, r)) - fail_count += 1 - if r.stderr: - print(r.stderr) - return fail_count + try: + raise r + except KeyError as e: + messages.append(f"Failed to look up {e} in scheme\n") + except Exception: + path = getattr(r, 'repo_path', '') + reason = getattr(r, 'stderr', '') + if path: + message = f"{path} failed" + if reason: + message += f"\n```\n{reason}\n```" + message += f"\n{r}" + if message: + messages.append(message + '\n') + print('---\n\n'.join(messages), file=sys.stderr) + return len(results) def confirm_tag_in_repo(tag, repo_name): @@ -100,20 +115,25 @@ def get_branch_for_repo(config, repo_name, scheme_name, scheme_map, cross_repo = False repo_branch = scheme_name if scheme_map: - scheme_branch = scheme_map[repo_name] - repo_branch = scheme_branch - remote_repo_id = config['repos'][repo_name]['remote']['id'] - if remote_repo_id in cross_repos_pr: - cross_repo = True - pr_id = cross_repos_pr[remote_repo_id] - repo_branch = "ci_pr_{0}".format(pr_id) - shell.run(["git", "checkout", scheme_branch], - echo=True) - shell.capture(["git", "branch", "-D", repo_branch], - echo=True, allow_non_zero_exit=True) - shell.run(["git", "fetch", "origin", - "pull/{0}/merge:{1}" - .format(pr_id, repo_branch), "--tags"], echo=True) + try: + scheme_branch = scheme_map[repo_name] + repo_branch = scheme_branch + remote_repo_id = config['repos'][repo_name]['remote']['id'] + if remote_repo_id in cross_repos_pr: + cross_repo = True + pr_id = cross_repos_pr[remote_repo_id] + repo_branch = "ci_pr_{0}".format(pr_id) + shell.run(["git", "checkout", scheme_branch], + echo=True) + shell.capture(["git", "branch", "-D", repo_branch], + echo=True, allow_non_zero_exit=True) + shell.run(["git", "fetch", "origin", + "pull/{0}/merge:{1}" + .format(pr_id, repo_branch), "--tags"], echo=True) + except KeyError as e: + print(f"Failed to look up {repo_name} in scheme", file=sys.stderr) + raise e + exit(1) return repo_branch, cross_repo @@ -634,10 +654,9 @@ def main(): update_results = update_all_repositories(args, config, scheme, cross_repos_pr) - fail_count = 0 - fail_count += check_parallel_results(clone_results, "CLONE") + fail_count = check_parallel_results(clone_results, "CLONE") fail_count += check_parallel_results(update_results, "UPDATE") - if fail_count > 0: + if fail_count: print("update-checkout failed, fix errors and try again") else: print("update-checkout succeeded") From 71b3ff524a4e0a9291e47c5c09484f79842cde92 Mon Sep 17 00:00:00 2001 From: Evan Wilde Date: Thu, 4 Aug 2022 22:33:29 -0700 Subject: [PATCH 2/2] Smol cleanups Removing the `exit(1)` after failing. This shouldn't have an impact since we've already failed and shouldn't have hit that. I may have kept it from earlier debugging. Replace `raise e` with just `raise`. It removes one level from the stack trace where the actual `raise` takes place. The place we're actually interested in is where the failure actually takes place. --- utils/update_checkout/update_checkout/update_checkout.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/utils/update_checkout/update_checkout/update_checkout.py b/utils/update_checkout/update_checkout/update_checkout.py index fd4ffe8319688..b4be9944c63ec 100755 --- a/utils/update_checkout/update_checkout/update_checkout.py +++ b/utils/update_checkout/update_checkout/update_checkout.py @@ -130,10 +130,9 @@ def get_branch_for_repo(config, repo_name, scheme_name, scheme_map, shell.run(["git", "fetch", "origin", "pull/{0}/merge:{1}" .format(pr_id, repo_branch), "--tags"], echo=True) - except KeyError as e: + except KeyError: print(f"Failed to look up {repo_name} in scheme", file=sys.stderr) - raise e - exit(1) + raise return repo_branch, cross_repo