Revert "Release 5.3.0 (#16074)"

This reverts commit 2077c54.

CODEOWNERS

@@ -1 +1,2 @@
+* @ckolli5
 * @kshyanashree

src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java

@@ -77,15 +77,15 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ
 
       process.waitFor();
       if (process.timedout()) {
-        throw new CredentialHelperException(
+        throw new IOException(
             String.format(
                 Locale.US,
                 "Failed to get credentials for '%s' from helper '%s': process timed out",
                 uri,
                 path));
       }
       if (process.exitValue() != 0) {
-        throw new CredentialHelperException(
+        throw new IOException(
             String.format(
                 Locale.US,
                 "Failed to get credentials for '%s' from helper '%s': process exited with code %d."
@@ -99,7 +99,7 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ
       try {
         GetCredentialsResponse response = GSON.fromJson(stdout, GetCredentialsResponse.class);
         if (response == null) {
-          throw new CredentialHelperException(
+          throw new IOException(
               String.format(
                   Locale.US,
                   "Failed to get credentials for '%s' from helper '%s': process exited without"
@@ -110,7 +110,7 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ
         }
         return response;
       } catch (JsonSyntaxException e) {
-        throw new CredentialHelperException(
+        throw new IOException(
             String.format(
                 Locale.US,
                 "Failed to get credentials for '%s' from helper '%s': error parsing output. stderr:"

src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java

@@ -34,7 +34,6 @@
 import com.google.devtools.build.lib.remote.util.DigestUtil;
 import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
 import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.PathFragment;
 import io.netty.util.AbstractReferenceCounted;
 import io.netty.util.ReferenceCounted;
 import io.reactivex.rxjava3.core.Flowable;
@@ -68,8 +67,8 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
   private final AtomicBoolean shutdown = new AtomicBoolean();
   private final Scheduler scheduler;
 
-  private final Set<PathFragment> omittedFiles = Sets.newConcurrentHashSet();
-  private final Set<PathFragment> omittedTreeRoots = Sets.newConcurrentHashSet();
+  private final Set<Path> omittedFiles = Sets.newConcurrentHashSet();
+  private final Set<Path> omittedTreeRoots = Sets.newConcurrentHashSet();
 
   ByteStreamBuildEventArtifactUploader(
       Executor executor,
@@ -90,11 +89,11 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
   }
 
   public void omitFile(Path file) {
-    omittedFiles.add(file.asFragment());
+    omittedFiles.add(file);
   }
 
   public void omitTree(Path treeRoot) {
-    omittedTreeRoots.add(treeRoot.asFragment());
+    omittedTreeRoots.add(treeRoot);
   }
 
   /** Returns {@code true} if Bazel knows that the file is stored on a remote system. */
@@ -154,14 +153,13 @@ private PathMetadata readPathMetadata(Path file) throws IOException {
           /* omitted= */ false);
     }
 
-    PathFragment filePathFragment = file.asFragment();
     boolean omitted = false;
-    if (omittedFiles.contains(filePathFragment)) {
+    if (omittedFiles.contains(file)) {
       omitted = true;
     } else {
-      for (PathFragment treeRoot : omittedTreeRoots) {
+      for (Path treeRoot : omittedTreeRoots) {
         if (file.startsWith(treeRoot)) {
-          omittedFiles.add(filePathFragment);
+          omittedFiles.add(file);
           omitted = true;
         }
       }

src/main/java/com/google/devtools/build/lib/remote/util/BUILD

@@ -19,7 +19,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
         "//src/main/java/com/google/devtools/build/lib/authandtls",
-        "//src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/remote:ExecutionStatusException",
         "//src/main/java/com/google/devtools/build/lib/remote/common",

src/main/java/com/google/devtools/build/lib/remote/util/Utils.java

@@ -39,7 +39,6 @@
 import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.authandtls.CallCredentialsProvider;
-import com.google.devtools.build.lib.authandtls.credentialhelper.CredentialHelperException;
 import com.google.devtools.build.lib.remote.ExecutionStatusException;
 import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
 import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
@@ -368,7 +367,6 @@ private static String executionStatusExceptionErrorMessage(ExecutionStatusExcept
   public static String grpcAwareErrorMessage(IOException e) {
     io.grpc.Status errStatus = io.grpc.Status.fromThrowable(e);
     if (e.getCause() instanceof ExecutionStatusException) {
-      // Display error message returned by the remote service.
       try {
         return "Remote Execution Failure:\n"
             + executionStatusExceptionErrorMessage((ExecutionStatusException) e.getCause());
@@ -380,20 +378,9 @@ public static String grpcAwareErrorMessage(IOException e) {
       }
     }
     if (!errStatus.getCode().equals(io.grpc.Status.UNKNOWN.getCode())) {
-      // Display error message returned by the gRPC library, prefixed by the status code.
-      StringBuilder sb = new StringBuilder();
-      sb.append(errStatus.getCode().name());
-      sb.append(": ");
-      sb.append(errStatus.getDescription());
-      // If the error originated from a credential helper, print additional debugging information.
-      for (Throwable t = errStatus.getCause(); t != null; t = t.getCause()) {
-        if (t instanceof CredentialHelperException) {
-          sb.append(": ");
-          sb.append(t.getMessage());
-          break;
-        }
-      }
-      return sb.toString();
+      // If the error originated in the gRPC library then display it as "STATUS: error message"
+      // to the user
+      return String.format("%s: %s", errStatus.getCode().name(), errStatus.getDescription());
     }
     return e.getMessage();
   }

src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java

@@ -39,14 +39,7 @@
  * (e.g., {@code %workspace%/foo} becomes {@code </path/to/workspace>/foo}).
  */
 public final class CommandLinePathFactory {
-  /** An exception thrown while attempting to resolve a path. */
-  public static class CommandLinePathFactoryException extends IOException {
-    public CommandLinePathFactoryException(String message) {
-      super(message);
-    }
-  }
-
-  private static final Pattern REPLACEMENT_PATTERN = Pattern.compile("^(%([a-z_]+)%/+)?([^%].*)$");
+  private static final Pattern REPLACEMENT_PATTERN = Pattern.compile("^(%([a-z_]+)%/)?([^%].*)$");
 
   private static final Splitter PATH_SPLITTER = Splitter.on(File.pathSeparator);
 
@@ -85,17 +78,20 @@ public Path create(Map<String, String> env, String value) throws IOException {
     String rootName = matcher.group(2);
     PathFragment path = PathFragment.create(matcher.group(3));
     if (path.containsUplevelReferences()) {
-      throw new CommandLinePathFactoryException(
+      throw new IllegalArgumentException(
           String.format(
               Locale.US, "Path must not contain any uplevel references ('..'), got '%s'", value));
     }
 
     // Case 1: `path` is relative to a well-known root.
     if (!Strings.isNullOrEmpty(rootName)) {
+      // The regex above cannot check that `value` is not of form `%foo%//abc` (group 2 will be
+      // `foo` and group 3 will be `/abc`).
+      Preconditions.checkArgument(!path.isAbsolute());
+
       Path root = roots.get(rootName);
       if (root == null) {
-        throw new CommandLinePathFactoryException(
-            String.format(Locale.US, "Unknown root %s", rootName));
+        throw new IllegalArgumentException(String.format(Locale.US, "Unknown root %s", rootName));
       }
       return root.getRelative(path);
     }
@@ -112,7 +108,7 @@ public Path create(Map<String, String> env, String value) throws IOException {
     // flag is from?), we only allow relative paths with a single segment (i.e., no `/`) and treat
     // it as relative to the user's `PATH`.
     if (path.segmentCount() > 1) {
-      throw new CommandLinePathFactoryException(
+      throw new IllegalArgumentException(
           "Path must either be absolute or not contain any path separators");
     }
 

src/test/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java

@@ -28,6 +28,7 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import com.google.devtools.build.runfiles.Runfiles;
+import java.io.IOException;
 import java.net.URI;
 import java.time.Duration;
 import org.junit.Test;
@@ -94,20 +95,18 @@ public void knownUriWithMultipleHeaders() throws Exception {
 
   @Test
   public void unknownUri() {
-    CredentialHelperException e =
+    IOException ioException =
         assertThrows(
-            CredentialHelperException.class,
-            () -> getCredentialsFromHelper("https://unknown.example.com"));
-    assertThat(e).hasMessageThat().contains("Unknown uri 'https://unknown.example.com'");
+            IOException.class, () -> getCredentialsFromHelper("https://unknown.example.com"));
+    assertThat(ioException).hasMessageThat().contains("Unknown uri 'https://unknown.example.com'");
   }
 
   @Test
   public void credentialHelperOutputsNothing() throws Exception {
-    CredentialHelperException e =
+    IOException ioException =
         assertThrows(
-            CredentialHelperException.class,
-            () -> getCredentialsFromHelper("https://printnothing.example.com"));
-    assertThat(e).hasMessageThat().contains("exited without output");
+            IOException.class, () -> getCredentialsFromHelper("https://printnothing.example.com"));
+    assertThat(ioException).hasMessageThat().contains("exited without output");
   }
 
   @Test
@@ -136,10 +135,9 @@ public void helperGetEnvironment() throws Exception {
 
   @Test
   public void helperTimeout() throws Exception {
-    CredentialHelperException e =
+    IOException ioException =
         assertThrows(
-            CredentialHelperException.class,
-            () -> getCredentialsFromHelper("https://timeout.example.com"));
-    assertThat(e).hasMessageThat().contains("process timed out");
+            IOException.class, () -> getCredentialsFromHelper("https://timeout.example.com"));
+    assertThat(ioException).hasMessageThat().contains("process timed out");
   }
 }

src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java

@@ -19,7 +19,6 @@
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.runtime.CommandLinePathFactory.CommandLinePathFactoryException;
 import com.google.devtools.build.lib.vfs.DigestHashFunction;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
@@ -100,9 +99,6 @@ public void createWithNamedRoot() throws Exception {
         .isEqualTo(filesystem.getPath("/path/to/output/base/foo"));
     assertThat(factory.create(ImmutableMap.of(), "%output_base%/foo/bar"))
         .isEqualTo(filesystem.getPath("/path/to/output/base/foo/bar"));
-
-    assertThat(factory.create(ImmutableMap.of(), "%workspace%//foo//bar"))
-        .isEqualTo(filesystem.getPath("/path/to/workspace/foo/bar"));
   }
 
   @Test
@@ -112,11 +108,9 @@ public void pathLeakingOutsideOfRoot() {
             filesystem, ImmutableMap.of("a", filesystem.getPath("/path/to/a")));
 
     assertThrows(
-        CommandLinePathFactoryException.class,
-        () -> factory.create(ImmutableMap.of(), "%a%/../foo"));
+        IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "%a%/../foo"));
     assertThrows(
-        CommandLinePathFactoryException.class,
-        () -> factory.create(ImmutableMap.of(), "%a%/b/../.."));
+        IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "%a%/b/../.."));
   }
 
   @Test
@@ -126,21 +120,29 @@ public void unknownRoot() {
             filesystem, ImmutableMap.of("a", filesystem.getPath("/path/to/a")));
 
     assertThrows(
-        CommandLinePathFactoryException.class,
-        () -> factory.create(ImmutableMap.of(), "%workspace%/foo"));
+        IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "%workspace%/foo"));
     assertThrows(
-        CommandLinePathFactoryException.class,
+        IllegalArgumentException.class,
         () -> factory.create(ImmutableMap.of(), "%output_base%/foo"));
   }
 
+  @Test
+  public void rootWithDoubleSlash() {
+    CommandLinePathFactory factory =
+        new CommandLinePathFactory(
+            filesystem, ImmutableMap.of("a", filesystem.getPath("/path/to/a")));
+
+    assertThrows(
+        IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "%a%//foo"));
+  }
+
   @Test
   public void relativePathWithMultipleSegments() {
     CommandLinePathFactory factory = new CommandLinePathFactory(filesystem, ImmutableMap.of());
 
+    assertThrows(IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "a/b"));
     assertThrows(
-        CommandLinePathFactoryException.class, () -> factory.create(ImmutableMap.of(), "a/b"));
-    assertThrows(
-        CommandLinePathFactoryException.class, () -> factory.create(ImmutableMap.of(), "a/b/c/d"));
+        IllegalArgumentException.class, () -> factory.create(ImmutableMap.of(), "a/b/c/d"));
   }
 
   @Test

src/test/shell/bazel/remote/remote_execution_test.sh

@@ -3536,29 +3536,6 @@ EOF
   expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
 }
 
-function test_uploader_respect_no_cache_minimal() {
-  mkdir -p a
-  cat > a/BUILD <<EOF
-genrule(
-  name = 'foo',
-  outs = ["foo.txt"],
-  cmd = "echo \"foo bar\" > \$@",
-  tags = ["no-cache"],
-)
-EOF
-
-  bazel build \
-      --remote_cache=grpc://localhost:${worker_port} \
-      --remote_download_minimal \
-      --incompatible_remote_build_event_upload_respect_no_cache \
-      --build_event_json_file=bep.json \
-      //a:foo >& $TEST_log || fail "Failed to build"
-
-  cat bep.json > $TEST_log
-  expect_not_log "a:foo.*bytestream://" || fail "local files are converted"
-  expect_log "command.profile.gz.*bytestream://" || fail "should upload profile data"
-}
-
 function test_uploader_alias_action_respect_no_cache() {
   mkdir -p a
   cat > a/BUILD <<EOF

tools/android/BUILD.tools

@@ -118,7 +118,6 @@ sh_binary(
     name = "desugar_java8",
     srcs = ["desugar.sh"],
     data = ["//src/tools/android/java/com/google/devtools/build/android/desugar:Desugar"],
-    deps = ["@bazel_tools//tools/bash/runfiles"],
 )
 
 alias(