Remove --incompatible_disallow_unverified_http_downloads

#8607

RELNOTES: The flag --incompatible_disallow_unverified_http_downloads is removed.
PiperOrigin-RevId: 289675443

Change-Id: I8c75545ad9b0997c197e7280577a5725bfc05b84
PiperOrigin-RevId: 292346444

src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java

@@ -47,7 +47,6 @@
 import com.google.devtools.build.lib.runtime.ProcessWrapperUtil;
 import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
 import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult;
-import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skylarkbuildapi.repository.SkylarkRepositoryContextApi;
 import com.google.devtools.build.lib.syntax.Dict;
 import com.google.devtools.build.lib.syntax.EvalException;
@@ -636,7 +635,6 @@ public StructImpl download(
         getUrls(
             url,
             /* ensureNonEmpty= */ !allowFail,
-            env,
             /* checksumGiven= */ !Strings.isNullOrEmpty(sha256)
                 || !Strings.isNullOrEmpty(integrity));
     Optional<Checksum> checksum;
@@ -752,7 +750,6 @@ public StructImpl downloadAndExtract(
         getUrls(
             url,
             /* ensureNonEmpty= */ !allowFail,
-            env,
             /* checksumGiven= */ !Strings.isNullOrEmpty(sha256)
                 || !Strings.isNullOrEmpty(integrity));
     Optional<Checksum> checksum;
@@ -925,8 +922,7 @@ private static ImmutableList<String> checkAllUrls(Iterable<?> urlList) throws Ev
     return result.build();
   }
 
-  private static List<URL> getUrls(
-      Object urlOrList, boolean ensureNonEmpty, Environment env, boolean checksumGiven)
+  private static List<URL> getUrls(Object urlOrList, boolean ensureNonEmpty, boolean checksumGiven)
       throws RepositoryFunctionException, EvalException, InterruptedException {
     List<String> urlStrings;
     if (urlOrList instanceof String) {
@@ -937,7 +933,6 @@ private static List<URL> getUrls(
     if (ensureNonEmpty && urlStrings.isEmpty()) {
       throw new RepositoryFunctionException(new IOException("urls not set"), Transience.PERSISTENT);
     }
-    StarlarkSemantics semantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
     List<URL> urls = new ArrayList<>();
     for (String urlString : urlStrings) {
       URL url;
@@ -951,7 +946,7 @@ private static List<URL> getUrls(
         throw new RepositoryFunctionException(
             new IOException("Unsupported protocol: " + url.getProtocol()), Transience.PERSISTENT);
       }
-      if (semantics.incompatibleDisallowUnverifiedHttpDownloads() && !checksumGiven) {
+      if (!checksumGiven) {
         if (!Ascii.equalsIgnoreCase("http", url.getProtocol())) {
           urls.add(url);
         }

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -392,18 +392,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "to the rule definition, rather than the rule usage.")
   public boolean incompatibleVisibilityPrivateAttributesAtDefinition;
 
-  @Option(
-      name = "incompatible_disallow_unverified_http_downloads",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
-      metadataTags = {
-        OptionMetadataTag.INCOMPATIBLE_CHANGE,
-        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
-      },
-      help = "If set, disallow downloads via plain http if no checksum is given")
-  public boolean incompatibleDisallowUnverifiedHttpDownloads;
-
   @Option(
       name = "incompatible_new_actions_api",
       defaultValue = "true",
@@ -642,8 +630,6 @@ public StarlarkSemantics toSkylarkSemantics() {
             .incompatibleDisableDepsetItems(incompatibleDisableDepsetItems)
             .incompatibleDisallowEmptyGlob(incompatibleDisallowEmptyGlob)
             .incompatibleDisallowStructProviderSyntax(incompatibleDisallowStructProviderSyntax)
-            .incompatibleDisallowUnverifiedHttpDownloads(
-                incompatibleDisallowUnverifiedHttpDownloads)
             .incompatibleNewActionsApi(incompatibleNewActionsApi)
             .incompatibleNoAttrLicense(incompatibleNoAttrLicense)
             .incompatibleNoImplicitFileExport(incompatibleNoImplicitFileExport)

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -178,8 +178,6 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleDisallowStructProviderSyntax();
 
-  public abstract boolean incompatibleDisallowUnverifiedHttpDownloads();
-
   public abstract boolean incompatibleNewActionsApi();
 
   public abstract boolean incompatibleNoAttrLicense();
@@ -271,7 +269,6 @@ public static Builder builderWithDefaults() {
           .incompatibleDisableDepsetItems(false)
           .incompatibleDisallowEmptyGlob(false)
           .incompatibleDisallowStructProviderSyntax(false)
-          .incompatibleDisallowUnverifiedHttpDownloads(true)
           .incompatibleNewActionsApi(true)
           .incompatibleNoAttrLicense(true)
           .incompatibleNoImplicitFileExport(false)
@@ -343,8 +340,6 @@ public abstract static class Builder {
 
     public abstract Builder incompatibleDisallowStructProviderSyntax(boolean value);
 
-    public abstract Builder incompatibleDisallowUnverifiedHttpDownloads(boolean value);
-
     public abstract Builder incompatibleNewActionsApi(boolean value);
 
     public abstract Builder incompatibleNoAttrLicense(boolean value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -147,7 +147,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--incompatible_disable_third_party_license_checking=" + rand.nextBoolean(),
         "--incompatible_disallow_empty_glob=" + rand.nextBoolean(),
         "--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
-        "--incompatible_disallow_unverified_http_downloads=" + rand.nextBoolean(),
         "--incompatible_do_not_split_linking_cmdline=" + rand.nextBoolean(),
         "--incompatible_new_actions_api=" + rand.nextBoolean(),
         "--incompatible_no_attr_license=" + rand.nextBoolean(),
@@ -198,7 +197,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleDisableThirdPartyLicenseChecking(rand.nextBoolean())
         .incompatibleDisallowEmptyGlob(rand.nextBoolean())
         .incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
-        .incompatibleDisallowUnverifiedHttpDownloads(rand.nextBoolean())
         .incompatibleDoNotSplitLinkingCmdline(rand.nextBoolean())
         .incompatibleNewActionsApi(rand.nextBoolean())
         .incompatibleNoAttrLicense(rand.nextBoolean())

src/test/py/bazel/bazel_external_repository_test.py

@@ -83,29 +83,22 @@ def testNewHttpArchive(self):
     self.ScratchFile('third_party/BUILD')
     self.ScratchFile('third_party/six.BUILD', build_file)
 
-    exit_code, _, stderr = self.RunBazel([
-        'build', '--noincompatible_disallow_unverified_http_downloads',
-        '@six_archive//...'
-    ])
+    exit_code, _, stderr = self.RunBazel(['build', '@six_archive//...'])
     self.assertEqual(exit_code, 0, os.linesep.join(stderr))
 
     fetching_disabled_msg = 'fetching is disabled'
 
     # Changing the mtime of the BUILD file shouldn't invalidate it.
     os.utime(self.Path('third_party/six.BUILD'), (100, 200))
-    exit_code, _, stderr = self.RunBazel([
-        'build', '--noincompatible_disallow_unverified_http_downloads',
-        '--nofetch', '@six_archive//...'
-    ])
+    exit_code, _, stderr = self.RunBazel(
+        ['build', '--nofetch', '@six_archive//...'])
     self.assertEqual(exit_code, 0, os.linesep.join(stderr))
     self.assertNotIn(fetching_disabled_msg, os.linesep.join(stderr))
 
     # Check that --nofetch prints a warning if the BUILD file is changed.
     self.ScratchFile('third_party/six.BUILD', build_file + ['"a noop string"'])
-    exit_code, _, stderr = self.RunBazel([
-        'build', '--noincompatible_disallow_unverified_http_downloads',
-        '--nofetch', '@six_archive//...'
-    ])
+    exit_code, _, stderr = self.RunBazel(
+        ['build', '--nofetch', '@six_archive//...'])
     self.assertEqual(exit_code, 0, os.linesep.join(stderr))
     self.assertIn(fetching_disabled_msg, os.linesep.join(stderr))
 
@@ -123,6 +116,8 @@ def testNewHttpArchiveWithSymlinks(self):
         '    name = "archive_with_symlink",',
         '    urls = ["http://%s:%s/archive_with_symlink.zip"],' % (ip, port),
         '    build_file = "@//:archive_with_symlink.BUILD",',
+        '    sha256 = ',
+        '  "c9c32a48ff65f6319885246b1bfc704e60dd72fb0405dfafdffe403421a4c83a",'
         ')',
     ]
     rule_definition.extend(self.GetDefaultRepoRules())
@@ -137,7 +132,6 @@ def testNewHttpArchiveWithSymlinks(self):
     self.ScratchFile('BUILD')
     exit_code, _, stderr = self.RunBazel([
         'build',
-        '--noincompatible_disallow_unverified_http_downloads',
         '@archive_with_symlink//:file-A',
     ])
     self.assertEqual(exit_code, 0, os.linesep.join(stderr))

src/test/shell/bazel/bazel_repository_cache_test.sh

@@ -258,9 +258,9 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
 http_archive(
     name = 'endangered',
-    url = 'http://localhost:$nc_port/bleh',
+    url = 'file://$repo2_zip',
     type = 'zip',
-)
+    )
 EOF
 
   # Fetch; as we did not specify a hash, we expect bazel to tell us the hash
@@ -271,7 +271,6 @@ EOF
   # to do without checksum. But we can safely do so, as the loopback device
   # is reasonably safe against man-in-the-middle attacks.
   bazel fetch --repository_cache="$repo_cache_dir" \
-        --noincompatible_disallow_unverified_http_downloads \
         //zoo:breeding-program >& $TEST_log \
     || fail "expected fetch to succeed"
 
@@ -280,6 +279,7 @@ EOF
   # Shutdown the server; so fetching again won't work
   shutdown_server
   bazel clean --expunge
+  rm -f $repo2_zip
 
   # As we don't have a predicted cache, we expect fetching to fail now.
   bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \

src/test/shell/bazel/bazel_workspaces_test.sh

@@ -246,7 +246,6 @@ function test_download_integrity_sha256() {
 
   set_workspace_command "repository_ctx.download(\"http://localhost:${fileserver_port}/file.txt\", \"file.txt\", integrity=\"${file_integrity}\")"
 
-  echo 'build --incompatible_disallow_unverified_http_downloads' >> .bazelrc
   build_and_process_log --exclude_rule "//external:local_config_cc"
 
   ensure_contains_exactly 'location: .*repos.bzl:2:3' 1
@@ -276,7 +275,6 @@ function test_download_integrity_sha512() {
 
   set_workspace_command "repository_ctx.download(\"http://localhost:${fileserver_port}/file.txt\", \"file.txt\", integrity=\"${file_integrity}\")"
 
-  echo 'build --incompatible_disallow_unverified_http_downloads' >> .bazelrc
   build_and_process_log --exclude_rule "//external:local_config_cc"
 
   ensure_contains_exactly 'location: .*repos.bzl:2:3' 1
@@ -294,7 +292,6 @@ function test_download_integrity_malformed() {
   mkdir -p "${server_dir}"
   local file="${server_dir}/file.txt"
   startup_server "${server_dir}"
-  echo 'build --incompatible_disallow_unverified_http_downloads' >> .bazelrc
   echo "file contents here" > "${file}"
 
   # Unsupported checksum algorithm

src/test/shell/bazel/skylark_repository_test.sh

@@ -1074,14 +1074,14 @@ function test_skylark_repository_context_downloads_return_struct() {
   cat >test.bzl <<EOF
 def _impl(repository_ctx):
   no_sha_return = repository_ctx.download(
-    url = "http://localhost:${fileserver_port}/download_no_sha256.txt",
+    url = "file://${server_dir}/download_no_sha256.txt",
     output = "download_no_sha256.txt")
   with_sha_return = repository_ctx.download(
     url = "http://localhost:${fileserver_port}/download_with_sha256.txt",
     output = "download_with_sha256.txt",
     sha256 = "${provided_sha256}")
   compressed_no_sha_return = repository_ctx.download_and_extract(
-    url = "http://localhost:${fileserver_port}/compressed_no_sha256.txt.zip",
+    url = "file://${server_dir}/compressed_no_sha256.txt.zip",
     output = "compressed_no_sha256.txt.zip")
   compressed_with_sha_return = repository_ctx.download_and_extract(
       url = "http://localhost:${fileserver_port}/compressed_with_sha256.txt.zip",
@@ -1101,7 +1101,7 @@ EOF
   # none was provided by the call to download_and_extract. So we do have to
   # allow a download without provided checksum, even though it is plain http;
   # nevertheless, localhost is pretty safe against man-in-the-middle attacs.
-  bazel build --noincompatible_disallow_unverified_http_downloads @foo//:all \
+  bazel build @foo//:all \
         >& $TEST_log && shutdown_server || fail "Execution of @foo//:all failed"
 
   output_base="$(bazel info output_base)"
@@ -1882,8 +1882,7 @@ genrule(
   cmd = "cp $< $@",
 )
 EOF
-  bazel build --incompatible_disallow_unverified_http_downloads //:it \
-        > "${TEST_log}" 2>&1 && fail "Expeceted failure" || :
+  bazel build //:it > "${TEST_log}" 2>&1 && fail "Expeceted failure" || :
   expect_log 'plain http.*missing checksum'
 
   # After adding a good checksum, we expect success
@@ -1895,8 +1894,7 @@ sha256 = "$sha256",
 w
 q
 EOF
-  bazel build --incompatible_disallow_unverified_http_downloads //:it \
-        || fail "Expected success one the checksum is given"
+  bazel build //:it || fail "Expected success one the checksum is given"
 
 }