Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5.x: Use .dylib extension for macOS dynamic libraries in Unix toolchain #14360

Conversation

brentleyjones
Copy link
Contributor

The autoconfigured Unix toolchain previously did not set any artifact_name_patterns, which meant that all artifacts used the default(Linux) patterns. This was incorrect in the case of dynamic libraries on macOS, which should have the extension .dylib rather than .so.

Fixes #11082.

Closes #14158.

PiperOrigin-RevId: 413363126
(cherry picked from commit 3f1672e)

The autoconfigured Unix toolchain previously did not set any artifact_name_patterns, which meant that all artifacts used the default(Linux) patterns. This was incorrect in the case of dynamic libraries on macOS, which should have the extension `.dylib` rather than `.so`.

Fixes bazelbuild#11082.

Closes bazelbuild#14158.

PiperOrigin-RevId: 413363126
(cherry picked from commit 3f1672e)
@google-cla google-cla bot added the cla: yes label Dec 1, 2021
@brentleyjones
Copy link
Contributor Author

cc: @meteorcloudy

@brentleyjones brentleyjones marked this pull request as draft December 1, 2021 15:21
@brentleyjones
Copy link
Contributor Author

Actually, let's wait for #14354 to be merged a well, and get both in at once.

@brentleyjones brentleyjones changed the title Use .dylib extension for macOS dynamic libraries in Unix toolchain 5.x: Use .dylib extension for macOS dynamic libraries in Unix toolchain Dec 1, 2021
@meteorcloudy
Copy link
Member

Please ping me again when it's ready

@fmeum
Copy link
Collaborator

fmeum commented Dec 3, 2021

@meteorcloudy @brentleyjones Added #14369 with the second half of the fix.

@brentleyjones brentleyjones marked this pull request as ready for review December 3, 2021 13:13
@meteorcloudy
Copy link
Member

https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2273#ce43079f-ab1f-45aa-be89-18cd0f0ce098

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //cmake_with_data:test_with_shared_lib
-----------------------------------------------------------------------------
libc++abi: terminating with uncaught exception of type std::runtime_error: Could not open file: ./cmake_with_data/lib_b/liblib_b.so

Looks like this is already breaking rules_foreign_cc in downstream

@oquenchil
Copy link
Contributor

How much of a breaking change is this and how beneficial is it? It might be a bit too late for 5.x.

@fmeum
Copy link
Collaborator

fmeum commented Dec 6, 2021

How much of a breaking change is this and how beneficial is it? It might be a bit too late for 5.x.

It changes the output name of shared library cc_binarys with the macOS auto-configured toolchain shipped with Bazel. The extension was arguably never correct (.so instead of .dylib, which e.g. leads to unexpected issues when loading JNI libraries), but of course it was convenient for tests that the name did not depend on macOS vs. Linux. This is also what seems to be breaking in rules_foreign_cc.

All in all, I agree that this may be too late for Bazel 5.

@meteorcloudy
Copy link
Member

@fmeum Yeah, this change is definitely nice to have to fix the wrong name, but we probably want to give downstream more time to migrate and fix the incompatibilities. Thanks for your understanding!

@brentleyjones brentleyjones deleted the bj/use-.dylib-extension-for-macos-dynamic-libraries-in-unix-toolchain branch December 6, 2021 15:47
@fmeum
Copy link
Collaborator

fmeum commented Dec 6, 2021

@fmeum Yeah, this change is definitely nice to have to fix the wrong name, but we probably want to give downstream more time to migrate and fix the incompatibilities. Thanks for your understanding!

Just for the record: Submitted bazel-contrib/rules_foreign_cc#834 to fix the issue before it becomes a failure.

@jsharpe
Copy link
Contributor

jsharpe commented Dec 6, 2021

Is there still a mechanism to compile a shared library with a .so extension on macOS?
Python extensions require a .so extension on macOS:

❯ find ~/Library/Python -iname '*.so'
/Users/james/Library/Python/3.9/lib/python/site-packages/psycopg2/_psycopg.cpython-39-darwin.so

@meteorcloudy
Copy link
Member

cc_binary(
   name = "libfoo.so",
   srcs = [],
   ...

would still work, as long as the target name already ending with a dynamic library extension (.so, .dylib, .so, .pyd), it won't prepend or append extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants