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 #14252.

PiperOrigin-RevId: 410243297
  • Loading branch information
coeuvre authored and Copybara-Service 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.
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
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
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.