From 5a79d72fbdd44980dfa424607dd646d8303d6fc1 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Tue, 14 Dec 2021 14:34:53 +0100 Subject: [PATCH 1/4] use absolute value of hash function to determine crate name (#1064) --- rust/private/utils.bzl | 7 +++++- test/BUILD.bazel | 2 +- test/unit/crate_name/crate_name_test.bzl | 28 ++++++++++++++++++++++++ test/unit/crate_name/slib.rs | 1 + 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 test/unit/crate_name/slib.rs diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index 218537bd72..bc00e690f8 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -132,7 +132,12 @@ def determine_output_hash(crate_root): Returns: str: A string representation of the hash. """ - return repr(hash(crate_root.path)) + + # Take the absolute value of hash() since it could be negative. + h = hash(crate_root.path) + if h < 0: + h = -h + return repr(h) def get_preferred_artifact(library_to_link): """Get the first available library to link from a LibraryToLink object. diff --git a/test/BUILD.bazel b/test/BUILD.bazel index 45e499a86a..499b269ef1 100644 --- a/test/BUILD.bazel +++ b/test/BUILD.bazel @@ -5,7 +5,7 @@ load( rule_test( name = "hello_lib_rule_test", - generates = ["libhello_lib--145651613.rlib"], + generates = ["libhello_lib-145651613.rlib"], rule = "//test/rust:hello_lib", ) diff --git a/test/unit/crate_name/crate_name_test.bzl b/test/unit/crate_name/crate_name_test.bzl index 2a5d8ce424..217bdfd92b 100644 --- a/test/unit/crate_name/crate_name_test.bzl +++ b/test/unit/crate_name/crate_name_test.bzl @@ -56,6 +56,21 @@ def _invalid_custom_crate_name_test_impl(ctx): asserts.expect_failure(env, "contains invalid character(s): -") return analysistest.end(env) +def _slib_library_name_test_impl(ctx): + """Regression test for extra-filename. + + Checks that the extra hash value appended to the library filename only + contains one dash. Previously, the hash for `slib` was negative, + resulting in an extra dash in the filename (--codegen_extra_filename=--517943325). + + Args: + ctx: rule context. + """ + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + assert_argv_contains(env, tut.actions[0], "--codegen=extra-filename=-517943325") + return analysistest.end(env) + default_crate_name_library_test = analysistest.make( _default_crate_name_library_test_impl, ) @@ -82,6 +97,9 @@ invalid_custom_crate_name_test = analysistest.make( _invalid_custom_crate_name_test_impl, expect_failure = True, ) +slib_library_name_test = analysistest.make( + _slib_library_name_test_impl, +) def _crate_name_test(): rust_library( @@ -130,6 +148,16 @@ def _crate_name_test(): tags = ["manual", "norustfmt"], ) + rust_library( + name = "slib", + srcs = ["slib.rs"], + ) + + slib_library_name_test( + name = "slib_library_name_test", + target_under_test = ":slib", + ) + default_crate_name_library_test( name = "default_crate_name_library_test", target_under_test = ":default-crate-name-library", diff --git a/test/unit/crate_name/slib.rs b/test/unit/crate_name/slib.rs new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/test/unit/crate_name/slib.rs @@ -0,0 +1 @@ + From 09c364fed3a83b0c488d251af8f91ba008c92f21 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 16 Dec 2021 10:53:06 +0000 Subject: [PATCH 2/4] cargo_build_script: Populate LD and LDFLAGS (#1067) These are sometimes invoked by -sys crates. --- cargo/cargo_build_script.bzl | 4 +++- cargo/cargo_build_script_runner/bin.rs | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/cargo/cargo_build_script.bzl b/cargo/cargo_build_script.bzl index 8e6f62f844..870916f24b 100644 --- a/cargo/cargo_build_script.bzl +++ b/cargo/cargo_build_script.bzl @@ -92,8 +92,10 @@ def _build_script_impl(ctx): # Pull in env vars which may be required for the cc_toolchain to work (e.g. on OSX, the SDK version). # We hope that the linker env is sufficient for the whole cc_toolchain. cc_toolchain, feature_configuration = find_cc_toolchain(ctx) - _, _, linker_env = get_linker_and_args(ctx, ctx.attr, cc_toolchain, feature_configuration, None) + linker, link_args, linker_env = get_linker_and_args(ctx, ctx.attr, cc_toolchain, feature_configuration, None) env.update(**linker_env) + env["LD"] = linker + env["LDFLAGS"] = " ".join(link_args) # MSVC requires INCLUDE to be set cc_env = get_cc_compile_env(cc_toolchain, feature_configuration) diff --git a/cargo/cargo_build_script_runner/bin.rs b/cargo/cargo_build_script_runner/bin.rs index 2d92293013..e99c102899 100644 --- a/cargo/cargo_build_script_runner/bin.rs +++ b/cargo/cargo_build_script_runner/bin.rs @@ -110,6 +110,10 @@ fn run_buildrs() -> Result<(), String> { } } + if let Some(ld_path) = env::var_os("LD") { + command.env("LD", exec_root.join(ld_path)); + } + // replace env vars with a ${pwd} prefix with the exec_root for (key, value) in env::vars() { let exec_root_str = exec_root.to_str().expect("exec_root not in utf8"); From 4c01e8f8dac468baa54d3522b5d2b83e6eafe8c2 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Thu, 16 Dec 2021 11:11:31 +0000 Subject: [PATCH 3/4] Init submodules for crate_universe git repos (#1068) Many sys crates pull in source as a submodule --- crate_universe/src/renderer.rs | 2 ++ crate_universe/src/templates/partials/git_repository.template | 1 + 2 files changed, 3 insertions(+) diff --git a/crate_universe/src/renderer.rs b/crate_universe/src/renderer.rs index 7ab450b011..ff94b896bf 100644 --- a/crate_universe/src/renderer.rs +++ b/crate_universe/src/renderer.rs @@ -614,6 +614,7 @@ mod tests { name = "rule_prefix__lazy_static__1_4_0", strip_prefix = "", build_file = Label("//:BUILD.lazy_static-1.4.0.bazel"), + init_submodules = True, remote = "https://github.com/rust-lang-nursery/lazy-static.rs.git", commit = "421669662b35fcb455f2902daed2e20bbbba79b6", ) @@ -655,6 +656,7 @@ mod tests { name = "rule_prefix__lazy_static__1_4_0", strip_prefix = "", build_file = Label("//:BUILD.lazy_static-1.4.0.bazel"), + init_submodules = True, remote = "https://github.com/rust-lang-nursery/lazy-static.rs.git", commit = "421669662b35fcb455f2902daed2e20bbbba79b6", ) diff --git a/crate_universe/src/templates/partials/git_repository.template b/crate_universe/src/templates/partials/git_repository.template index 1f1a0ea0d4..b3ea7ec728 100644 --- a/crate_universe/src/templates/partials/git_repository.template +++ b/crate_universe/src/templates/partials/git_repository.template @@ -2,6 +2,7 @@ name = "{{ repository_name }}", strip_prefix = "{{ repo.path_to_crate_root }}", build_file = Label("//:BUILD.{{crate.pkg_name}}-{{crate.pkg_version}}.bazel"), + init_submodules = True, remote = "{{ repo.remote }}", commit = "{{ repo.commit }}", ) From 004e629d719248df3e16a7589615b3cd4908a175 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Thu, 16 Dec 2021 07:38:12 -0800 Subject: [PATCH 4/4] Added `incompatible_disable_custom_test_launcher` (#1070) --- rust/private/rust.bzl | 5 +- rust/settings/BUILD.bazel | 6 ++ rust/toolchain.bzl | 5 ++ .../disable_custom_test_launcher/BUILD.bazel | 5 ++ .../disable_custom_test_launcher_test.bzl | 83 +++++++++++++++++++ util/launcher/BUILD.bazel | 3 + 6 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 test/unit/disable_custom_test_launcher/BUILD.bazel create mode 100644 test/unit/disable_custom_test_launcher/disable_custom_test_launcher_test.bzl diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index 23acb4e375..43c68cf84c 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -320,6 +320,9 @@ def _rust_binary_impl(ctx): def _create_test_launcher(ctx, toolchain, output, env, providers): """Create a process wrapper to ensure runtime environment variables are defined for the test binary + WARNING: This function is subject to deletion with the removal of + incompatible_disable_custom_test_launcher + Args: ctx (ctx): The rule's context object toolchain (rust_toolchain): The current rust toolchain @@ -474,7 +477,7 @@ def _rust_test_common(ctx, toolchain, output): ) providers.append(testing.TestEnvironment(env)) - if any(["{pwd}" in v for v in env.values()]): + if not toolchain._incompatible_disable_custom_test_launcher and any(["{pwd}" in v for v in env.values()]): # Some of the environment variables require expanding {pwd} placeholder at runtime, # we need a launcher for that. return _create_test_launcher(ctx, toolchain, output, env, providers) diff --git a/rust/settings/BUILD.bazel b/rust/settings/BUILD.bazel index b583bc0e5d..74dcc043af 100644 --- a/rust/settings/BUILD.bazel +++ b/rust/settings/BUILD.bazel @@ -15,6 +15,12 @@ incompatible_flag( issue = "https://github.com/bazelbuild/rules_rust/issues/1051", ) +incompatible_flag( + name = "incompatible_disable_custom_test_launcher", + build_setting_default = False, + issue = "https://github.com/bazelbuild/rules_rust/issues/1069", +) + bzl_library( name = "rules", srcs = glob(["**/*.bzl"]), diff --git a/rust/toolchain.bzl b/rust/toolchain.bzl index b25725a54e..4538b21bd9 100644 --- a/rust/toolchain.bzl +++ b/rust/toolchain.bzl @@ -233,6 +233,7 @@ def _rust_toolchain_impl(ctx): make_rust_providers_target_independent = ctx.attr._incompatible_make_rust_providers_target_independent[IncompatibleFlagInfo] remove_transitive_libs_from_dep_info = ctx.attr._incompatible_remove_transitive_libs_from_dep_info[IncompatibleFlagInfo] + disable_custom_test_launcher = ctx.attr._incompatible_disable_custom_test_launcher[IncompatibleFlagInfo] expanded_stdlib_linkflags = [] for flag in ctx.attr.stdlib_linkflags: @@ -286,6 +287,7 @@ def _rust_toolchain_impl(ctx): libstd_and_allocator_ccinfo = _make_libstd_and_allocator_ccinfo(ctx, ctx.attr.rust_lib, ctx.attr.allocator_library), _incompatible_make_rust_providers_target_independent = make_rust_providers_target_independent.enabled, _incompatible_remove_transitive_libs_from_dep_info = remove_transitive_libs_from_dep_info.enabled, + _incompatible_disable_custom_test_launcher = disable_custom_test_launcher.enabled, ) return [toolchain] @@ -398,6 +400,9 @@ rust_toolchain = rule( "_crosstool": attr.label( default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"), ), + "_incompatible_disable_custom_test_launcher": attr.label( + default = Label("@rules_rust//rust/settings:incompatible_disable_custom_test_launcher"), + ), "_incompatible_make_rust_providers_target_independent": attr.label( default = "@rules_rust//rust/settings:incompatible_make_rust_providers_target_independent", ), diff --git a/test/unit/disable_custom_test_launcher/BUILD.bazel b/test/unit/disable_custom_test_launcher/BUILD.bazel new file mode 100644 index 0000000000..b6fd3e9ce5 --- /dev/null +++ b/test/unit/disable_custom_test_launcher/BUILD.bazel @@ -0,0 +1,5 @@ +load(":disable_custom_test_launcher_test.bzl", "disable_custom_test_launcher_test_suite") + +disable_custom_test_launcher_test_suite( + name = "disable_custom_test_launcher_test_suite", +) diff --git a/test/unit/disable_custom_test_launcher/disable_custom_test_launcher_test.bzl b/test/unit/disable_custom_test_launcher/disable_custom_test_launcher_test.bzl new file mode 100644 index 0000000000..f4dbb0c81b --- /dev/null +++ b/test/unit/disable_custom_test_launcher/disable_custom_test_launcher_test.bzl @@ -0,0 +1,83 @@ +"""Unittests for rust rules.""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("//rust:defs.bzl", "rust_test") + +def _incompatible_enable_custom_test_launcher_test_impl(ctx): + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + + executable = tut.files_to_run.executable + asserts.true(env, executable.basename.endswith(".launcher") or executable.basename.endswith(".launcher.exe")) + + return analysistest.end(env) + +def _incompatible_disable_custom_test_launcher_test_impl(ctx): + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + + executable = tut.files_to_run.executable + asserts.false(env, executable.basename.endswith(".launcher") or executable.basename.endswith(".launcher.exe")) + + return analysistest.end(env) + +incompatible_enable_custom_test_launcher_test = analysistest.make( + _incompatible_enable_custom_test_launcher_test_impl, + config_settings = { + "@//rust/settings:incompatible_disable_custom_test_launcher": False, + }, +) + +incompatible_disable_custom_test_launcher_test = analysistest.make( + _incompatible_disable_custom_test_launcher_test_impl, + config_settings = { + "@//rust/settings:incompatible_disable_custom_test_launcher": True, + }, +) + +def _disable_custom_test_launcher_test(): + write_file( + name = "src", + out = "lib.rs", + content = [], + ) + + write_file( + name = "data", + out = "data.txt", + content = [], + ) + + rust_test( + name = "disable_custom_test_launcher_test", + srcs = [":lib.rs"], + env = {"CUSTOM_TEST_ENV": "$(execpath :data)"}, + data = [":data"], + ) + + incompatible_enable_custom_test_launcher_test( + name = "incompatible_enable_custom_test_launcher_test", + target_under_test = ":disable_custom_test_launcher_test", + ) + + incompatible_disable_custom_test_launcher_test( + name = "incompatible_disable_custom_test_launcher_test", + target_under_test = ":disable_custom_test_launcher_test", + ) + +def disable_custom_test_launcher_test_suite(name): + """Entry-point macro called from the BUILD file. + + Args: + name: Name of the macro. + """ + _disable_custom_test_launcher_test() + + native.test_suite( + name = name, + tests = [ + ":incompatible_disable_custom_test_launcher_test", + ":incompatible_enable_custom_test_launcher_test", + ], + ) diff --git a/util/launcher/BUILD.bazel b/util/launcher/BUILD.bazel index c12fdd7d58..ca8fb72439 100644 --- a/util/launcher/BUILD.bazel +++ b/util/launcher/BUILD.bazel @@ -1,3 +1,6 @@ +# WARNING: This package is subject to deletion with the removal of +# incompatible_disable_custom_test_launcher + load("//rust:defs.bzl", "rust_binary") package(default_visibility = ["//visibility:public"])