From 4312b94e823342ded2dd79e049cce18e3cb84427 Mon Sep 17 00:00:00 2001 From: helly25 Date: Sun, 11 May 2025 12:34:23 +0000 Subject: [PATCH 1/2] Improve error messages. Keep old short messages for the tests. --- toolchain/internal/llvm_distributions.bzl | 31 ++++++++++++++--------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/toolchain/internal/llvm_distributions.bzl b/toolchain/internal/llvm_distributions.bzl index 86ef84d02..c4e01ca3a 100644 --- a/toolchain/internal/llvm_distributions.bzl +++ b/toolchain/internal/llvm_distributions.bzl @@ -947,19 +947,22 @@ def _find_llvm_basename_list(llvm_version, host_info): def _find_llvm_basename_or_error(llvm_version, host_info): basenames = _find_llvm_basename_list(llvm_version, host_info) if len(basenames) > 1: - return None, "ERROR: Multiple configurations found [{basenames}].".format( + return None, "ERROR: Multiple configurations found for version {llvm_version} on {os}/{dist_name}/{dist_version} with arch {arch}: [{basenames}].".format( + llvm_version = llvm_version, + os = host_info.os, + dist_name = host_info.dist.name, + dist_version = host_info.dist.version, + arch = host_info.arch, basenames = ", ".join(basenames), ) if not basenames: - return None, "ERROR: No version selected" - # TODO(helly25): Enable better error message: - #"ERROR: No matching config could be found for version {llvm_version} on {os}/{dist_name}/{dist_version} with arch {arch}.".format( - # llvm_version = llvm_version, - # os = host_info.os, - # dist_name = host_info.dist.name, - # dist_version = host_info.dist.version, - # arch = host_info.arch, - #) + return None, "ERROR: No matching config could be found for version {llvm_version} on {os}/{dist_name}/{dist_version} with arch {arch}.".format( + llvm_version = llvm_version, + os = host_info.os, + dist_name = host_info.dist.name, + dist_version = host_info.dist.version, + arch = host_info.arch, + ) # Use the following for debugging: # print("Found LLVM: " + basenames[0]) # buildifier: disable=print @@ -977,7 +980,7 @@ def _distribution_urls(rctx): basename = rctx.attr.distribution if basename not in _llvm_distributions: - fail("Unknown LLVM release: %s\nPlease ensure file name is correct." % basename) + fail("ERROR: Unknown LLVM release: %s\nPlease ensure file name is correct." % basename) urls = [] url_suffix = "{0}/{1}".format(llvm_version, basename).replace("+", "%2B") @@ -1135,7 +1138,11 @@ def _write_distributions_impl(ctx): _version_string(version), host_info, ) - if not error: + if error: + if error.startswith("ERROR: No matching config could be found for version"): + # Simplify test output: + error = "ERROR: No version selected" + else: if predicted.endswith(".exe"): error = "ERROR: Windows .exe is not supported: " + predicted elif predicted not in _llvm_distributions: From 5f55078c64999503ef0158b80155a46e18b0e9c4 Mon Sep 17 00:00:00 2001 From: helly25 Date: Sun, 11 May 2025 12:54:08 +0000 Subject: [PATCH 2/2] Dont route errors in test through prediction. --- toolchain/internal/llvm_distributions.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/toolchain/internal/llvm_distributions.bzl b/toolchain/internal/llvm_distributions.bzl index c4e01ca3a..3f2f27e25 100644 --- a/toolchain/internal/llvm_distributions.bzl +++ b/toolchain/internal/llvm_distributions.bzl @@ -1148,11 +1148,11 @@ def _write_distributions_impl(ctx): elif predicted not in _llvm_distributions: error = "ERROR: Unavailable prediction: " + predicted elif len(basenames) == 0: - predicted = "ERROR: No version selected" + error = "ERROR: No version selected" elif len(basenames) == 1: predicted = basenames[0] else: - predicted = "ERROR: Multiple selections" + error = "ERROR: Multiple selections" if not error: arch_found = [arch for arch in arch_list if arch in predicted] if len(arch_found) == 1 and arch_found[0] != arch: