Fix linking against versioned shared library

[djmarcin]

I was having trouble linking against libruby.so because at runtime the linker tries to find a versioned form like libruby.so.2.7. Versioned shared libraries were being filtered out of the depset because they don't end with .so. rust_test rules would then fail at runtime because although libruby.so existed in the sandboxed _solib directory, libruby.so.2.7 did not.

starlark does not have regex support, so I cannot directly check for version numbers after the extension. Instead, it uses the assumption (also made elsewhere) that library names do not contain periods before the extension.

[djmarcin]

I believe this will likely fix #91

[illicitonion]

Seems reasonable - any chance you can add/modify an example so that we have a test for this behaviour?

[djmarcin]

I was trying to figure out the right way to do this. I'm not sure if I can get bazel to create a shared library with the appropriate suffix, so I might have to include a very small dummy shared object as a binary in order to write a test, would that be reasonable or do you have a better idea?

…

On Tue, Dec 1, 2020 at 12:59 AM Daniel Wagner-Hall ***@***.***> wrote: ***@***.**** commented on this pull request. Seems reasonable - any chance you can add/modify an example so that we have a test for this behaviour? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#513 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA5AFZXCJGJADKUGHQZOTDSSSV7PANCNFSM4UHLPZBQ> .

[dae]

Maybe you could use copy_file to rename it?

[illicitonion]

If copy_file works, that would be grand, if not, introducing a <1k shared library (ideally for at least two platforms with differing extensions) doesn't feel like the end of the world :)

[djmarcin]

I added a test, but I'm having trouble making it pass on non-linux platforms. The problem is that it appears that dynamic linking is not allowed on darwin:

(09:49:40) ERROR: /Users/buildkite/builds/bk-imacpro-3/bazel/rules-rust-rustlang/test/versioned_dylib/BUILD:9:12: in rust_binary rule //test/versioned_dylib:versioned_dylib:
  | Traceback (most recent call last):
  | File "/Users/buildkite/builds/bk-imacpro-3/bazel/rules-rust-rustlang/rust/private/rust.bzl", line 200, column 32, in _rust_binary_impl
  | return rustc_compile_action(
  | File "/Users/buildkite/builds/bk-imacpro-3/bazel/rules-rust-rustlang/rust/private/rustc.bzl", line 613, column 36, in rustc_compile_action
  | args, env = construct_arguments(
  | File "/Users/buildkite/builds/bk-imacpro-3/bazel/rules-rust-rustlang/rust/private/rustc.bzl", line 539, column 33, in construct_arguments
  | rpaths = _compute_rpaths(toolchain, output_dir, dep_info)
  | File "/Users/buildkite/builds/bk-imacpro-3/bazel/rules-rust-rustlang/rust/private/rustc.bzl", line 777, column 13, in _compute_rpaths
  | fail("Runtime linking is not supported on {}, but found {}".format(
  | Error in fail: Runtime linking is not supported on darwin, but found depset([<generated file _solib_darwin_x86_64/_U_S_Stest_Sversioned_Udylib_Sc_Clibreturn_Uzero___Utest_Sversioned_Udylib_Sc/libreturn_zero.2.dylib>, <generated file _solib_darwin_x86_64/_U_S_Stest_Sversioned_Udylib_Sc_Clibreturn_Uzero___Utest_Sversioned_Udylib_Sc/libreturn_zero.dylib>], order = "topological")

Additionally, there doesn't appear to be a way to have targets that only build on one platform until bazel 4.0 comes out. 3.7.0 has compatible_with on the cc_library which looks like what I want, but it can't be used with contraints:

in compatible_with attribute of cc_binary rule //test/versioned_dylib/c:libreturn_zero.so: constraint_value rule '@platforms//os:linux' is misplaced here (expected environment). Since this rule was c
reated by the macro 'cc_binary', the error might have been caused by the macro implementation

The last idea I had was to mark the rules manual and use tags = select({...}) on the test to make it manual on non-linux and automatically run on linux. Unfortunately tags are not configurable:

//test/versioned_dylib:versioned_dylib_test: attribute "tags" is not configurable

I'm out of ideas on how to make the test work on other platforms. Any tips?

[illicitonion]

Sigh. Sorry about the wild chase! Let's make the test linux only, and add it as an excluded test in .bazelci/presumbit.yml for now - we can tidy things up when 4.0 happens and if/when the linux-only error you hit gets resolved.

[UebelAndre]

Sigh. Sorry about the wild chase! Let's make the test linux only, and add it as an excluded test in .bazelci/presumbit.yml for now - we can tidy things up when 4.0 happens and if/when the linux-only error you hit gets resolved.

Can we please just add the test in a later PR? I'd rather not break the ability to bazel test //... for MacOS 🙏

[illicitonion]

If we tag it as manual, and explicitly include it in the linux version of .bazelci/presumbit.yml, we get the best of both worlds :)

[illicitonion]

Thanks!