From 222ded5c614b37dd528df4ac8a5f520ca3e5b903 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Mon, 13 Dec 2021 11:18:39 +0100 Subject: [PATCH 1/3] use absolute value of hash function to determine crate name No functional changes intended. Previously the library names of rlibs sometimes had two dashes, like libslib--389267823.rlib. This happens because the hash() function may return a negative value, so the second dash is the minus sign. However such names are a bit confusing since they look like there's a missing component between the two dashes that didn't get properly propagated, which is not true. For the unit test, since the hash() function is the standard Java String hashCode function, I chose "slib" since that produces a negative hash code. --- rust/private/utils.bzl | 7 ++++++- test/unit/crate_name/crate_name_test.bzl | 22 ++++++++++++++++++++++ test/unit/crate_name/slib.rs | 1 + 3 files changed, 29 insertions(+), 1 deletion(-) 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/unit/crate_name/crate_name_test.bzl b/test/unit/crate_name/crate_name_test.bzl index 2a5d8ce424..a0b3dae23f 100644 --- a/test/unit/crate_name/crate_name_test.bzl +++ b/test/unit/crate_name/crate_name_test.bzl @@ -56,6 +56,15 @@ def _invalid_custom_crate_name_test_impl(ctx): asserts.expect_failure(env, "contains invalid character(s): -") return analysistest.end(env) +# Regression test to check 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). +def _slib_library_name_test_impl(ctx): + 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 +91,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 +142,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 60a125d3637d7cb80980c142f87810019ccf1273 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Mon, 13 Dec 2021 11:46:23 +0100 Subject: [PATCH 2/3] update hello test --- test/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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", ) From e6a6e49c88fdb30319c9c1a82f2105d4886bf6cc Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Tue, 14 Dec 2021 11:19:04 +0100 Subject: [PATCH 3/3] move comment to docstring --- test/unit/crate_name/crate_name_test.bzl | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/unit/crate_name/crate_name_test.bzl b/test/unit/crate_name/crate_name_test.bzl index a0b3dae23f..217bdfd92b 100644 --- a/test/unit/crate_name/crate_name_test.bzl +++ b/test/unit/crate_name/crate_name_test.bzl @@ -56,10 +56,16 @@ def _invalid_custom_crate_name_test_impl(ctx): asserts.expect_failure(env, "contains invalid character(s): -") return analysistest.end(env) -# Regression test to check 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). 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")