Make --incompatible_remote_results_ignore_disk a no-op.

Disk cache should be always treated as local cache. The flag was introduced for migration and now it's the time to remove it. See #8216. PiperOrigin-RevId: 542881044 Change-Id: I1a6e6c995e83f10dfe7c812f697d037e5c5afb3f
bazelbuild · Jun 23, 2023 · c88f374 · c88f374
1 parent bf0ca1c
commit c88f374
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 60 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -286,23 +286,13 @@ public CachePolicy getReadCachePolicy(Spawn spawn) {
     if (useRemoteCache(remoteOptions)) {
       allowRemoteCache = remoteOptions.remoteAcceptCached && Spawns.mayBeCachedRemotely(spawn);
       if (useDiskCache(remoteOptions)) {
-        // Combined cache
-        if (remoteOptions.incompatibleRemoteResultsIgnoreDisk) {
-          // --incompatible_remote_results_ignore_disk is set. Disk cache is treated as local cache.
-          // Actions which are tagged with `no-remote-cache` can still hit the disk cache.
-          allowDiskCache = Spawns.mayBeCached(spawn);
-        } else {
-          // Disk cache is treated as a remote cache and disabled for `no-remote-cache`.
-          allowDiskCache = allowRemoteCache;
-        }
+        // Combined cache. Disk cache is treated as local cache. Actions which are tagged with
+        // `no-remote-cache` can still hit the disk cache.
+        allowDiskCache = Spawns.mayBeCached(spawn);
       }
     } else {
       // Disk cache only
-      if (remoteOptions.incompatibleRemoteResultsIgnoreDisk) {
-        allowDiskCache = Spawns.mayBeCached(spawn);
-      } else {
-        allowDiskCache = remoteOptions.remoteAcceptCached && Spawns.mayBeCached(spawn);
-      }
+      allowDiskCache = Spawns.mayBeCached(spawn);
     }
 
     return CachePolicy.create(allowRemoteCache, allowDiskCache);
@@ -321,24 +311,13 @@ public CachePolicy getWriteCachePolicy(Spawn spawn) {
           shouldUploadLocalResultsToRemoteCache(remoteOptions, spawn.getExecutionInfo())
               && remoteCache.actionCacheSupportsUpdate();
       if (useDiskCache(remoteOptions)) {
-        // Combined cache
-        if (remoteOptions.incompatibleRemoteResultsIgnoreDisk) {
-          // If --incompatible_remote_results_ignore_disk is set, we treat the disk cache part as
-          // local cache. Actions which are tagged with `no-remote-cache` can still hit the disk
-          // cache.
-          allowDiskCache = Spawns.mayBeCached(spawn);
-        } else {
-          // Otherwise, it's treated as a remote cache and disabled for `no-remote-cache`.
-          allowDiskCache = allowRemoteCache;
-        }
+        // Combined cache. Treat the disk cache part as local cache. Actions which are tagged with
+        // `no-remote-cache` can still hit the disk cache.
+        allowDiskCache = Spawns.mayBeCached(spawn);
       }
     } else {
       // Disk cache only
-      if (remoteOptions.incompatibleRemoteResultsIgnoreDisk) {
-        allowDiskCache = Spawns.mayBeCached(spawn);
-      } else {
-        allowDiskCache = remoteOptions.remoteUploadLocalResults && Spawns.mayBeCached(spawn);
-      }
+      allowDiskCache = Spawns.mayBeCached(spawn);
     }
 
     return CachePolicy.create(allowRemoteCache, allowDiskCache);

diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java
@@ -300,16 +300,7 @@ public RemoteBuildEventUploadModeConverter() {
       documentationCategory = OptionDocumentationCategory.REMOTE,
       effectTags = {OptionEffectTag.UNKNOWN},
       metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
-      help =
-          "If set to true, --noremote_upload_local_results and --noremote_accept_cached will not"
-              + " apply to the disk cache. If both --disk_cache and --remote_cache are set"
-              + " (combined cache):\n"
-              + "\t--noremote_upload_local_results will cause results to be written to the disk"
-              + " cache, but not uploaded to the remote cache.\n"
-              + "\t--noremote_accept_cached will result in Bazel checking for results in the disk"
-              + " cache, but not in the remote cache.\n"
-              + "\tno-remote-exec actions can hit the disk cache.\n"
-              + "See #8216 for details.")
+      help = "No-op")
   public boolean incompatibleRemoteResultsIgnoreDisk;
 
   @Option(

diff --git a/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh b/src/test/shell/bazel/remote/remote_build_event_uploader_test.sh
@@ -236,7 +236,6 @@ EOF
   bazel build \
       --remote_cache=grpc://localhost:${worker_port} \
       --disk_cache=$cache_dir \
-      --incompatible_remote_results_ignore_disk \
       --remote_upload_local_results=false \
       --remote_build_event_upload=minimal \
       --build_event_json_file=$BEP_JSON \
@@ -266,7 +265,6 @@ EOF
   bazel build \
       --remote_cache=grpc://localhost:${worker_port} \
       --disk_cache=$cache_dir \
-      --incompatible_remote_results_ignore_disk \
       --remote_upload_local_results=false \
       --remote_build_event_upload=all \
       --build_event_json_file=$BEP_JSON \

diff --git a/src/test/shell/bazel/remote/remote_execution_http_test.sh b/src/test/shell/bazel/remote/remote_execution_http_test.sh
@@ -349,21 +349,21 @@ EOF
   mkdir $cache
 
   # Build and push to disk cache but not http cache
-  bazel build $disk_flags $http_flags --incompatible_remote_results_ignore_disk=true --noremote_upload_local_results //a:test \
+  bazel build $disk_flags $http_flags --noremote_upload_local_results //a:test \
     || fail "Failed to build //a:test with combined disk http cache"
   cp -f bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected
 
   # Fetch from disk cache
   bazel clean
-  bazel build $disk_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_upload_local_results &> $TEST_log \
+  bazel build $disk_flags //a:test --noremote_upload_local_results &> $TEST_log \
     || fail "Failed to fetch //a:test from disk cache"
   expect_log "1 disk cache hit" "Fetch from disk cache failed"
   diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
     || fail "Disk cache generated different result"
 
   # No cache result from http cache, rebuild target
   bazel clean
-  bazel build $http_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_upload_local_results &> $TEST_log \
+  bazel build $http_flags //a:test --noremote_upload_local_results &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_not_log "1 remote cache hit" "Should not get cache hit from http cache"
   expect_log "1 .*-sandbox" "Rebuild target failed"
@@ -375,7 +375,7 @@ EOF
 
   # No cache result from http cache, rebuild target, and upload result to http cache
   bazel clean
-  bazel build $http_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_accept_cached &> $TEST_log \
+  bazel build $http_flags //a:test --noremote_accept_cached &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_not_log "1 remote cache hit" "Should not get cache hit from http cache"
   expect_log "1 .*-sandbox" "Rebuild target failed"
@@ -384,7 +384,7 @@ EOF
 
   # No cache result from http cache, rebuild target, and upload result to disk cache
   bazel clean
-  bazel build $disk_flags $http_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_accept_cached &> $TEST_log \
+  bazel build $disk_flags $http_flags //a:test --noremote_accept_cached &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_not_log "1 remote cache hit" "Should not get cache hit from http cache"
   expect_log "1 .*-sandbox" "Rebuild target failed"
@@ -393,7 +393,7 @@ EOF
 
   # Fetch from disk cache
   bazel clean
-  bazel build $disk_flags $http_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_accept_cached &> $TEST_log \
+  bazel build $disk_flags $http_flags //a:test --noremote_accept_cached &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_log "1 disk cache hit" "Fetch from disk cache failed"
   diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \

diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -1360,10 +1360,8 @@ function test_combined_disk_remote_exec_with_flag_combinations() {
      # ensure CAS entries get uploaded even when action entries don't.
      "--noremote_upload_local_results"
      "--remote_upload_local_results"
-     # we should see no cache hits  [incompatible_remote_results_ignore_disk=false is default]
-     "--noremote_accept_cached"
      # Should be some disk cache hits, just not remote.
-     "--noremote_accept_cached --incompatible_remote_results_ignore_disk"
+     "--noremote_accept_cached"
   )
 
   for flags in "${testcases[@]}"; do
@@ -1576,21 +1574,21 @@ EOF
   mkdir $cache
 
   # Build and push to disk cache but not grpc cache
-  bazel build $disk_flags $grpc_flags --incompatible_remote_results_ignore_disk=true --noremote_upload_local_results //a:test \
+  bazel build $disk_flags $grpc_flags --noremote_upload_local_results //a:test \
     || fail "Failed to build //a:test with combined disk grpc cache"
   cp -f bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected
 
   # Fetch from disk cache
   bazel clean
-  bazel build $disk_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_upload_local_results &> $TEST_log \
+  bazel build $disk_flags //a:test --noremote_upload_local_results &> $TEST_log \
     || fail "Failed to fetch //a:test from disk cache"
   expect_log "1 disk cache hit" "Fetch from disk cache failed"
   diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
     || fail "Disk cache generated different result"
 
   # No cache result from grpc cache, rebuild target
   bazel clean
-  bazel build $grpc_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_upload_local_results &> $TEST_log \
+  bazel build $grpc_flags //a:test --noremote_upload_local_results &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_not_log "1 remote cache hit" "Should not get cache hit from grpc cache"
   expect_log "1 .*-sandbox" "Rebuild target failed"
@@ -1602,7 +1600,7 @@ EOF
 
   # No cache result from grpc cache, rebuild target, and upload result to grpc cache
   bazel clean
-  bazel build $grpc_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_accept_cached &> $TEST_log \
+  bazel build $grpc_flags //a:test --noremote_accept_cached &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_not_log "1 remote cache hit" "Should not get cache hit from grpc cache"
   expect_log "1 .*-sandbox" "Rebuild target failed"
@@ -1611,7 +1609,7 @@ EOF
 
   # No cache result from grpc cache, rebuild target, and upload result to disk cache
   bazel clean
-  bazel build $disk_flags $grpc_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_accept_cached &> $TEST_log \
+  bazel build $disk_flags $grpc_flags //a:test --noremote_accept_cached &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_not_log "1 remote cache hit" "Should not get cache hit from grpc cache"
   expect_log "1 .*-sandbox" "Rebuild target failed"
@@ -1620,7 +1618,7 @@ EOF
 
   # Fetch from disk cache
   bazel clean
-  bazel build $disk_flags $grpc_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_accept_cached &> $TEST_log \
+  bazel build $disk_flags $grpc_flags //a:test --noremote_accept_cached &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_log "1 disk cache hit" "Fetch from disk cache failed"
   diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
@@ -1735,21 +1733,21 @@ EOF
   mkdir $cache
 
   # Build and push to disk cache but not remote cache
-  bazel build $disk_flags $grpc_flags --incompatible_remote_results_ignore_disk=true //a:test \
+  bazel build $disk_flags $grpc_flags //a:test \
     || fail "Failed to build //a:test with combined cache"
   cp -f bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected
 
   # Fetch from disk cache
   bazel clean
-  bazel build $disk_flags //a:test --incompatible_remote_results_ignore_disk=true &> $TEST_log \
+  bazel build $disk_flags //a:test &> $TEST_log \
     || fail "Failed to fetch //a:test from disk cache"
   expect_log "1 disk cache hit" "Fetch from disk cache failed"
   diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
     || fail "Disk cache generated different result"
 
   # No cache result from grpc cache, rebuild target
   bazel clean
-  bazel build $grpc_flags //a:test --incompatible_remote_results_ignore_disk=true &> $TEST_log \
+  bazel build $grpc_flags //a:test &> $TEST_log \
     || fail "Failed to build //a:test"
   expect_not_log "1 remote cache hit" "Should not get cache hit from grpc cache"
   diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
@@ -1775,7 +1773,6 @@ EOF
   bazel build \
     --disk_cache=${cache_dir} \
     --remote_executor=grpc://localhost:${worker_port} \
-    --incompatible_remote_results_ignore_disk=true \
     //a:test &> $TEST_log \
     || fail "Failed to build //a:test"