From eda0eea23208b60f930502e36ed84c6188b89716 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Fri, 10 May 2024 13:51:47 -0700 Subject: [PATCH] Add support for remote_files for http_archive This issue adds support necessary to tackle https://github.com/bazelbuild/bazel-central-registry/issues/1566 Add two new attributes to http_archive: remote_file_urls and remote_file_integrity. The purpose of these two attributes is to allow files to effectively be overlaid ontop of an http_archive. The goal of such functionality would be useful for BCR since the BUILD & WORKSPACE files need no longer be stored as patch files. This means we could probably deprecate `build_file` since that could be referenced as a file:// url in the remote_file_urls attribute. Co-authored-by: Mark Williams CC @fmeum Closes #22155. PiperOrigin-RevId: 632594203 Change-Id: I6310093482c5c58537ed6dbe4ff90bafdbd696ff --- MODULE.bazel.lock | 8 +- src/test/shell/bazel/BUILD | 11 + .../shell/bazel/external_remote_file_test.sh | 323 ++++++++++++++++++ src/test/tools/bzlmod/MODULE.bazel.lock | 16 +- tools/build_defs/repo/http.bzl | 20 ++ tools/build_defs/repo/utils.bzl | 28 ++ 6 files changed, 394 insertions(+), 12 deletions(-) create mode 100755 src/test/shell/bazel/external_remote_file_test.sh diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 2b8ffa5cbd66bb..095a3f29b77bb3 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -2917,7 +2917,7 @@ "bzlTransitiveDigest": "tunTSmgwd2uvTzkCLtdbuCp0AI+WR+ftiPNqZ0rmcZk=", "recordedFileInputs": { "@@//MODULE.bazel": "939a48e4dbb71472db80f16a6a639f65cb4ddb07d8e1fd01db14cf1d0323917d", - "@@//src/test/tools/bzlmod/MODULE.bazel.lock": "1fffc063b7830519a4f8ea2b8a1192fef8c4065e4f2e5a1aeedd4f563a5bc683" + "@@//src/test/tools/bzlmod/MODULE.bazel.lock": "43259b270347fc1a76734df4b73b2d8c4e2c14293f3c0fb7dca5c48fd4b22758" }, "recordedDirentsInputs": {}, "envVariables": {}, @@ -3469,7 +3469,7 @@ }, "//tools/android:android_extensions.bzl%android_sdk_proxy_extensions": { "general": { - "bzlTransitiveDigest": "JQoCqdz3xQGJ+/ILJLx/pnLPCjAYRvq3Tmj3wkpmflI=", + "bzlTransitiveDigest": "XLVk7GLje5iUTApmiCu5fapV2+/7HqWPF7cQEu7SPOE=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, @@ -3485,7 +3485,7 @@ }, "//tools/android:android_extensions.bzl%remote_android_tools_extensions": { "general": { - "bzlTransitiveDigest": "JQoCqdz3xQGJ+/ILJLx/pnLPCjAYRvq3Tmj3wkpmflI=", + "bzlTransitiveDigest": "XLVk7GLje5iUTApmiCu5fapV2+/7HqWPF7cQEu7SPOE=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, @@ -3512,7 +3512,7 @@ }, "//tools/test:extensions.bzl%remote_coverage_tools_extension": { "general": { - "bzlTransitiveDigest": "kcwMubtgxBru6ttbkwo5IjUmLeSz0AODaQoox5L4zVU=", + "bzlTransitiveDigest": "ia4DMKKuTJ4QAHKSrRs4v/QBfK48p52gn2SwHL6BXd8=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, "envVariables": {}, diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index eb05d50cca6b47..a7103af489996d 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -803,6 +803,17 @@ sh_test( shard_count = 3, ) +sh_test( + name = "external_remote_file_test", + size = "small", + srcs = ["external_remote_file_test.sh"], + data = [ + ":test-deps", + "@bazel_tools//tools/bash/runfiles", + ], + shard_count = 3, +) + sh_test( name = "external_path_test", size = "medium", diff --git a/src/test/shell/bazel/external_remote_file_test.sh b/src/test/shell/bazel/external_remote_file_test.sh new file mode 100755 index 00000000000000..7fc3a291035ca6 --- /dev/null +++ b/src/test/shell/bazel/external_remote_file_test.sh @@ -0,0 +1,323 @@ +#!/bin/bash +# +# Copyright 2017 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Tests the patching functionality of external repositories. + +set -euo pipefail +# --- begin runfiles.bash initialization --- +if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + if [[ -f "$0.runfiles_manifest" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" + elif [[ -f "$0.runfiles/MANIFEST" ]]; then + export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + fi +fi +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" +elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ + "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" +else + echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash" + exit 1 +fi +# --- end runfiles.bash initialization --- + +source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +# `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux". +# `tr` converts all upper case letters to lower case. +# `case` matches the result if the `uname | tr` expression to string prefixes +# that use the same wildcards as names do in Bash, i.e. "msys*" matches strings +# starting with "msys", and "*" matches everything (it's the default case). +case "$(uname -s | tr [:upper:] [:lower:])" in +msys*) + # As of 2019-01-15, Bazel on Windows only supports MSYS Bash. + declare -r is_windows=true + ;; +*) + declare -r is_windows=false + ;; +esac + +if "$is_windows"; then + # Disable MSYS path conversion that converts path-looking command arguments to + # Windows paths (even if they arguments are not in fact paths). + export MSYS_NO_PATHCONV=1 + export MSYS2_ARG_CONV_EXCL="*" +fi + + +if $is_windows; then + export MSYS_NO_PATHCONV=1 + export MSYS2_ARG_CONV_EXCL="*" +fi + +function set_up() { + WRKDIR=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") + cd "${WRKDIR}" + write_default_lockfile "MODULE.bazel.lock" + # create an archive file with files interesting for patching + mkdir hello_world-0.1.2 + cat > hello_world-0.1.2/hello_world.c <<'EOF' +#include +int main() { + printf("Hello, world!\n"); + return 0; +} +EOF + zip hello_world.zip hello_world-0.1.2/* + rm -rf hello_world-0.1.2 +} + +function get_extrepourl() { + if $is_windows; then + echo "file:///$(cygpath -m $1)" + else + echo "file://$1" + fi +} + + +test_overlay_remote_file_multiple_segments() { + EXTREPODIR=`pwd` + EXTREPOURL="$(get_extrepourl ${EXTREPODIR})" + + # Generate the remote files to overlay + mkdir -p child + cat > child/foo_bar.c <<'EOF' +#include +int main() { + printf("Foo, Bar!\n"); + return 0; +} +EOF + cat > child/BUILD.bazel <<'EOF' +load("@rules_cc//cc:defs.bzl", "cc_binary") + +cc_binary( + name = "foo_bar", + srcs = ["foo_bar.c"], +) +EOF + touch BUILD.bazel + touch WORKSPACE + + mkdir main + cd main + cat > WORKSPACE < BUILD.bazel <<'EOF' +load("@rules_cc//cc:defs.bzl", "cc_binary") + +cc_binary( + name = "hello_world", + srcs = ["hello_world.c"], +) +EOF + touch WORKSPACE + + mkdir main + cd main + cat > WORKSPACE < BUILD.bazel <<'EOF' +load("@rules_cc//cc:defs.bzl", "cc_binary") + +cc_binary( + name = "hello_world", + srcs = ["hello_world.c"], +) +EOF + touch WORKSPACE + + mkdir main + cd main + cat > WORKSPACE < $TEST_log 2>&1 && fail "Expected to fail" + expect_log "but wanted sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFY=" +} + +test_overlay_remote_file_without_integrity() { + EXTREPODIR=`pwd` + EXTREPOURL="$(get_extrepourl ${EXTREPODIR})" + + # Generate the remote files to overlay + cat > BUILD.bazel <<'EOF' +load("@rules_cc//cc:defs.bzl", "cc_binary") + +cc_binary( + name = "hello_world", + srcs = ["hello_world.c"], +) +EOF + touch WORKSPACE + + mkdir main + cd main + cat > WORKSPACE < BUILD.bazel <<'EOF' +load("@rules_cc//cc:defs.bzl", "cc_binary") + +cc_binary( + name = "hello_world", + srcs = ["hello_world.c"], +) +EOF + touch WORKSPACE + + mkdir main + cd main + cat > WORKSPACE < $TEST_log 2>&1 && fail "Expected to fail" + expect_log "Error in download: Cannot write outside of the repository directory" +} + +test_overlay_remote_file_disallow_absolute_path() { + EXTREPODIR=`pwd` + EXTREPOURL="$(get_extrepourl ${EXTREPODIR})" + + cat > BUILD.bazel <<'EOF' +load("@rules_cc//cc:defs.bzl", "cc_binary") + +cc_binary( + name = "hello_world", + srcs = ["hello_world.c"], +) +EOF + touch WORKSPACE + + mkdir main + cd main + cat > WORKSPACE < $TEST_log 2>&1 && fail "Expected to fail" + expect_log "Error in download: Cannot write outside of the repository directory" +} + +run_suite "external remote file tests" diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index 625916c7f78f97..4672dfda1ea873 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -1216,7 +1216,7 @@ }, "@@bazel_tools//tools/android:android_extensions.bzl%android_sdk_proxy_extensions": { "general": { - "bzlTransitiveDigest": "JQoCqdz3xQGJ+/ILJLx/pnLPCjAYRvq3Tmj3wkpmflI=", + "bzlTransitiveDigest": "XLVk7GLje5iUTApmiCu5fapV2+/7HqWPF7cQEu7SPOE=", "usagesDigest": "MxHwjf0M2Z/ciNsN0U0t4T1LF5A1ZXwM1kigPKI+3SA=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, @@ -1233,7 +1233,7 @@ }, "@@bazel_tools//tools/android:android_extensions.bzl%remote_android_tools_extensions": { "general": { - "bzlTransitiveDigest": "JQoCqdz3xQGJ+/ILJLx/pnLPCjAYRvq3Tmj3wkpmflI=", + "bzlTransitiveDigest": "XLVk7GLje5iUTApmiCu5fapV2+/7HqWPF7cQEu7SPOE=", "usagesDigest": "pfoI3DAAesYHdp38xIvu9Ry0AM3EIPGwvav/Cyv23Kw=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, @@ -1326,7 +1326,7 @@ }, "@@bazel_tools//tools/test:extensions.bzl%remote_coverage_tools_extension": { "general": { - "bzlTransitiveDigest": "kcwMubtgxBru6ttbkwo5IjUmLeSz0AODaQoox5L4zVU=", + "bzlTransitiveDigest": "ia4DMKKuTJ4QAHKSrRs4v/QBfK48p52gn2SwHL6BXd8=", "usagesDigest": "5AzD10fWCl4vQ0i0Zl96Ial7tU8raC84LTK6DeHefuk=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, @@ -1391,7 +1391,7 @@ }, "@@rules_java~//java:extensions.bzl%toolchains": { "general": { - "bzlTransitiveDigest": "vVD15tu/rctjxFcmEY/kGV8BU29Wa5EgvggNQpJVYrE=", + "bzlTransitiveDigest": "3/zOmP1mlgajX04YvWpwPL1sUWIIbCnLT3W3i7GTSAI=", "usagesDigest": "km/MmwGP92TBuE8sy/Jk2BmLrfOADnezet92ElEDuSA=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, @@ -1957,7 +1957,7 @@ }, "@@rules_jvm_external~//:extensions.bzl%maven": { "general": { - "bzlTransitiveDigest": "B3lrNG2za+3c/y7Bldx38hxzAkTa6uYaesdEicYzZes=", + "bzlTransitiveDigest": "226YAg74jgPpN2590v8XjmUVZU1nLLfY26Q0RHpkZYo=", "usagesDigest": "UPebZtX4g40+QepdK3oMHged0o0tq6ojKbW84wE6XRA=", "recordedFileInputs": { "@@rules_jvm_external~//rules_jvm_external_deps_install.json": "10442a5ae27d9ff4c2003e5ab71643bf0d8b48dcf968b4173fa274c3232a8c06" @@ -2981,7 +2981,7 @@ }, "@@rules_jvm_external~//:non-module-deps.bzl%non_module_deps": { "general": { - "bzlTransitiveDigest": "4OsuF/ozYz/n2U+6GGxM/3IzzX+qvQtjlY1fXeg2jE0=", + "bzlTransitiveDigest": "5AogXHZkwASsdwr6DZ7FypQvxHHp3r0l7HGPUx4wbgY=", "usagesDigest": "bTG4ItERqhG1LeSs62hQ01DiMarFsflWgpZaghM5qik=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, @@ -3009,7 +3009,7 @@ }, "@@rules_python~//python/extensions:python.bzl%python": { "general": { - "bzlTransitiveDigest": "f/rffFOqp0Y5r6QNmElWpkVrIk/9b0pfhd/j+JtG0YM=", + "bzlTransitiveDigest": "vmavs6wkry+PckMdBQFrxN22WzruPHKNVE+U7VyukC0=", "usagesDigest": "7vjNHuEgQORYN9+9/77Q4zw1kawobM2oCQb9p0uhL68=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, @@ -3039,7 +3039,7 @@ }, "@@rules_python~//python/extensions/private:internal_deps.bzl%internal_deps": { "general": { - "bzlTransitiveDigest": "2QkuIItPQyACdxi2VVWUF2DrUHZ4M8To5Q5TMr6R+CU=", + "bzlTransitiveDigest": "6AMTpVCtd1n3YvJkISRnSZyaiRuIDQejEOplE09R2sM=", "usagesDigest": "b+nMDqtqPCBxiMBewNNde3aNjzKqZyvJuN5/49xB62s=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, diff --git a/tools/build_defs/repo/http.bzl b/tools/build_defs/repo/http.bzl index 9735cc8f02e0b9..642d19daa132fc 100644 --- a/tools/build_defs/repo/http.bzl +++ b/tools/build_defs/repo/http.bzl @@ -51,6 +51,7 @@ load( ) load( ":utils.bzl", + "download_remote_files", "get_auth", "patch", "update_attrs", @@ -139,6 +140,8 @@ def _http_archive_impl(ctx): auth = get_auth(ctx, all_urls) download_info = ctx.download_and_extract( + # TODO(fzakaria): all_urls here has the remote_patch URL which is incorrect + # I believe this to be a file all_urls, ctx.attr.add_prefix, ctx.attr.sha256, @@ -149,6 +152,8 @@ def _http_archive_impl(ctx): integrity = ctx.attr.integrity, ) workspace_and_buildfile(ctx) + + download_remote_files(ctx, auth = auth) patch(ctx, auth = auth) return _update_integrity_attr(ctx, _http_archive_attrs, download_info) @@ -304,6 +309,21 @@ following: `"zip"`, `"jar"`, `"war"`, `"aar"`, `"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_file_urls": attr.string_list_dict( + default = {}, + doc = + "A map of relative paths (key) to a list of URLs (value) that are to be downloaded " + + "and made available as overlaid files on the repo. This is useful when you want " + + "to add WORKSPACE or BUILD.bazel files atop an existing repository. The files " + + "are downloaded before applying the patches in the `patches` attribute and the list of URLs " + + "should all be possible mirrors of the same file. The URLs are tried in order until one succeeds. ", + ), + "remote_file_integrity": attr.string_dict( + default = {}, + doc = + "A map of file relative paths (key) to its integrity value (value). These relative paths should map " + + "to the files (key) in the `remote_file_urls` attribute.", + ), "remote_patches": attr.string_dict( default = {}, doc = diff --git a/tools/build_defs/repo/utils.bzl b/tools/build_defs/repo/utils.bzl index 89bcf0f0a00e51..9620853446454a 100644 --- a/tools/build_defs/repo/utils.bzl +++ b/tools/build_defs/repo/utils.bzl @@ -91,6 +91,34 @@ def _download_patch(ctx, patch_url, integrity, auth): ) return patch_path +def download_remote_files(ctx, auth = None): + """Utility function for downloading remote files. + + This rule is intended to be used in the implementation function of + a repository rule. It assumes the parameters `remote_file_urls` and + `remote_file_integrity` to be present in `ctx.attr`. + + Args: + ctx: The repository context of the repository rule calling this utility + function. + auth: An optional dict specifying authentication information for some of the URLs. + """ + pending = [ + ctx.download( + remote_file_urls, + path, + canonical_id = ctx.attr.canonical_id, + auth = auth, + integrity = ctx.attr.remote_file_integrity.get(path, ""), + block = False, + ) + for path, remote_file_urls in ctx.attr.remote_file_urls.items() + ] + + # Wait until the requests are done + for p in pending: + p.wait() + def patch(ctx, patches = None, patch_cmds = None, patch_cmds_win = None, patch_tool = None, patch_args = None, auth = None): """Implementation of patching an already extracted repository.