bzlmod: Add new features to http_archive rule

Those features are needed by the Bzlmod system to fetch Bazel module dependencies from Bazel registries. - integrity: In addition to "sha256", users can specify the integrity value to verify the downloaded archive content. - remote_patches, remote_patch_strip: apply patche files downloaded from given URLs. This will be used to apply patch files hosted in Bazel registries. Related: #13316 RELNOTES: None PiperOrigin-RevId: 383386903
bazelbuild · Jul 7, 2021 · e9a4e93 · e9a4e93
1 parent 05265a4
commit e9a4e93
Show file tree

Hide file tree

Showing 5 changed files with 294 additions and 10 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java
@@ -25,6 +25,10 @@ public static final class InvalidChecksumException extends Exception {
     private InvalidChecksumException(KeyType keyType, String hash) {
       super("Invalid " + keyType + " checksum '" + hash + "'");
     }
+
+    private InvalidChecksumException(String msg) {
+      super(msg);
+    }
   }
 
   private final KeyType keyType;
@@ -73,14 +77,14 @@ public static Checksum fromSubresourceIntegrity(String integrity)
     }
 
     if (keyType == null) {
-      throw new IllegalArgumentException(
+      throw new InvalidChecksumException(
           "Unsupported checksum algorithm: '"
               + integrity
               + "' (expected SHA-1, SHA-256, SHA-384, or SHA-512)");
     }
 
     if (hash.length != expectedLength) {
-      throw new IllegalArgumentException(
+      throw new InvalidChecksumException(
           "Invalid " + keyType + " SRI checksum '" + integrity + "'");
     }
 

diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
@@ -937,7 +937,76 @@ EOF
   shutdown_server
 }
 
+function test_integrity_correct() {
+  REPO_PATH=$TEST_TMPDIR/repo
+  mkdir -p "$REPO_PATH"
+  cd "$REPO_PATH"
+  create_workspace_with_default_repos WORKSPACE
+  touch BUILD
+  zip -r repo.zip *
+  integrity="sha256-$(cat repo.zip | openssl dgst -sha256 -binary | openssl base64 -A)"
+  startup_server $PWD
+  cd -
+
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+    name = "repo",
+    integrity = "$integrity",
+    url = "http://127.0.0.1:$fileserver_port/repo.zip",
+)
+EOF
+  bazel build @repo//... || fail "Expected integrity check to succeed"
+  shutdown_server
+}
 
+function test_integrity_weird() {
+  REPO_PATH=$TEST_TMPDIR/repo
+  mkdir -p "$REPO_PATH"
+  cd "$REPO_PATH"
+  create_workspace_with_default_repos WORKSPACE
+  touch BUILD
+  zip -r repo.zip *
+  startup_server $PWD
+  cd -
+
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+    name = "repo",
+    integrity = "a random string",
+    url = "http://127.0.0.1:$fileserver_port/repo.zip",
+)
+EOF
+  bazel build @repo//... &> $TEST_log 2>&1 && fail "Expected to fail"
+  expect_log "Unsupported checksum algorithm: 'a random string'"
+  shutdown_server
+}
+
+function test_integrity_incorrect() {
+  REPO_PATH=$TEST_TMPDIR/repo
+  mkdir -p "$REPO_PATH"
+  cd "$REPO_PATH"
+  create_workspace_with_default_repos WORKSPACE
+  touch BUILD
+  zip -r repo.zip *
+  startup_server $PWD
+  cd -
+
+  cat >> $(create_workspace_with_default_repos WORKSPACE) <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+    name = "repo",
+    integrity = "sha256-Yab3Yqr2BlLL8zKHm43MLP2BviEpoGHalX0Dnq538LA=",
+    url = "http://127.0.0.1:$fileserver_port/repo.zip",
+)
+EOF
+  bazel build @repo//... &> $TEST_log 2>&1 && fail "Expected to fail"
+  expect_log "Error downloading \\[http://127.0.0.1:$fileserver_port/repo.zip\\] to"
+  # Bazel translates the integrity value back to the sha256 checksum.
+  expect_log "but wanted 61a6f762aaf60652cbf332879b8dcc2cfd81be2129a061da957d039eae77f0b0"
+  shutdown_server
+}
 
 function test_same_name() {
   mkdir ext

diff --git a/src/test/shell/bazel/external_patching_test.sh b/src/test/shell/bazel/external_patching_test.sh
@@ -196,6 +196,161 @@ EOF
   expect_log 'Helpful message'
 }
 
+test_remote_patch_on_top_of_local_patch() {
+  EXTREPODIR=`pwd`
+  EXTREPOURL="$(get_extrepourl ${EXTREPODIR})"
+  # Generate the remote patch file
+  cat > remote.patch <<'EOF'
+--- a/foo.sh	2018-01-15 10:39:20.183909147 +0100
++++ b/foo.sh	2018-01-15 10:43:35.331566052 +0100
+@@ -1,3 +1,3 @@
+ #!/usr/bin/env sh
+
+-echo Here be dragons...
++echo There are dragons...
+EOF
+  integrity="sha256-$(cat remote.patch | openssl dgst -sha256 -binary | openssl base64 -A)"
+
+  mkdir main
+  cd main
+
+  # Generate the local patch file
+  cat > local.patch <<'EOF'
+--- foo.sh.orig	2021-07-05 15:16:49.000000000 +0200
++++ foo.sh	2021-07-05 15:17:15.000000000 +0200
+@@ -1,3 +1,3 @@
+-#!/usr/bin/env sh
++#!/bin/sh
+
+ echo There are dragons...
+EOF
+
+  cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+  name="ext",
+  strip_prefix="ext-0.1.2",
+  urls=["${EXTREPOURL}/ext.zip"],
+  build_file_content="exports_files([\"foo.sh\"])",
+  remote_patches = {"${EXTREPOURL}/remote.patch": "${integrity}"},
+  remote_patch_strip = 1,
+  patches = ["//:local.patch"],
+)
+EOF
+  cat > BUILD <<'EOF'
+genrule(
+  name = "foo",
+  outs = ["foo.sh"],
+  srcs = ["@ext//:foo.sh"],
+  cmd = "cp $< $@; chmod u+x $@",
+  executable = True,
+)
+EOF
+
+  bazel build :foo.sh
+  foopath=`bazel info bazel-bin`/foo.sh
+  grep -q 'There are' $foopath || fail "expected remote patch to be applied"
+  grep -q '/bin/sh' $foopath || fail "expected local patch to be applied"
+}
+
+test_remote_patch_integrity_incorrect() {
+  EXTREPODIR=`pwd`
+  EXTREPOURL="$(get_extrepourl ${EXTREPODIR})"
+  # Generate the remote patch file
+  cat > remote.patch <<'EOF'
+--- a/foo.sh	2018-01-15 10:39:20.183909147 +0100
++++ b/foo.sh	2018-01-15 10:43:35.331566052 +0100
+@@ -1,3 +1,3 @@
+ #!/usr/bin/env sh
+
+-echo Here be dragons...
++echo There are dragons...
+EOF
+
+  mkdir main
+  cd main
+
+  cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+  name="ext",
+  strip_prefix="ext-0.1.2",
+  urls=["${EXTREPOURL}/ext.zip"],
+  build_file_content="exports_files([\"foo.sh\"])",
+  remote_patches = {"${EXTREPOURL}/remote.patch": "sha256-Yab3Yqr2BlLL8zKHm43MLP2BviEpoGHalX0Dnq538LA="},
+  remote_patch_strip = 1,
+  patches = ["//:local.patch"],
+)
+EOF
+
+  bazel build @ext//... &> $TEST_log 2>&1 && fail "Expected to fail"
+  expect_log "Error downloading \\[.*/remote.patch\\] to"
+  expect_log "but wanted 61a6f762aaf60652cbf332879b8dcc2cfd81be2129a061da957d039eae77f0b0"
+}
+
+test_remote_patches_with_same_base_name() {
+  EXTREPODIR=`pwd`
+  EXTREPOURL="$(get_extrepourl ${EXTREPODIR})"
+
+  mkdir a
+  # Generate a remote patch file
+  cat > a/remote.patch <<'EOF'
+--- a/foo.sh	2018-01-15 10:39:20.183909147 +0100
++++ b/foo.sh	2018-01-15 10:43:35.331566052 +0100
+@@ -1,3 +1,3 @@
+ #!/usr/bin/env sh
+
+-echo Here be dragons...
++echo There are dragons...
+EOF
+  integrity_a="sha256-$(cat a/remote.patch | openssl dgst -sha256 -binary | openssl base64 -A)"
+
+  mkdir b
+  # Generate another remote patch file with the same base name
+  cat > b/remote.patch <<'EOF'
+--- a/foo.sh	2021-07-05 15:16:49.000000000 +0200
++++ b/foo.sh	2021-07-05 15:17:15.000000000 +0200
+@@ -1,3 +1,3 @@
+-#!/usr/bin/env sh
++#!/bin/sh
+
+ echo There are dragons...
+EOF
+  integrity_b="sha256-$(cat b/remote.patch | openssl dgst -sha256 -binary | openssl base64 -A)"
+
+  mkdir main
+  cd main
+
+  cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+  name="ext",
+  strip_prefix="ext-0.1.2",
+  urls=["${EXTREPOURL}/ext.zip"],
+  build_file_content="exports_files([\"foo.sh\"])",
+  remote_patches = {
+    "${EXTREPOURL}/a/remote.patch": "$integrity_a",
+    "${EXTREPOURL}/b/remote.patch": "$integrity_b",
+  },
+  remote_patch_strip = 1,
+)
+EOF
+  cat > BUILD <<'EOF'
+genrule(
+  name = "foo",
+  outs = ["foo.sh"],
+  srcs = ["@ext//:foo.sh"],
+  cmd = "cp $< $@; chmod u+x $@",
+  executable = True,
+)
+EOF
+
+  bazel build :foo.sh
+  foopath=`bazel info bazel-bin`/foo.sh
+  grep -q 'There are' $foopath || fail "expected a/remote.patch to be applied"
+  grep -q '/bin/sh' $foopath || fail "expected b/remote.patch to be applied"
+}
+
 test_patch_git() {
   EXTREPODIR=`pwd`
   if $is_windows; then

diff --git a/tools/build_defs/repo/http.bzl b/tools/build_defs/repo/http.bzl
@@ -116,11 +116,14 @@ def _http_archive_impl(ctx):
         ctx.attr.strip_prefix,
         canonical_id = ctx.attr.canonical_id,
         auth = auth,
+        integrity = ctx.attr.integrity,
     )
     workspace_and_buildfile(ctx)
-    patch(ctx)
+    patch(ctx, auth = auth)
 
-    return update_attrs(ctx.attr, _http_archive_attrs.keys(), {"sha256": download_info.sha256})
+    # We don't need to override the sha256 attribute if integrity is already specified.
+    sha256_override = {} if ctx.attr.integrity else {"sha256": download_info.sha256}
+    return update_attrs(ctx.attr, _http_archive_attrs.keys(), sha256_override)
 
 _HTTP_FILE_BUILD = """
 package(default_visibility = ["//visibility:public"])
@@ -227,7 +230,15 @@ If all downloads fail, the rule will fail.""",
 This must match the SHA-256 of the file downloaded. _It is a security risk
 to omit the SHA-256 as remote files can change._ At best omitting this
 field will make your build non-hermetic. It is optional to make development
-easier but should be set before shipping.""",
+easier but either this attribute or `integrity` should be set before shipping.""",
+    ),
+    "integrity": attr.string(
+        doc = """Expected checksum in Subresource Integrity format of the file downloaded.
+
+This must match the checksum of the file downloaded. _It is a security risk
+to omit the checksum as remote files can change._ At best omitting this
+field will make your build non-hermetic. It is optional to make development
+easier but either this attribute or `sha256` should be set before shipping.""",
     ),
     "netrc": attr.string(
         doc = "Location of the .netrc file to use for authentication",
@@ -279,6 +290,19 @@ following: `"zip"`, `"jar"`, `"war"`, `"tar"`, `"tar.gz"`, `"tgz"`,
             "patch command line tool if `patch_tool` attribute is specified or there are " +
             "arguments other than `-p` in `patch_args` attribute.",
     ),
+    "remote_patches": attr.string_dict(
+        default = {},
+        doc =
+            "A map of patch file URL to its integrity value, they are applied after extracting " +
+            "the archive and before applying patch files from the `patches` attribute. " +
+            "It uses the Bazel-native patch implementation, you can specify the patch strip " +
+            "number with `remote_patch_strip`",
+    ),
+    "remote_patch_strip": attr.int(
+        default = 0,
+        doc =
+            "The number of leading slashes to be stripped from the file name in the remote patches.",
+    ),
     "patch_tool": attr.string(
         default = "",
         doc = "The patch(1) utility to use. If this is specified, Bazel will use the specifed " +
@@ -293,7 +317,7 @@ following: `"zip"`, `"jar"`, `"war"`, `"tar"`, `"tar.gz"`, `"tgz"`,
             "If arguments other than -p are specified, Bazel will fall back to use patch " +
             "command line tool instead of the Bazel-native patch implementation. When falling " +
             "back to patch command line tool and patch_tool attribute is not specified, " +
-            "`patch` will be used.",
+            "`patch` will be used. This only affects patch files in the `patches` attribute.",
     ),
     "patch_cmds": attr.string_list(
         default = [],