From 75e03277085be61fd073d5cfce76cf052b00d387 Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Mon, 17 Jun 2024 10:47:53 +0100 Subject: [PATCH 1/2] Be more precise about where to read the URL rewriter config from There is an [open issue](https://github.com/bazelbuild/bazel/issues/22104) where bazel will occasionally fail at start up time because it is unable to parse the downloader config. While I've yet to create a reliable reproduction for this issue, the one time I've triggered this error with a build with additional logging, it seemed to be because the current working directory wasn't the root of the workspace. While I'm not sure this will actually fully resolve the problem (since I don't know what's causing it) being clearer about where the config file is meant to be can only be a Good Thing. --- .../build/lib/bazel/BazelRepositoryModule.java | 2 +- .../repository/downloader/UrlRewriter.java | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 0187c6bf91c0ad..b2f43f52ad24e8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -385,7 +385,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { } try { UrlRewriter rewriter = - UrlRewriter.getDownloaderUrlRewriter(repoOptions.downloaderConfig, env.getReporter()); + UrlRewriter.getDownloaderUrlRewriter(env.getWorkspace(), repoOptions.downloaderConfig, env.getReporter()); downloadManager.setUrlRewriter(rewriter); } catch (UrlRewriterParseException e) { // It's important that the build stops ASAP, because this config file may be required for diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java index 3cb965c1bd946d..24d11aaabe26fd 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java @@ -85,7 +85,7 @@ public class UrlRewriter { * @param configPath Path to the config file to use. May be null. * @param reporter Used for logging when URLs are rewritten. */ - public static UrlRewriter getDownloaderUrlRewriter(String configPath, Reporter reporter) + public static UrlRewriter getDownloaderUrlRewriter(Path workspaceRoot, String configPath, Reporter reporter) throws UrlRewriterParseException { Consumer log = str -> reporter.handle(Event.info(str)); @@ -94,6 +94,21 @@ public static UrlRewriter getDownloaderUrlRewriter(String configPath, Reporter r return new UrlRewriter(log, "", new StringReader("")); } + // If the `configPath` is absolute, use that. Otherwise, prepend the `workspaceRoot`. + // There have been reports (eg. https://github.com/bazelbuild/bazel/issues/22104) that + // there are occasional errors when `configFile` can't be found, and when this happens + // investigation suggests that the current working directory isn't the workspace root. + Path actualConfigPath; + if (Paths.get(configPath).isAbsolute()) { + actualConfigPath = workspaceRoot.getFileSystem().getPath(configPath); + } else { + actualConfigPath = workspaceRoot.getRelative(configPath); + } + + if (!actualConfigPath.exists()) { + throw new UrlRewriterParseException(String.format("Unable to find downloader config file %s", configPath)); + } + try (BufferedReader reader = Files.newBufferedReader(Paths.get(configPath))) { return new UrlRewriter(log, configPath, reader); } catch (IOException e) { From ebda2c329175ddddc93bce091d2faa608625a455 Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Tue, 18 Jun 2024 11:05:32 +0100 Subject: [PATCH 2/2] Respond to review comment --- .../lib/bazel/repository/downloader/UrlRewriter.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java index 24d11aaabe26fd..28bcbcdc559529 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java @@ -41,7 +41,6 @@ import java.net.URISyntaxException; import java.net.URL; import java.nio.file.Files; -import java.nio.file.Paths; import java.util.Base64; import java.util.Collection; import java.util.HashMap; @@ -94,22 +93,16 @@ public static UrlRewriter getDownloaderUrlRewriter(Path workspaceRoot, String co return new UrlRewriter(log, "", new StringReader("")); } - // If the `configPath` is absolute, use that. Otherwise, prepend the `workspaceRoot`. // There have been reports (eg. https://github.com/bazelbuild/bazel/issues/22104) that // there are occasional errors when `configFile` can't be found, and when this happens // investigation suggests that the current working directory isn't the workspace root. - Path actualConfigPath; - if (Paths.get(configPath).isAbsolute()) { - actualConfigPath = workspaceRoot.getFileSystem().getPath(configPath); - } else { - actualConfigPath = workspaceRoot.getRelative(configPath); - } + Path actualConfigPath = workspaceRoot.getRelative(configPath); if (!actualConfigPath.exists()) { throw new UrlRewriterParseException(String.format("Unable to find downloader config file %s", configPath)); } - try (BufferedReader reader = Files.newBufferedReader(Paths.get(configPath))) { + try (BufferedReader reader = Files.newBufferedReader(actualConfigPath.getPathFile().toPath())) { return new UrlRewriter(log, configPath, reader); } catch (IOException e) { throw new UrlRewriterParseException(e.getMessage());