Skip to content

Commit

Permalink
Remote: Invalidate actions if previous build used BwoB and remote cac…
Browse files Browse the repository at this point in the history
…he is changed from enabled to disabled.

Part of bazelbuild#14252.

PiperOrigin-RevId: 410243297
  • Loading branch information
coeuvre authored and copybara-github committed Nov 16, 2021
1 parent f7b8b72 commit ed68933
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -557,10 +557,14 @@ public RemoteOutputsStrategyConverter() {
/** The maximum size of an outbound message sent via a gRPC channel. */
public int maxOutboundMessageSize = 1024 * 1024;

public boolean isRemoteEnabled() {
return !Strings.isNullOrEmpty(remoteCache) || isRemoteExecutionEnabled();
/** Returns {@code true} if remote cache or disk cache is enabled. */
public boolean isRemoteCacheEnabled() {
return !Strings.isNullOrEmpty(remoteCache)
|| !(diskCache == null || diskCache.isEmpty())
|| isRemoteExecutionEnabled();
}

/** Returns {@code true} if remote execution is enabled. */
public boolean isRemoteExecutionEnabled() {
return !Strings.isNullOrEmpty(remoteExecutor);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory, Configur

private Map<String, String> lastRemoteDefaultExecProperties;
private RemoteOutputsMode lastRemoteOutputsMode;
private Boolean lastRemoteCacheEnabled;

class PathResolverFactoryImpl implements PathResolverFactory {
@Override
Expand Down Expand Up @@ -1727,10 +1728,25 @@ private void deleteActionsIfRemoteOptionsChanged(OptionsProvider options)
lastRemoteDefaultExecProperties != null
&& !remoteDefaultExecProperties.equals(lastRemoteDefaultExecProperties);
lastRemoteDefaultExecProperties = remoteDefaultExecProperties;

boolean remoteCacheEnabled = remoteOptions != null && remoteOptions.isRemoteCacheEnabled();
// If we have remote metadata from last build, and the remote cache is not
// enabled in this build, invalidate actions since they can't download those
// remote files.
//
// TODO(chiwang): Re-evaluate this after action rewinding is implemented in
// Bazel since we can treat that case as lost inputs.
if (lastRemoteOutputsMode != RemoteOutputsMode.ALL) {
needsDeletion |=
lastRemoteCacheEnabled != null && lastRemoteCacheEnabled && !remoteCacheEnabled;
}
lastRemoteCacheEnabled = remoteCacheEnabled;

RemoteOutputsMode remoteOutputsMode =
remoteOptions != null ? remoteOptions.remoteOutputsMode : RemoteOutputsMode.ALL;
needsDeletion |= lastRemoteOutputsMode != null && lastRemoteOutputsMode != remoteOutputsMode;
this.lastRemoteOutputsMode = remoteOutputsMode;

if (needsDeletion) {
memoizingEvaluator.delete(k -> SkyFunctions.ACTION_EXECUTION.equals(k.functionName()));
}
Expand Down
46 changes: 46 additions & 0 deletions src/test/shell/bazel/remote/remote_execution_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3204,4 +3204,50 @@ EOF
expect_not_log "WARNING: Writing to Remote Cache:"
}

function test_download_toplevel_when_turn_remote_cache_off() {
# Test that BwtB doesn't cause build failure if remote cache is disabled in a following build.
# See https://github.com/bazelbuild/bazel/issues/13882.

cat > .bazelrc <<EOF
build --verbose_failures
EOF
mkdir a
cat > a/BUILD <<'EOF'
genrule(
name = "producer",
outs = ["a.txt", "b.txt"],
cmd = "touch $(OUTS)",
)
genrule(
name = "consumer",
outs = ["out.txt"],
srcs = [":b.txt", "in.txt"],
cmd = "cat $(SRCS) > $@",
)
EOF
echo 'foo' > a/in.txt

# populate the cache
bazel build \
--remote_cache=grpc://localhost:${worker_port} \
--remote_download_toplevel \
//a:consumer >& $TEST_log || fail "Failed to populate the cache"

bazel clean >& $TEST_log || fail "Failed to clean"

# download top level outputs
bazel build \
--remote_cache=grpc://localhost:${worker_port} \
--remote_download_toplevel \
//a:consumer >& $TEST_log || fail "Failed to download outputs"
[[ -f bazel-bin/a/a.txt ]] || [[ -f bazel-bin/a/b.txt ]] \
&& fail "Expected outputs of producer are not downloaded"

# build without remote cache
echo 'bar' > a/in.txt
bazel build \
--remote_download_toplevel \
//a:consumer >& $TEST_log || fail "Failed to build without remote cache"
}

run_suite "Remote execution and remote cache tests"

0 comments on commit ed68933

Please sign in to comment.