Skip to content

Commit

Permalink
Fixed regression in load_arbitrary_tool causing it to require attri…
Browse files Browse the repository at this point in the history
…butes repository rules that call it. (#554)

In #545 I fixed an issue where `load_arbitrary_tool` could not be used in repository rules that did not specify particular attributes. This issue was then reintroduced in #551. This PR fixes the issue again and adds a test to prevent this from happening again in the future.
  • Loading branch information
UebelAndre committed Jan 21, 2021
1 parent 8c388e1 commit 8826d30
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 4 deletions.
4 changes: 4 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")

bazel_skylib_workspace()

load("//test:deps.bzl", "io_bazel_rules_rust_test_deps")

io_bazel_rules_rust_test_deps()

# --- end stardoc

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
Expand Down
9 changes: 5 additions & 4 deletions rust/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ load(
)

DEFAULT_TOOLCHAIN_NAME_PREFIX = "toolchain_for"
DEFAULT_STATIC_RUST_URL_TEMPLATES = ["https://static.rust-lang.org/dist/{}.tar.gz"]

# buildifier: disable=unnamed-macro
def rust_repositories(
Expand All @@ -22,7 +23,7 @@ def rust_repositories(
edition = None,
dev_components = False,
sha256s = None,
urls = ["https://static.rust-lang.org/dist/{}.tar.gz"]):
urls = DEFAULT_STATIC_RUST_URL_TEMPLATES):
"""Emits a default set of toolchains for Linux, MacOS, and Freebsd
Skip this macro and call the `rust_repository_set` macros directly if you need a compiler for \
Expand Down Expand Up @@ -448,7 +449,7 @@ def load_arbitrary_tool(ctx, tool_name, tool_subdirectories, version, iso_date,
static_rust = ctx.os.environ.get("STATIC_RUST_URL", "https://static.rust-lang.org")
urls = ["{}/dist/{}.tar.gz".format(static_rust, tool_suburl)]

for url in ctx.attr.urls:
for url in getattr(ctx.attr, "urls", DEFAULT_STATIC_RUST_URL_TEMPLATES):
new_url = url.format(tool_suburl)
if new_url not in urls:
urls.append(new_url)
Expand Down Expand Up @@ -671,7 +672,7 @@ rust_toolchain_repository = repository_rule(
),
"urls": attr.string_list(
doc = "A list of mirror urls containing the tools from the Rust-lang static file server. These must contain the '{}' used to substitute the tool being fetched (using .format).",
default = ["https://static.rust-lang.org/dist/{}.tar.gz"],
default = DEFAULT_STATIC_RUST_URL_TEMPLATES,
),
},
implementation = _rust_toolchain_repository_impl,
Expand Down Expand Up @@ -711,7 +712,7 @@ def rust_repository_set(
edition = None,
dev_components = False,
sha256s = None,
urls = ["https://static.rust-lang.org/dist/{}.tar.gz"]):
urls = DEFAULT_STATIC_RUST_URL_TEMPLATES):
"""Assembles a remote repository for the given toolchain params, produces a proxy repository \
to contain the toolchain declaration, and registers the toolchains.
Expand Down
8 changes: 8 additions & 0 deletions test/deps.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
"""A module defining dependencies of the `io_bazel_rules_rust` tests"""

load("//test/load_arbitrary_tool:load_arbitrary_tool_test.bzl", "load_arbitrary_tool_test")

def io_bazel_rules_rust_test_deps():
"""Load dependencies for io_bazel_rules_rust tests"""

load_arbitrary_tool_test()
15 changes: 15 additions & 0 deletions test/load_arbitrary_tool/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
load("@bazel_skylib//rules:build_test.bzl", "build_test")

filegroup(
name = "load_arbitrary_tool_test_src",
srcs = [
"@io_bazel_rules_rust_load_arbitrary_tool_test//:bin/cargo",
],
)

build_test(
name = "load_arbitrary_tool_test",
targets = [
":load_arbitrary_tool_test_src",
],
)
40 changes: 40 additions & 0 deletions test/load_arbitrary_tool/load_arbitrary_tool_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# buildifier: disable=module-docstring
load("//rust:repositories.bzl", "load_arbitrary_tool")

def _load_arbitrary_tool_test_impl(repository_ctx):
if "mac" in repository_ctx.os.name:
target_triple = "x86_64-apple-darwin"
elif "windows" in repository_ctx.os.name:
target_triple = "x86_64-pc-windows-msvc"
else:
target_triple = "x86_64-unknown-linux-gnu"

# Download cargo
load_arbitrary_tool(
ctx = repository_ctx,
tool_name = "cargo",
tool_subdirectories = ["cargo"],
version = "1.49.0",
iso_date = None,
target_triple = target_triple,
)

repo_path = repository_ctx.path(".")
repository_ctx.file(
"{}/BUILD.bazel".format(repo_path),
content = "exports_files([\"bin/cargo\"])",
)

_load_arbitrary_tool_test = repository_rule(
implementation = _load_arbitrary_tool_test_impl,
doc = (
"A test repository rule ensuring `load_arbitrary_tool` functions " +
"without requiring any attributes on a repository rule"
),
)

def load_arbitrary_tool_test():
"""Define the a test repository for ensuring `load_arbitrary_tool` has no attribute requirements"""
_load_arbitrary_tool_test(
name = "io_bazel_rules_rust_load_arbitrary_tool_test",
)

0 comments on commit 8826d30

Please sign in to comment.