Make bes_best_effort a no-op
The current semantics of the flag is to allow BES upload to continue
past the nominal end of the build invocation, and possibly overlapping
a subsequent build invocation. This conflicts with file uploads, which
must read the file before it is removed or modified by the subsequent
build invocation. On Linux, we could just open a file handle, but this
isn't possible on Windows.
We decided to make the flag a no-op for now. Note that the default is
already set to false. We may resurrect this option in the future if
there's a strong use case for it, possibly with slightly different
semantics.
PiperOrigin-RevId: 199620392
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
index a4e2957..5138792 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
@@ -176,14 +176,10 @@
commandLineReporter,
startupOptionsProvider);
} catch (Exception e) {
- if (besOptions.besBestEffort) {
- commandLineReporter.handle(Event.warn(format(UPLOAD_FAILED_MESSAGE, e.getMessage())));
- } else {
- commandLineReporter.handle(Event.error(format(UPLOAD_FAILED_MESSAGE, e.getMessage())));
- moduleEnvironment.exit(new AbruptExitException(
- "Failed while creating BuildEventTransport", ExitCode.PUBLISH_ERROR));
- return null;
- }
+ commandLineReporter.handle(Event.error(format(UPLOAD_FAILED_MESSAGE, e.getMessage())));
+ moduleEnvironment.exit(new AbruptExitException(
+ "Failed while creating BuildEventTransport", ExitCode.PUBLISH_ERROR));
+ return null;
}
ImmutableSet<BuildEventTransport> bepTransports =
@@ -247,7 +243,6 @@
new BuildEventServiceTransport(
createBesClient(besOptions, authTlsOptions),
besOptions.besTimeout,
- besOptions.besBestEffort,
besOptions.besLifecycleEvents,
buildRequestId,
invocationId,
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceOptions.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceOptions.java
index df9436d..ce99eee 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceOptions.java
@@ -51,14 +51,16 @@
public Duration besTimeout;
@Option(
- name = "bes_best_effort",
- defaultValue = "false",
- documentationCategory = OptionDocumentationCategory.LOGGING,
- effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
- help =
- "Specifies whether a failure to upload the BES protocol should also result in a build "
- + "failure. If 'false', bazel exits with ExitCode.PUBLISH_ERROR. (defaults to 'false')."
- )
+ name = "bes_best_effort",
+ defaultValue = "false",
+ deprecationWarning =
+ "BES best effort upload has been removed. The flag has no more "
+ + "functionality attached to it and will be removed in a future release.",
+ documentationCategory = OptionDocumentationCategory.LOGGING,
+ effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
+ help =
+ "BES best effort upload has been removed. The flag has no more "
+ + "functionality attached to it and will be removed in a future release.")
public boolean besBestEffort;
@Option(
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
index ee5c654..42c363b 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
@@ -18,7 +18,6 @@
import static com.google.common.util.concurrent.MoreExecutors.listeningDecorator;
import static com.google.devtools.build.lib.events.EventKind.ERROR;
import static com.google.devtools.build.lib.events.EventKind.INFO;
-import static com.google.devtools.build.lib.events.EventKind.WARNING;
import static com.google.devtools.build.v1.BuildEvent.EventCase.COMPONENT_STREAM_FINISHED;
import static com.google.devtools.build.v1.BuildStatus.Result.COMMAND_FAILED;
import static com.google.devtools.build.v1.BuildStatus.Result.COMMAND_SUCCEEDED;
@@ -90,7 +89,6 @@
private final ListeningExecutorService uploaderExecutorService;
private final Duration uploadTimeout;
private final boolean publishLifecycleEvents;
- private final boolean bestEffortUpload;
private final BuildEventServiceClient besClient;
private final BuildEventServiceProtoUtil besProtoUtil;
private final ModuleEnvironment moduleEnvironment;
@@ -125,7 +123,6 @@
public BuildEventServiceTransport(
BuildEventServiceClient besClient,
Duration uploadTimeout,
- boolean bestEffortUpload,
boolean publishLifecycleEvents,
String buildRequestId,
String invocationId,
@@ -137,16 +134,27 @@
EventHandler commandLineReporter,
@Nullable String projectId,
Set<String> keywords) {
- this(besClient, uploadTimeout, bestEffortUpload, publishLifecycleEvents, buildRequestId,
- invocationId, command, moduleEnvironment, clock, protocolOptions, pathConverter,
- commandLineReporter, projectId, keywords, new JavaSleeper());
+ this(
+ besClient,
+ uploadTimeout,
+ publishLifecycleEvents,
+ buildRequestId,
+ invocationId,
+ command,
+ moduleEnvironment,
+ clock,
+ protocolOptions,
+ pathConverter,
+ commandLineReporter,
+ projectId,
+ keywords,
+ new JavaSleeper());
}
@VisibleForTesting
public BuildEventServiceTransport(
BuildEventServiceClient besClient,
Duration uploadTimeout,
- boolean bestEffortUpload,
boolean publishLifecycleEvents,
String buildRequestId,
String invocationId,
@@ -177,7 +185,6 @@
this.pathConverter = pathConverter;
this.invocationResult = UNKNOWN_STATUS;
this.uploadTimeout = uploadTimeout;
- this.bestEffortUpload = bestEffortUpload;
this.sleeper = sleeper;
}
@@ -229,33 +236,17 @@
return;
}
- if (bestEffortUpload) {
- // TODO(buchgr): The code structure currently doesn't allow to enforce a timeout for
- // best effort upload.
- if (!uploadComplete.isDone()) {
- report(INFO, "Asynchronous Build Event Protocol upload.");
+ report(INFO, "Waiting for Build Event Protocol upload to finish.");
+ try {
+ if (Duration.ZERO.equals(uploadTimeout)) {
+ uploadComplete.get();
} else {
- Throwable uploadError = fromFuture(uploadComplete);
-
- if (uploadError != null) {
- report(WARNING, UPLOAD_FAILED_MESSAGE, uploadError.getMessage());
- } else {
- report(INFO, UPLOAD_SUCCEEDED_MESSAGE);
- }
+ uploadComplete.get(uploadTimeout.toMillis(), MILLISECONDS);
}
- } else {
- report(INFO, "Waiting for Build Event Protocol upload to finish.");
- try {
- if (Duration.ZERO.equals(uploadTimeout)) {
- uploadComplete.get();
- } else {
- uploadComplete.get(uploadTimeout.toMillis(), MILLISECONDS);
- }
- report(INFO, UPLOAD_SUCCEEDED_MESSAGE);
- } catch (Exception e) {
- uploadComplete.cancel(true);
- reportErrorAndFailBuild(e);
- }
+ report(INFO, UPLOAD_SUCCEEDED_MESSAGE);
+ } catch (Exception e) {
+ uploadComplete.cancel(true);
+ reportErrorAndFailBuild(e);
}
} finally {
shutdownFuture.set(null);
@@ -347,8 +338,6 @@
}
private void reportErrorAndFailBuild(Throwable t) {
- checkState(!bestEffortUpload);
-
String message = errorMessageFromException(t);
report(ERROR, message);
@@ -364,11 +353,7 @@
Throwable uploadError = fromFuture(uploadComplete);
if (uploadError != null) {
errorsReported = true;
- if (bestEffortUpload) {
- report(WARNING, UPLOAD_FAILED_MESSAGE, uploadError.getMessage());
- } else {
- reportErrorAndFailBuild(uploadError);
- }
+ reportErrorAndFailBuild(uploadError);
}
}
