Revert "Release 5.3.0 (#16074)" (#16075)
This reverts commit 2077c54069ed062587591a073ad0f03d521083e6.
diff --git a/CODEOWNERS b/CODEOWNERS
index 9c34fcf..bd22af1 100644
--- a/CODEOWNERS
+++ b/CODEOWNERS
@@ -1 +1,2 @@
+* @ckolli5
* @kshyanashree
diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java
index db9b43b..e410c0b 100644
--- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java
@@ -77,7 +77,7 @@
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",
@@ -85,7 +85,7 @@
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 @@
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 @@
}
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:"
diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperException.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperException.java
deleted file mode 100644
index 7d8718f..0000000
--- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperException.java
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright 2022 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.authandtls.credentialhelper;
-
-import java.io.IOException;
-
-/** An {@link Exception} thrown while invoking a credential helper. */
-public class CredentialHelperException extends IOException {
- public CredentialHelperException(String message) {
- super(message);
- }
-
- public CredentialHelperException(String message, Throwable cause) {
- super(message, cause);
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java
index 15c12bb..0e3b79c 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java
+++ b/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 @@
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 @@
}
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 @@
/* 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;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD
index 13d4514..9afc158 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/util/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/util/BUILD
@@ -19,7 +19,6 @@
"//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",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
index c775040..8c4099a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java
+++ b/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 @@
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 @@
}
}
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();
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java
index 9994089..074bda7 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandLinePathFactory.java
+++ b/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 @@
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 @@
// 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");
}
diff --git a/src/test/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java b/src/test/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java
index 0db1127..4aeb459 100644
--- a/src/test/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelperTest.java
+++ b/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 @@
@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 @@
@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");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java b/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java
index dab8e39..223f39b 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/CommandLinePathFactoryTest.java
+++ b/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 @@
.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 @@
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 @@
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
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index e79b2df..f45f93d 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -3536,29 +3536,6 @@
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
diff --git a/tools/android/BUILD.tools b/tools/android/BUILD.tools
index 45ccf43..fd6f0ec 100644
--- a/tools/android/BUILD.tools
+++ b/tools/android/BUILD.tools
@@ -118,7 +118,6 @@
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(
diff --git a/tools/android/desugar.sh b/tools/android/desugar.sh
index fd24e34..74eee3d 100644
--- a/tools/android/desugar.sh
+++ b/tools/android/desugar.sh
@@ -17,31 +17,38 @@
# jdk.internal.lambda.dumpProxyClasses and configures Java 8 library rewriting
# through additional flags.
-RUNFILES="${RUNFILES:-$0.runfiles}"
-CHECK_FOR_EXE=0
-if [[ ! -d $RUNFILES ]]; then
- # Try the Windows path
- RUNFILES="${RUNFILES:-$0.exe.runfiles}"
- CHECK_FOR_EXE=1
-fi
-RUNFILES_MANIFEST_FILE="${RUNFILES_MANIFEST_FILE:-$RUNFILES/MANIFEST}"
-export JAVA_RUNFILES=$RUNFILES
-export RUNFILES_LIB_DEBUG=1
-# --- begin runfiles.bash initialization v2 ---
-# Copy-pasted from the Bazel Bash runfiles library v2.
-set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash
-source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
- source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
- source "$0.runfiles/$f" 2>/dev/null || \
- source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
- source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
- { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
-# --- end runfiles.bash initialization v2 ---
+# exit on errors and uninitialized variables
+set -eu
-if [[ $CHECK_FOR_EXE -eq 0 ]]; then
- DESUGAR="$(rlocation "bazel_tools/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar")"
+RUNFILES="${RUNFILES:-$0.runfiles}"
+RUNFILES_MANIFEST_FILE="${RUNFILES_MANIFEST_FILE:-$RUNFILES/MANIFEST}"
+
+IS_WINDOWS=false
+case "$(uname | tr [:upper:] [:lower:])" in
+msys*|mingw*|cygwin*)
+ IS_WINDOWS=true
+esac
+
+if "$IS_WINDOWS" && ! type rlocation &> /dev/null; then
+ function rlocation() {
+ # Use 'sed' instead of 'awk', so if the absolute path ($2) has spaces, it
+ # will be printed completely.
+ local result="$(grep "$1" "${RUNFILES_MANIFEST_FILE}" | head -1)"
+ # If the entry has a space, it is a mapping from a runfiles-path to absolute
+ # path, otherwise it resolves to itself.
+ echo "$result" | grep -q " " \
+ && echo "$result" | sed 's/^[^ ]* //' \
+ || echo "$result"
+ }
+fi
+
+# Find script to call:
+# Windows (in MANIFEST): <repository_name>/<path/to>/tool
+# Linux/MacOS (symlink): ${RUNFILES}/<repository_name>/<path/to>/tool
+if "$IS_WINDOWS"; then
+ DESUGAR="$(rlocation "[^/]*/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar")"
else
- DESUGAR="$(rlocation "bazel_tools/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar.exe")"
+ DESUGAR="$(find "${RUNFILES}" -path "*/src/tools/android/java/com/google/devtools/build/android/desugar/Desugar" | head -1)"
fi
readonly TMPDIR="$(mktemp -d)"
@@ -111,4 +118,3 @@
"--jvm_flag=-Djdk.internal.lambda.dumpProxyClasses=${TMPDIR}" \
"$@" \
"${DESUGAR_JAVA8_LIBS_CONFIG[@]}"
-