Don't perform crate renaming within rules_rust workspace; minor fixes. (#1143)
Co-authored-by: cfredric <cfredric@chromium.org>
Co-authored-by: Marcel Hlopko <hlopko@google.com>
diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl
index def840e..897cd18 100644
--- a/rust/private/rust.bzl
+++ b/rust/private/rust.bzl
@@ -253,7 +253,7 @@
else:
output_hash = None
- crate_name = compute_crate_name(ctx.label, toolchain, ctx.attr.crate_name)
+ crate_name = compute_crate_name(ctx.workspace_name, ctx.label, toolchain, ctx.attr.crate_name)
rust_lib_name = _determine_lib_name(
crate_name,
crate_type,
@@ -297,7 +297,7 @@
list: A list of providers. See `rustc_compile_action`
"""
toolchain = find_toolchain(ctx)
- crate_name = compute_crate_name(ctx.label, toolchain, ctx.attr.crate_name)
+ crate_name = compute_crate_name(ctx.workspace_name, ctx.label, toolchain, ctx.attr.crate_name)
_assert_correct_dep_mapping(ctx)
output = ctx.actions.declare_file(ctx.label.name + toolchain.binary_ext)
@@ -340,7 +340,7 @@
_assert_no_deprecated_attributes(ctx)
_assert_correct_dep_mapping(ctx)
- crate_name = compute_crate_name(ctx.label, toolchain, ctx.attr.crate_name)
+ crate_name = compute_crate_name(ctx.workspace_name, ctx.label, toolchain, ctx.attr.crate_name)
crate_type = "bin"
deps = transform_deps(ctx.attr.deps)
diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl
index 0433d69..04f09bc 100644
--- a/rust/private/utils.bzl
+++ b/rust/private/utils.bzl
@@ -265,10 +265,11 @@
return dict([(c, ()) for c in name.elems() if not (c.isalnum() or c == "_")]).keys()
-def compute_crate_name(label, toolchain, name_override = None):
+def compute_crate_name(workspace_name, label, toolchain, name_override = None):
"""Returns the crate name to use for the current target.
Args:
+ workspace_name (string): The current workspace name.
label (struct): The label of the current target.
toolchain (struct): The toolchain in use for the target.
name_override (String): An optional name to use (as an override of label.name).
@@ -285,11 +286,9 @@
))
return name_override
- if toolchain and label and toolchain._rename_first_party_crates:
- if should_encode_label_in_crate_name(label.package, toolchain._third_party_dir):
- crate_name = label.name
- else:
- crate_name = encode_label_as_crate_name(label.package, label.name)
+ if (toolchain and label and toolchain._rename_first_party_crates and
+ should_encode_label_in_crate_name(workspace_name, label, toolchain._third_party_dir)):
+ crate_name = encode_label_as_crate_name(label.package, label.name)
else:
crate_name = name_to_crate_name(label.name)
@@ -400,13 +399,16 @@
cc_info = dep[CcInfo] if CcInfo in dep else None,
) for dep in deps]
-def should_encode_label_in_crate_name(package, third_party_dir):
+def should_encode_label_in_crate_name(workspace_name, label, third_party_dir):
"""Determines if the crate's name should include the Bazel label, encoded.
- Names of third-party crates do not encode the full label.
+ Crate names may only encode the label if the target is in the current repo,
+ the target is not in the third_party_dir, and the current repo is not
+ rules_rust.
Args:
- package (string): The package in question.
+ workspace_name (string): The name of the current workspace.
+ label (Label): The package in question.
third_party_dir (string): The directory in which third-party packages are kept.
Returns:
@@ -415,7 +417,11 @@
# TODO(hlopko): This code assumes a monorepo; make it work with external
# repositories as well.
- return ("//" + package + "/").startswith(third_party_dir + "/")
+ return (
+ workspace_name != "rules_rust" and
+ not label.workspace_root and
+ not ("//" + label.package + "/").startswith(third_party_dir + "/")
+ )
# This is a list of pairs, where the first element of the pair is a character
# that is allowed in Bazel package or target names but not in crate names; and
@@ -522,25 +528,21 @@
A string with the appropriate substitutions performed.
"""
- # Find the highest-priority pattern matches.
+ # Find the highest-priority pattern matches for each string index, going
+ # left-to-right and skipping indices that are already involved in a
+ # pattern match.
plan = {}
- for pattern, replacement in substitutions:
- for pattern_start in range(len(string)):
- if not pattern_start in plan and string.startswith(pattern, pattern_start):
- plan[pattern_start] = (len(pattern), replacement)
-
- # Drop replacements that overlap with a replacement earlier in the string.
- replaced_indices_set = {}
- leftmost_plan = {}
- for pattern_start in sorted(plan.keys()):
- length, _ = plan[pattern_start]
- pattern_indices = list(range(pattern_start, pattern_start + length))
- if any([i in replaced_indices_set for i in pattern_indices]):
+ matched_indices_set = {}
+ for pattern_start in range(len(string)):
+ if pattern_start in matched_indices_set:
continue
- replaced_indices_set.update([(i, True) for i in pattern_indices])
- leftmost_plan[pattern_start] = plan[pattern_start]
-
- plan = leftmost_plan
+ for (pattern, replacement) in substitutions:
+ if not string.startswith(pattern, pattern_start):
+ continue
+ length = len(pattern)
+ plan[pattern_start] = (length, replacement)
+ matched_indices_set.update([(pattern_start + i, True) for i in range(length)])
+ break
# Execute the replacement plan, working from right to left.
for pattern_start in sorted(plan.keys(), reverse = True):
diff --git a/rust/settings/BUILD.bazel b/rust/settings/BUILD.bazel
index 3ff0ef6..638f1a7 100644
--- a/rust/settings/BUILD.bazel
+++ b/rust/settings/BUILD.bazel
@@ -14,7 +14,7 @@
)
# A flag specifying the location of vendored third-party rust crates within this
-# repository that must not be renamed when `rename_1p_crates` is enabled.
+# repository that must not be renamed when `rename_first_party_crates` is enabled.
#
# Must be specified as a Bazel package, e.g. "//some/location/in/repo".
string_flag(
diff --git a/test/unit/rename_first_party_crates/BUILD.bazel b/test/unit/rename_first_party_crates/BUILD.bazel
deleted file mode 100644
index 8849fd0..0000000
--- a/test/unit/rename_first_party_crates/BUILD.bazel
+++ /dev/null
@@ -1,4 +0,0 @@
-load(":rename_first_party_crates_test.bzl", "rename_first_party_crates_test_suite")
-
-############################ UNIT TESTS #############################
-rename_first_party_crates_test_suite(name = "rename_first_party_crates_test_suite")
diff --git a/test/unit/rename_first_party_crates/lib.rs b/test/unit/rename_first_party_crates/lib.rs
deleted file mode 100644
index 8b13789..0000000
--- a/test/unit/rename_first_party_crates/lib.rs
+++ /dev/null
@@ -1 +0,0 @@
-
diff --git a/test/unit/rename_first_party_crates/main.rs b/test/unit/rename_first_party_crates/main.rs
deleted file mode 100644
index f328e4d..0000000
--- a/test/unit/rename_first_party_crates/main.rs
+++ /dev/null
@@ -1 +0,0 @@
-fn main() {}
diff --git a/test/unit/rename_first_party_crates/my_third_party_dir/BUILD.bazel b/test/unit/rename_first_party_crates/my_third_party_dir/BUILD.bazel
deleted file mode 100644
index ae2ece1..0000000
--- a/test/unit/rename_first_party_crates/my_third_party_dir/BUILD.bazel
+++ /dev/null
@@ -1,7 +0,0 @@
-load("//rust:defs.bzl", "rust_library")
-
-rust_library(
- name = "third_party_lib",
- srcs = ["lib.rs"],
- visibility = ["//visibility:public"],
-)
diff --git a/test/unit/rename_first_party_crates/my_third_party_dir/lib.rs b/test/unit/rename_first_party_crates/my_third_party_dir/lib.rs
deleted file mode 100644
index 8b13789..0000000
--- a/test/unit/rename_first_party_crates/my_third_party_dir/lib.rs
+++ /dev/null
@@ -1 +0,0 @@
-
diff --git a/test/unit/rename_first_party_crates/rename_first_party_crates_test.bzl b/test/unit/rename_first_party_crates/rename_first_party_crates_test.bzl
deleted file mode 100644
index 3bf5700..0000000
--- a/test/unit/rename_first_party_crates/rename_first_party_crates_test.bzl
+++ /dev/null
@@ -1,206 +0,0 @@
-"""Unit tests for renaming first-party crates."""
-
-load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
-load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_test")
-load("//test/unit:common.bzl", "assert_argv_contains")
-
-def _default_crate_name_library_test_impl(ctx):
- env = analysistest.begin(ctx)
- tut = analysistest.target_under_test(env)
-
- # Note: crate name encodes entire label.
- assert_argv_contains(env, tut.actions[0], "--crate-name=test_slash_unit_slash_rename_first_party_crates_colon_default_dash_crate_dash_name_dash_library")
- return analysistest.end(env)
-
-def _custom_crate_name_library_test_impl(ctx):
- env = analysistest.begin(ctx)
- tut = analysistest.target_under_test(env)
- assert_argv_contains(env, tut.actions[0], "--crate-name=custom_name")
- return analysistest.end(env)
-
-def _default_crate_name_binary_test_impl(ctx):
- env = analysistest.begin(ctx)
- tut = analysistest.target_under_test(env)
-
- # Note: crate name encodes entire label.
- assert_argv_contains(env, tut.actions[0], "--crate-name=test_slash_unit_slash_rename_first_party_crates_colon_default_dash_crate_dash_name_dash_binary")
- return analysistest.end(env)
-
-def _custom_crate_name_binary_test_impl(ctx):
- env = analysistest.begin(ctx)
- tut = analysistest.target_under_test(env)
- assert_argv_contains(env, tut.actions[0], "--crate-name=custom_name")
- return analysistest.end(env)
-
-def _default_crate_name_test_test_impl(ctx):
- env = analysistest.begin(ctx)
- tut = analysistest.target_under_test(env)
-
- # Note: crate name encodes entire label.
- assert_argv_contains(env, tut.actions[0], "--crate-name=test_slash_unit_slash_rename_first_party_crates_colon_default_dash_crate_dash_name_dash_test")
- return analysistest.end(env)
-
-def _custom_crate_name_test_test_impl(ctx):
- env = analysistest.begin(ctx)
- tut = analysistest.target_under_test(env)
- assert_argv_contains(env, tut.actions[0], "--crate-name=custom_name")
- return analysistest.end(env)
-
-def _invalid_custom_crate_name_test_impl(ctx):
- env = analysistest.begin(ctx)
- asserts.expect_failure(env, "contains invalid character(s): -")
- return analysistest.end(env)
-
-def _third_party_lib_test_impl(ctx):
- env = analysistest.begin(ctx)
- tut = analysistest.target_under_test(env)
-
- # Note: not renamed.
- assert_argv_contains(env, tut.actions[0], "--crate-name=third_party_lib")
- return analysistest.end(env)
-
-config_settings = {
- "@//rust/settings:rename_first_party_crates": True,
-}
-third_party_config_settings = dict(
- config_settings,
- **{"@//rust/settings:third_party_dir": "//test/unit/rename_first_party_crates/my_third_party_dir"}
-)
-
-default_crate_name_library_test = analysistest.make(
- _default_crate_name_library_test_impl,
- config_settings = config_settings,
-)
-custom_crate_name_library_test = analysistest.make(
- _custom_crate_name_library_test_impl,
- config_settings = config_settings,
-)
-default_crate_name_binary_test = analysistest.make(
- _default_crate_name_binary_test_impl,
- config_settings = config_settings,
-)
-custom_crate_name_binary_test = analysistest.make(
- _custom_crate_name_binary_test_impl,
- config_settings = config_settings,
-)
-default_crate_name_test_test = analysistest.make(
- _default_crate_name_test_test_impl,
- config_settings = config_settings,
-)
-custom_crate_name_test_test = analysistest.make(
- _custom_crate_name_test_test_impl,
- config_settings = config_settings,
-)
-invalid_custom_crate_name_test = analysistest.make(
- _invalid_custom_crate_name_test_impl,
- config_settings = config_settings,
- expect_failure = True,
-)
-third_party_lib_test = analysistest.make(
- _third_party_lib_test_impl,
- config_settings = third_party_config_settings,
-)
-
-def _rename_first_party_crates_test():
- rust_library(
- name = "default-crate-name-library",
- srcs = ["lib.rs"],
- )
-
- rust_library(
- name = "custom-crate-name-library",
- crate_name = "custom_name",
- srcs = ["lib.rs"],
- )
-
- rust_binary(
- name = "default-crate-name-binary",
- srcs = ["main.rs"],
- )
-
- rust_binary(
- name = "custom-crate-name-binary",
- crate_name = "custom_name",
- srcs = ["main.rs"],
- )
-
- rust_test(
- name = "default-crate-name-test",
- srcs = ["main.rs"],
- )
-
- rust_test(
- name = "custom-crate-name-test",
- crate_name = "custom_name",
- srcs = ["main.rs"],
- )
-
- rust_library(
- name = "invalid-custom-crate-name",
- crate_name = "hyphens-not-allowed",
- srcs = ["lib.rs"],
- tags = ["manual", "norustfmt"],
- )
-
- default_crate_name_library_test(
- name = "default_crate_name_library_test",
- target_under_test = ":default-crate-name-library",
- )
-
- custom_crate_name_library_test(
- name = "custom_crate_name_library_test",
- target_under_test = ":custom-crate-name-library",
- )
-
- default_crate_name_binary_test(
- name = "default_crate_name_binary_test",
- target_under_test = ":default-crate-name-binary",
- )
-
- custom_crate_name_binary_test(
- name = "custom_crate_name_binary_test",
- target_under_test = ":custom-crate-name-binary",
- )
-
- default_crate_name_test_test(
- name = "default_crate_name_test_test",
- target_under_test = ":default-crate-name-test",
- )
-
- custom_crate_name_test_test(
- name = "custom_crate_name_test_test",
- target_under_test = ":custom-crate-name-test",
- )
-
- invalid_custom_crate_name_test(
- name = "invalid_custom_crate_name_test",
- target_under_test = ":invalid-custom-crate-name",
- )
-
- third_party_lib_test(
- name = "third_party_lib_test",
- target_under_test = "//test/unit/rename_first_party_crates/my_third_party_dir:third_party_lib",
- )
-
-def rename_first_party_crates_test_suite(name):
- """Entry-point macro called from the BUILD file.
-
- Args:
- name: Name of the macro.
- """
-
- _rename_first_party_crates_test()
-
- native.test_suite(
- name = name,
- tests = [
- ":default_crate_name_library_test",
- ":custom_crate_name_library_test",
- ":default_crate_name_binary_test",
- ":custom_crate_name_binary_test",
- ":default_crate_name_test_test",
- ":custom_crate_name_test_test",
- ":invalid_custom_crate_name_test",
- ":third_party_lib_test",
- ],
- )
diff --git a/test/unit/utils/utils_test.bzl b/test/unit/utils/utils_test.bzl
index 293887f..dd7bbe5 100644
--- a/test/unit/utils/utils_test.bzl
+++ b/test/unit/utils/utils_test.bzl
@@ -58,19 +58,23 @@
def _is_third_party_crate_test_impl(ctx):
env = unittest.begin(ctx)
- # A target at the root of the 3p dir is considered 3p:
- asserts.true(env, should_encode_label_in_crate_name("third_party", "//third_party"))
+ # A target at the root of the third-party dir is considered third-party:
+ asserts.false(env, should_encode_label_in_crate_name("some_workspace", Label("//third_party:foo"), "//third_party"))
# Targets in subpackages are detected properly:
- asserts.true(env, should_encode_label_in_crate_name("third_party/serde", "//third_party"))
- asserts.true(env, should_encode_label_in_crate_name("third_party/serde/v1", "//third_party"))
+ asserts.false(env, should_encode_label_in_crate_name("some_workspace", Label("//third_party/serde:serde"), "//third_party"))
+ asserts.false(env, should_encode_label_in_crate_name("some_workspace", Label("//third_party/serde/v1:serde"), "//third_party"))
# Ensure the directory name truly matches, and doesn't just include the
- # 3p dir as a substring (or vice versa).
- asserts.false(env, should_encode_label_in_crate_name("third_party_decoy", "//third_party"))
- asserts.false(env, should_encode_label_in_crate_name("decoy_third_party", "//third_party"))
- asserts.false(env, should_encode_label_in_crate_name("third_", "//third_party"))
- asserts.false(env, should_encode_label_in_crate_name("third_party_decoy/serde", "//third_party"))
+ # third-party dir as a substring (or vice versa).
+ asserts.true(env, should_encode_label_in_crate_name("some_workspace", Label("//third_party_decoy:thing"), "//third_party"))
+ asserts.true(env, should_encode_label_in_crate_name("some_workspace", Label("//decoy_third_party:thing"), "//third_party"))
+ asserts.true(env, should_encode_label_in_crate_name("some_workspace", Label("//third_:thing"), "//third_party"))
+ asserts.true(env, should_encode_label_in_crate_name("some_workspace", Label("//third_party_decoy/serde:serde"), "//third_party"))
+
+ # Targets in rules_rust's repo should never be renamed.
+ asserts.false(env, should_encode_label_in_crate_name("rules_rust", Label("//some_package:foo"), "//third_party"))
+
return unittest.end(env)
encode_label_as_crate_name_test = unittest.make(_encode_label_as_crate_name_test_impl)
