Add versioned soname support to interface_library #16100
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
m3rcuriel
force-pushed
the
add-versioned-soname-to-interface-library
branch
3 times, most recently
from
8e9315e
to
52387b5
Compare
m3rcuriel
force-pushed
the
add-versioned-soname-to-interface-library
branch
from
52387b5
to
c0ca3db
Compare
sgowroji
added
team-Rules-CPP
oquenchil
requested review from
buildbreaker2021
and removed request for
lberki and
oquenchil
| @@ -57,6 +57,10 @@ def _perform_error_checks( | |||
| not cc_helper.is_shared_library_extension_valid(shared_library_artifact.basename)): | |||
| fail("'shared_library' does not produce any cc_import shared_library files (expected .so, .dylib or .dll)") | |||
|
|
|||
| if (interface_library_artifact != None and | |||
| not cc_helper.is_interface_library_extension_valid(interface_library_artifact.basename)): | |||
| fail("'interface_library' does not produce any cc_import interface_library files (expected .ifso, .tbd, .dll.a, .dylib, .so, or .lib)") | |||
There was a problem hiding this comment.
Remove comma before "or" for consistency.
| @@ -532,6 +532,9 @@ def _is_versioned_shared_library_extension_valid(shared_library_name): | |||
| return True | |||
| return False | |||
|
|
|||
| def _is_versioned_shared_library_extension_valid(shared_library_name): | |||
There was a problem hiding this comment.
Instead of adding this, could we rename _is_versioned_shared_library_extension_valid to _is_versioned_library_extension_valid and do a minor refactoring in cc_helper.bzl? _is_versioned_shared_library_extension_valid is private and not exported so it should just touch one file.
buildbreaker2021
added
awaiting-user-response
m3rcuriel
force-pushed
the
add-versioned-soname-to-interface-library
branch
from
2825199
to
af98999
Compare
buildbreaker2021
approved these changes
Jan 30, 2023
buildbreaker2021
added
awaiting-PR-merge
|
Hello @m3rcuriel, Could you please fix the buildkite errors. Above PR is awaiting to merge. Thanks! |
sgowroji
added
awaiting-user-response
|
Does this allow linking two versions of the same library? I'm seeing this issue when doing so: The same example can be linked successfully outside of Bazel. |
|
Hi @m3rcuriel, Could you provide an update on this PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There doesn't seem to be a compelling reason to not support this, and there's no other way (it seems) to allow one to provide a shim (with a versioned filename) that won't end up in the runfiles tree.
This mostly consists of adding the
VERSIONED_SHARED_LIBRARYsupport through to theinterface_libraryand has the practical effect of allowing.dylib.VERSIONand.so.VERSIONfor that field. I don't know enough aboutsoname/versioning/Windows to claim confidently that these are the only two extensions one would put versions on but it's all I've ever seen and matchesshared_library.Probably fixes #7059