Fix versioned shared libraries for macOS toolchain

src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java

@@ -671,6 +671,7 @@ public StructureValue build() {
  public abstract static class LibraryToLinkValue extends VariableValueAdapter {
  public static final String OBJECT_FILES_FIELD_NAME = "object_files";
  public static final String NAME_FIELD_NAME = "name";
+ public static final String PATH_FIELD_NAME = "path";
  public static final String TYPE_FIELD_NAME = "type";
  public static final String IS_WHOLE_ARCHIVE_FIELD_NAME = "is_whole_archive";
 
@@ -680,8 +681,8 @@ public static LibraryToLinkValue forDynamicLibrary(String name) {
  return new ForDynamicLibrary(name);
  }
 
- public static LibraryToLinkValue forVersionedDynamicLibrary(String name) {
- return new ForVersionedDynamicLibrary(name);
+ public static LibraryToLinkValue forVersionedDynamicLibrary(String name, String path) {
+ return new ForVersionedDynamicLibrary(name, path);
  }
 
  public static LibraryToLinkValue forInterfaceLibrary(String name) {
@@ -715,6 +716,8 @@ public VariableValue getFieldValue(
  return new StringValue(getTypeName());
  } else if (IS_WHOLE_ARCHIVE_FIELD_NAME.equals(field)) {
  return new IntegerValue(getIsWholeArchive() ? 1 : 0);
+ } else if (PATH_FIELD_NAME.equals(field)) {
+ return new StringValue(getPath());
  }
  return null;
  }
@@ -723,6 +726,10 @@ protected boolean getIsWholeArchive() {
  return false;
  }
 
+ protected String getPath() {
+ return null;
+ }
+
  protected abstract String getTypeName();
 
  @Override
@@ -802,10 +809,17 @@ protected String getTypeName() {
  }
 
  private static final class ForVersionedDynamicLibrary extends LibraryToLinkValueWithName {
- private ForVersionedDynamicLibrary(String name) {
+ private String path;
+
+ private ForVersionedDynamicLibrary(String name, String path) {
  super(name);
+ this.path = path;
  }
 
+ @Override
+ protected String getPath() {
+ return path;
+ }
  @Override
  protected String getTypeName() {
  return "versioned_dynamic_library";

src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java

@@ -613,7 +613,7 @@ private void addDynamicInputLinkOptions(
  librariesToLink.addValue(LibraryToLinkValue.forDynamicLibrary(libName));
  } else if (CppFileTypes.SHARED_LIBRARY.matches(name)
  || CppFileTypes.VERSIONED_SHARED_LIBRARY.matches(name)) {
- librariesToLink.addValue(LibraryToLinkValue.forVersionedDynamicLibrary(name));
+ librariesToLink.addValue(LibraryToLinkValue.forVersionedDynamicLibrary(name, inputArtifact.getExecPathString()));
  } else {
  // Interface shared objects have a non-standard extension
  // that the linker won't be able to find. So use the

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test

@@ -115,6 +115,7 @@ cc_shared_library(
  "bar_so",
  "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg_so",
  "private_lib_so",
+ "renamed_so_file_2",
  ],
  features = ["windows_export_all_symbols"],
  exports_filter = [
@@ -385,6 +386,11 @@ cc_library(
  srcs = [":private_cc_library.cc"]
 )
 
+cc_library(
+ name = "private_lib_2",
+ srcs = [":private_cc_library.cc"]
+)
+
 build_failure_test(
  name = "link_once_repeated_test_binary",
  messages = [
@@ -447,6 +453,15 @@ cc_shared_library(
  shared_lib_name = "renamed_so_file.so",
 )
 
+cc_shared_library(
+ name = "renamed_so_file_2",
+ features = ["windows_export_all_symbols"],
+ deps = [
+ ":private_lib_2",
+ ],
+ shared_lib_name = "renamed_so_file_2.so",
+)
+
 cc_library(
  name = "direct_so_file_cc_lib",
  srcs = [

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl

@@ -166,6 +166,7 @@ def _runfiles_test_impl(ctx):
  "libbar_so.so",
  "libdiff_pkg_so.so",
  "libprivate_lib_so.so",
+ "renamed_so_file_2.so",
  "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibfoo_Uso.so",
  "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibbar_Uso.so",
  "Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary3_Slibdiff_Upkg_Uso.so",

src/test/java/com/google/devtools/build/lib/rules/cpp/LibraryToLinkValueTest.java

@@ -47,9 +47,9 @@ public void equalsAndHashCode() {
 
  // #forVersionedDynamicLibrary
  equalsTester.addEqualityGroup(
- LibraryToLinkValue.forVersionedDynamicLibrary("foo"),
- LibraryToLinkValue.forVersionedDynamicLibrary("foo"));
- equalsTester.addEqualityGroup(LibraryToLinkValue.forVersionedDynamicLibrary("bar"));
+ LibraryToLinkValue.forVersionedDynamicLibrary("foo", /* path= */ null),
+ LibraryToLinkValue.forVersionedDynamicLibrary("foo", /* path= */ null));
+ equalsTester.addEqualityGroup(LibraryToLinkValue.forVersionedDynamicLibrary("bar", /* path= */ null));
 
  // #forInterfaceLibrary
  equalsTester.addEqualityGroup(
@@ -135,14 +135,23 @@ public void getFieldValue_forDynamicLibrary() {
 
  @Test
  public void getFieldValue_forVersionedDynamicLibrary() {
- LibraryToLinkValue libraryToLinkValue = LibraryToLinkValue.forVersionedDynamicLibrary("foo");
+ LibraryToLinkValue libraryToLinkValue = LibraryToLinkValue.forVersionedDynamicLibrary("foo", "foo/bar.so");
  assertThat(
  libraryToLinkValue.getFieldValue(
  /*variableName=*/ "variable name doesn't matter",
  /*field=*/ "type",
  /*expander=*/ null,
  /*throwOnMissingVariable=*/ false))
  .isEqualTo(new StringValue("versioned_dynamic_library"));
+ assertThat(
+ libraryToLinkValue
+ .getFieldValue(
+ /* variableName= */ "variable name doesn't matter",
+ /* field= */ "path",
+ /* expander= */ null,
+ /* throwOnMissingVariable= */ false)
+ .getStringValue("variable name doesn't matter"))
+ .isEqualTo("foo/bar.so");
  assertThat(
  libraryToLinkValue.getFieldValue(
  /*variableName=*/ "variable name doesn't matter",

tools/cpp/unix_cc_toolchain_config.bzl

@@ -811,6 +811,24 @@ def _impl(ctx):
  ],
  )
 
+ is_linux = ctx.attr.target_libc != "macosx"
+ if is_linux:
+ versioned_library_flag_group = flag_group(
+ flags = ["-l:%{libraries_to_link.name}"],
+ expand_if_equal = variable_with_value(
+ name = "libraries_to_link.type",
+ value = "versioned_dynamic_library",
+ ),
+ )
+ else:
+ versioned_library_flag_group = flag_group(
+ flags = ["%{libraries_to_link.path}"],
+ expand_if_equal = variable_with_value(
+ name = "libraries_to_link.type",
+ value = "versioned_dynamic_library",
+ ),
+ )
+
  libraries_to_link_feature = feature(
  name = "libraries_to_link",
  flag_sets = [
@@ -868,13 +886,7 @@ def _impl(ctx):
  value = "dynamic_library",
  ),
  ),
- flag_group(
- flags = ["-l:%{libraries_to_link.name}"],
- expand_if_equal = variable_with_value(
- name = "libraries_to_link.type",
- value = "versioned_dynamic_library",
- ),
- ),
+ versioned_library_flag_group,
  flag_group(
  flags = ["-Wl,-no-whole-archive"],
  expand_if_true = "libraries_to_link.is_whole_archive",
@@ -1283,7 +1295,6 @@ def _impl(ctx):
  ],
  )
 
- is_linux = ctx.attr.target_libc != "macosx"
  libtool_feature = feature(
  name = "libtool",
  enabled = not is_linux,