Support `layering_check` with C++ path mapping
Users can opt into path mapping for C++ module map actions via `--modify_execution_info=CppModuleMap=+supports-path-mapping`. This is achieved by mapping paths in the module map files as well as converting the sequence variable for module map paths to a new structured `ArtifactSequenceVariable`.
Also makes it so that `ExecutionServer` gracefully handles failing commands instead of crashing.
Closes #22957.
PiperOrigin-RevId: 675073116
Change-Id: I13835c7fb01354a89ec5fd141cf892c6b733efe4
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
index 817a525..9d02b70 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
@@ -574,8 +574,8 @@
}
}
- /** @return modules maps from direct dependencies. */
- public Iterable<Artifact> getDirectModuleMaps() {
+ /** Returns modules maps from direct dependencies. */
+ public ImmutableList<Artifact> getDirectModuleMaps() {
return directModuleMaps;
}
@@ -588,8 +588,8 @@
}
/**
- * @return all declared headers of the current module if the current target is compiled as a
- * module.
+ * Returns all declared headers of the current module if the current target is compiled as a
+ * module.
*/
ImmutableList<Artifact> getHeaderModuleSrcs(boolean separateModule) {
if (separateModule) {
@@ -644,7 +644,7 @@
headerTokens.build());
}
- /** @return the C++ module map of the owner. */
+ /** Returns the C++ module map of the owner. */
public CppModuleMap getCppModuleMap() {
return cppModuleMap;
}
@@ -1147,7 +1147,7 @@
* Gathers data about the PIC and no-PIC .pcm files belonging to this context and the associated
* information about the headers, e.g. modular vs. textual headers and pre-grepped header files.
*
- * <p>This also implements a data structure very similar to NestedSet, but chosing slightly
+ * <p>This also implements a data structure very similar to NestedSet, but choosing slightly
* different trade-offs to account for the specific data stored in here, specifically, we know
* that there is going to be a single entry in every node of the DAG. Contrary to NestedSet, we
* reuse memoization data from dependencies to conserve both runtime and memory. Experiments have
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java
index 3b00657..5309498 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.analysis.LicensesProvider.TargetLicense;
import com.google.devtools.build.lib.analysis.LicensesProviderImpl;
import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.actions.PathMappers;
import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.starlark.Args;
@@ -419,7 +420,11 @@
compiledModule,
moduleMapHomeIsCwd,
generateSubmodules,
- withoutExternDependencies));
+ withoutExternDependencies,
+ PathMappers.getOutputPathsMode(ruleContext.getConfiguration()),
+ ruleContext
+ .getConfiguration()
+ .modifiedExecutionInfo(ImmutableMap.of(), CppModuleMapAction.MNEMONIC)));
}
@SerializationConstant @VisibleForSerialization
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java
index 7150b1d..c725d4d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java
@@ -1185,6 +1185,54 @@
}
}
+ @Immutable
+ private static final class ArtifactSetSequence extends VariableValueAdapter {
+ private final NestedSet<Artifact> values;
+
+ private ArtifactSetSequence(NestedSet<Artifact> values) {
+ Preconditions.checkNotNull(values);
+ this.values = values;
+ }
+
+ @Override
+ public ImmutableList<VariableValue> getSequenceValue(
+ String variableName, PathMapper pathMapper) {
+ ImmutableList<Artifact> valuesList = values.toList();
+ ImmutableList.Builder<VariableValue> sequences =
+ ImmutableList.builderWithExpectedSize(valuesList.size());
+ for (Artifact value : valuesList) {
+ sequences.add(new StringValue(pathMapper.getMappedExecPathString(value)));
+ }
+ return sequences.build();
+ }
+
+ @Override
+ public String getVariableTypeName() {
+ return Sequence.SEQUENCE_VARIABLE_TYPE_NAME;
+ }
+
+ @Override
+ public boolean isTruthy() {
+ return !values.isEmpty();
+ }
+
+ @Override
+ public boolean equals(Object other) {
+ if (this == other) {
+ return true;
+ }
+ if (!(other instanceof ArtifactSetSequence otherArtifacts)) {
+ return false;
+ }
+ return values.shallowEquals(otherArtifacts.values);
+ }
+
+ @Override
+ public int hashCode() {
+ return values.shallowHashCode();
+ }
+ }
+
/**
* Single structure value. Be careful not to create sequences of single structures, as the memory
* overhead is prohibitively big.
@@ -1497,6 +1545,19 @@
}
/**
+ * Add a sequence variable that expands {@code name} to {@link Artifact} {@code values}.
+ *
+ * <p>Accepts values as NestedSet. Nested set is stored directly, not cloned, not flattened.
+ */
+ @CanIgnoreReturnValue
+ public Builder addArtifactSequenceVariable(String name, NestedSet<Artifact> values) {
+ checkVariableNotPresentAlready(name);
+ Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name);
+ variablesMap.put(name, new ArtifactSetSequence(values));
+ return this;
+ }
+
+ /**
* Add a variable built using {@code VariableValueBuilder} api that expands {@code name} to the
* value returned by the {@code builder}.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
index 4f62040..54f8257 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
@@ -162,7 +162,7 @@
Artifact diagnosticsFile,
ImmutableList<VariablesExtension> variablesExtensions,
ImmutableMap<String, String> additionalBuildVariables,
- Iterable<Artifact> directModuleMaps,
+ ImmutableList<Artifact> directModuleMaps,
NestedSet<PathFragment> includeDirs,
NestedSet<PathFragment> quoteIncludeDirs,
NestedSet<PathFragment> systemIncludeDirs,
@@ -229,7 +229,7 @@
Artifact diagnosticsFile,
ImmutableList<VariablesExtension> variablesExtensions,
ImmutableMap<String, String> additionalBuildVariables,
- Iterable<Artifact> directModuleMaps,
+ ImmutableList<Artifact> directModuleMaps,
NestedSet<String> includeDirs,
NestedSet<String> quoteIncludeDirs,
NestedSet<String> systemIncludeDirs,
@@ -297,7 +297,7 @@
Artifact diagnosticsFile,
ImmutableList<VariablesExtension> variablesExtensions,
ImmutableMap<String, String> additionalBuildVariables,
- Iterable<Artifact> directModuleMaps,
+ ImmutableList<Artifact> directModuleMaps,
NestedSet<PathFragment> includeDirs,
NestedSet<PathFragment> quoteIncludeDirs,
NestedSet<PathFragment> systemIncludeDirs,
@@ -471,7 +471,7 @@
boolean isUsingMemProf,
List<VariablesExtension> variablesExtensions,
Map<String, String> additionalBuildVariables,
- Iterable<Artifact> directModuleMaps,
+ ImmutableList<Artifact> directModuleMaps,
ImmutableList<PathFragment> includeDirs,
ImmutableList<PathFragment> quoteIncludeDirs,
ImmutableList<PathFragment> systemIncludeDirs,
@@ -507,7 +507,7 @@
boolean isUsingMemProf,
List<VariablesExtension> variablesExtensions,
Map<String, String> additionalBuildVariables,
- Iterable<Artifact> directModuleMaps,
+ ImmutableList<Artifact> directModuleMaps,
NestedSet<PathFragment> includeDirs,
NestedSet<PathFragment> quoteIncludeDirs,
NestedSet<PathFragment> systemIncludeDirs,
@@ -526,9 +526,9 @@
buildVariables.addStringVariable(MODULE_NAME.getVariableName(), cppModuleMap.getName());
buildVariables.addArtifactVariable(
MODULE_MAP_FILE.getVariableName(), cppModuleMap.getArtifact());
- buildVariables.addStringSequenceVariable(
+ buildVariables.addArtifactSequenceVariable(
DEPENDENT_MODULE_MAP_FILES.getVariableName(),
- Iterables.transform(directModuleMaps, Artifact::getExecPathString));
+ NestedSetBuilder.wrap(Order.STABLE_ORDER, directModuleMaps));
}
if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) {
// Module inputs will be set later when the action is executed.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
index bc37093..8ffbe6c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
@@ -16,15 +16,26 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSortedMap;
+import com.google.common.collect.Interner;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactExpander;
+import com.google.devtools.build.lib.actions.CommandLineExpansionException;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
+import com.google.devtools.build.lib.actions.PathMapper;
import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
import com.google.devtools.build.lib.analysis.actions.DeterministicWriter;
+import com.google.devtools.build.lib.analysis.actions.PathMappers;
+import com.google.devtools.build.lib.analysis.config.CoreOptions.OutputPathsMode;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -35,6 +46,8 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
import javax.annotation.Nullable;
/**
@@ -43,8 +56,11 @@
*/
@Immutable
public final class CppModuleMapAction extends AbstractFileWriteAction {
+ public static final String MNEMONIC = "CppModuleMap";
private static final String GUID = "4f407081-1951-40c1-befc-d6b4daff5de3";
+ private static final Interner<ImmutableSortedMap<String, String>> executionInfoInterner =
+ BlazeInterners.newWeakInterner();
// C++ module map of the current target
private final CppModuleMap cppModuleMap;
@@ -65,6 +81,7 @@
private final boolean compiledModule;
private final boolean generateSubmodules;
private final boolean externDependencies;
+ private final ImmutableSortedMap<String, String> executionInfo;
public CppModuleMapAction(
ActionOwner owner,
@@ -77,7 +94,9 @@
boolean compiledModule,
boolean moduleMapHomeIsCwd,
boolean generateSubmodules,
- boolean externDependencies) {
+ boolean externDependencies,
+ OutputPathsMode outputPathsMode,
+ ImmutableMap<String, String> executionInfo) {
super(
owner,
NestedSetBuilder.<Artifact>stableOrder()
@@ -95,6 +114,22 @@
this.compiledModule = compiledModule;
this.generateSubmodules = generateSubmodules;
this.externDependencies = externDependencies;
+ // Save memory by storing outputPathsMode implicitly via the presence of
+ // ExecutionRequirements.SUPPORTS_PATH_MAPPING in the key set. Path mapping is only effectively
+ // enabled if the key is present *and* the mode is set to STRIP, so if the latter is not the
+ // case, we can safely not store the key.
+ Map<String, String> storedExecutionInfo;
+ if (outputPathsMode == OutputPathsMode.STRIP) {
+ storedExecutionInfo = executionInfo;
+ } else {
+ storedExecutionInfo =
+ Maps.filterKeys(
+ executionInfo, k -> !k.equals(ExecutionRequirements.SUPPORTS_PATH_MAPPING));
+ }
+ this.executionInfo =
+ storedExecutionInfo.isEmpty()
+ ? ImmutableSortedMap.of()
+ : executionInfoInterner.intern(ImmutableSortedMap.copyOf(storedExecutionInfo));
}
@Override
@@ -110,9 +145,16 @@
@Override
public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
final ArtifactExpander artifactExpander = ctx.getArtifactExpander();
+ // TODO: It is possible that compile actions consuming the module map have path mapping disabled
+ // due to inputs conflicting across configurations. Since these inputs aren't inputs of the
+ // module map action, the generated map still contains mapped paths, which then results in
+ // compilation failures. This should be very rare as #include doesn't allow to disambiguate
+ // between headers from different configurations but with identical root-relative paths.
+ final PathMapper pathMapper =
+ PathMappers.create(this, getOutputPathsMode(), /* isStarlarkAction= */ false);
return out -> {
OutputStreamWriter content = new OutputStreamWriter(out, StandardCharsets.ISO_8859_1);
- PathFragment fragment = cppModuleMap.getArtifact().getExecPath();
+ PathFragment fragment = pathMapper.map(cppModuleMap.getArtifact().getExecPath());
int segmentsToExecPath = fragment.segmentCount() - 1;
Optional<Artifact> umbrellaHeader = cppModuleMap.getUmbrellaHeader();
String leadingPeriods = moduleMapHomeIsCwd ? "" : "../".repeat(segmentsToExecPath);
@@ -134,7 +176,8 @@
leadingPeriods,
/* canCompile= */ false,
deduper,
- /*isUmbrellaHeader*/ true);
+ /*isUmbrellaHeader*/ true,
+ pathMapper);
} else {
for (Artifact artifact : expandedHeaders(artifactExpander, publicHeaders)) {
appendHeader(
@@ -144,7 +187,8 @@
leadingPeriods,
/* canCompile= */ true,
deduper,
- /*isUmbrellaHeader*/ false);
+ /*isUmbrellaHeader*/ false,
+ pathMapper);
}
for (Artifact artifact : expandedHeaders(artifactExpander, privateHeaders)) {
appendHeader(
@@ -154,7 +198,8 @@
leadingPeriods,
/* canCompile= */ true,
deduper,
- /*isUmbrellaHeader*/ false);
+ /*isUmbrellaHeader*/ false,
+ pathMapper);
}
for (Artifact artifact : separateModuleHdrs) {
appendHeader(
@@ -164,7 +209,8 @@
leadingPeriods,
/* canCompile= */ false,
deduper,
- /*isUmbrellaHeader*/ false);
+ /*isUmbrellaHeader*/ false,
+ pathMapper);
}
for (PathFragment additionalExportedHeader : additionalExportedHeaders) {
appendHeader(
@@ -174,7 +220,8 @@
leadingPeriods,
/*canCompile*/ false,
deduper,
- /*isUmbrellaHeader*/ false);
+ /*isUmbrellaHeader*/ false,
+ pathMapper);
}
}
for (CppModuleMap dep : dependencies) {
@@ -196,7 +243,8 @@
leadingPeriods,
/* canCompile= */ true,
deduper,
- /*isUmbrellaHeader*/ false);
+ /*isUmbrellaHeader*/ false,
+ pathMapper);
}
for (CppModuleMap dep : dependencies) {
content.append(" use \"").append(dep.getName()).append("\"\n");
@@ -211,7 +259,7 @@
.append(dep.getName())
.append("\" \"")
.append(leadingPeriods)
- .append(dep.getArtifact().getExecPathString())
+ .append(pathMapper.getMappedExecPathString(dep.getArtifact()))
.append("\"");
}
}
@@ -219,8 +267,8 @@
};
}
- private static Iterable<Artifact> expandedHeaders(ArtifactExpander artifactExpander,
- Iterable<Artifact> unexpandedHeaders) {
+ private static ImmutableList<Artifact> expandedHeaders(
+ ArtifactExpander artifactExpander, Iterable<Artifact> unexpandedHeaders) {
List<Artifact> expandedHeaders = new ArrayList<>();
for (Artifact unexpandedHeader : unexpandedHeaders) {
if (unexpandedHeader.isTreeArtifact()) {
@@ -233,9 +281,17 @@
return ImmutableList.copyOf(expandedHeaders);
}
- private void appendHeader(Appendable content, String visibilitySpecifier,
- PathFragment path, String leadingPeriods, boolean canCompile, HashSet<PathFragment> deduper,
- boolean isUmbrellaHeader) throws IOException {
+ private void appendHeader(
+ Appendable content,
+ String visibilitySpecifier,
+ PathFragment unmappedPath,
+ String leadingPeriods,
+ boolean canCompile,
+ Set<PathFragment> deduper,
+ boolean isUmbrellaHeader,
+ PathMapper pathMapper)
+ throws IOException {
+ PathFragment path = pathMapper.map(unmappedPath);
if (deduper.contains(path)) {
return;
}
@@ -268,14 +324,15 @@
@Override
public String getMnemonic() {
- return "CppModuleMap";
+ return MNEMONIC;
}
@Override
protected void computeKey(
ActionKeyContext actionKeyContext,
@Nullable ArtifactExpander artifactExpander,
- Fingerprint fp) {
+ Fingerprint fp)
+ throws CommandLineExpansionException, InterruptedException {
fp.addString(GUID);
fp.addInt(privateHeaders.size());
for (Artifact artifact : privateHeaders) {
@@ -308,6 +365,25 @@
fp.addBoolean(compiledModule);
fp.addBoolean(generateSubmodules);
fp.addBoolean(externDependencies);
+ PathMappers.addToFingerprint(
+ getMnemonic(),
+ getExecutionInfo(),
+ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
+ actionKeyContext,
+ getOutputPathsMode(),
+ fp);
+ }
+
+ @Override
+ public ImmutableMap<String, String> getExecutionInfo() {
+ return executionInfo;
+ }
+
+ private OutputPathsMode getOutputPathsMode() {
+ // See comment in the constructor for how outputPathsMode is stored implicitly.
+ return executionInfo.containsKey(ExecutionRequirements.SUPPORTS_PATH_MAPPING)
+ ? OutputPathsMode.STRIP
+ : OutputPathsMode.OFF;
}
@VisibleForTesting
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
index cdc2026..4e032ca 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
@@ -496,6 +496,7 @@
deps = [
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
+ "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/test/java/com/google/devtools/build/lib/actions/util",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapActionTest.java
index 4d53fbb..5762fde 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapActionTest.java
@@ -16,11 +16,13 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
+import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
@@ -60,15 +62,17 @@
return new CppModuleMapAction(
ActionsTestUtil.NULL_ACTION_OWNER,
cppModuleMap,
- /*privateHeaders=*/ ImmutableList.of(),
- /*publicHeaders=*/ ImmutableList.of(),
+ /* privateHeaders= */ ImmutableList.of(),
+ /* publicHeaders= */ ImmutableList.of(),
ImmutableList.copyOf(dependencies),
- /*additionalExportedHeaders=*/ ImmutableList.of(),
- /*separateModuleHeaders=*/ ImmutableList.of(),
- /*compiledModule=*/ false,
- /*moduleMapHomeIsCwd=*/ false,
- /*generateSubmodules=*/ false,
- /*externDependencies=*/ false);
+ /* additionalExportedHeaders= */ ImmutableList.of(),
+ /* separateModuleHeaders= */ ImmutableList.of(),
+ /* compiledModule= */ false,
+ /* moduleMapHomeIsCwd= */ false,
+ /* generateSubmodules= */ false,
+ /* externDependencies= */ false,
+ CoreOptions.OutputPathsMode.OFF,
+ /* executionInfo= */ ImmutableMap.of());
}
private Artifact createOutputArtifact(String rootRelativePath) {
diff --git a/src/test/shell/bazel/path_mapping_test.sh b/src/test/shell/bazel/path_mapping_test.sh
index d16a38c..239d6e9 100755
--- a/src/test/shell/bazel/path_mapping_test.sh
+++ b/src/test/shell/bazel/path_mapping_test.sh
@@ -623,9 +623,9 @@
bazel run \
--verbose_failures \
--experimental_output_paths=strip \
- --modify_execution_info=CppCompile=+supports-path-mapping \
+ --modify_execution_info=CppCompile=+supports-path-mapping,CppModuleMap=+supports-path-mapping \
--remote_executor=grpc://localhost:${worker_port} \
- --features=-module_maps \
+ --features=layering_check \
"//$pkg:main" &>"$TEST_log" || fail "Expected success"
expect_log 'Hello, lib1!'
@@ -636,9 +636,9 @@
bazel run \
--verbose_failures \
--experimental_output_paths=strip \
- --modify_execution_info=CppCompile=+supports-path-mapping \
+ --modify_execution_info=CppCompile=+supports-path-mapping,CppModuleMap=+supports-path-mapping \
--remote_executor=grpc://localhost:${worker_port} \
- --features=-module_maps \
+ --features=layering_check \
"//$pkg:transitioned_main" &>"$TEST_log" || fail "Expected success"
expect_log 'Hi there, lib1!'
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
index 22c893c..cbbb55e 100644
--- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
+++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
@@ -175,11 +175,7 @@
return;
}
((ServerCallStreamObserver<Operation>) responseObserver)
- .setOnCancelHandler(
- () -> {
- future.cancel(true);
- operationsCache.remove(opName);
- });
+ .setOnCancelHandler(() -> operationsCache.remove(opName));
waitExecution(opName, future, responseObserver);
}
@@ -241,11 +237,7 @@
executorService.submit(() -> execute(context, request, opName));
operationsCache.put(opName, future);
((ServerCallStreamObserver<Operation>) responseObserver)
- .setOnCancelHandler(
- () -> {
- future.cancel(true);
- operationsCache.remove(opName);
- });
+ .setOnCancelHandler(() -> operationsCache.remove(opName));
// Send the first operation.
responseObserver.onNext(Operation.newBuilder().setName(opName).build());
// When the operation completes, send the result.
