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

Add -install_name when linking shared libraries on macOS #12304

Closed
wants to merge 10 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ public CcToolchainVariables getLinkBuildVariables(
isUsingLinkerNotArchiver,
/* binDirectoryPath= */ null,
convertFromNoneable(outputFile, /* defaultValue= */ null),
/* runtimeSolibName= */ null,
isCreatingSharedLibrary,
convertFromNoneable(paramFile, /* defaultValue= */ null),
/* thinltoParamFile= */ null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -770,4 +770,9 @@ public boolean useCppCompileHeaderMnemonic() {
public boolean generateLlvmLCov() {
return cppOptions.generateLlvmLcov;
}

@Override
public boolean macosSetInstallName() {
return cppOptions.macosSetInstallName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,7 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
getLinkType().linkerOrArchiver().equals(LinkerOrArchiver.LINKER),
configuration.getBinDirectory(repositoryName).getExecPath(),
output.getExecPathString(),
SolibSymlinkAction.getDynamicLibrarySoname(output.getRootRelativePath(), false),
linkType.equals(LinkTargetType.DYNAMIC_LIBRARY),
paramFile != null ? paramFile.getExecPathString() : null,
thinltoParamFile != null ? thinltoParamFile.getExecPathString() : null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,20 @@ public Label getPropellerOptimizeLabel() {
help = "If enabled, give distinguishing mnemonic to header processing actions")
public boolean useCppCompileHeaderMnemonic;

@Option(
name = "incompatible_macos_set_install_name",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"Whether to explicitly set `-install_name` when creating dynamic libraries. "
+ "See https://github.com/bazelbuild/bazel/issues/12370")
public boolean macosSetInstallName;

/** See {@link #targetLibcTopLabel} documentation. * */
@Override
public FragmentOptions getNormalized() {
Expand Down Expand Up @@ -1133,6 +1147,8 @@ public FragmentOptions getHost() {
host.hostLinkoptList = hostLinkoptList;

host.experimentalStarlarkCcImport = experimentalStarlarkCcImport;

host.macosSetInstallName = macosSetInstallName;

return host;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ public enum LinkBuildVariables {
/** Path to the context sensitive fdo instrument. */
CS_FDO_INSTRUMENT_PATH("cs_fdo_instrument_path"),
/** Path to the Propeller Optimize linker profile artifact */
PROPELLER_OPTIMIZE_LD_PATH("propeller_optimize_ld_path");
PROPELLER_OPTIMIZE_LD_PATH("propeller_optimize_ld_path"),
/** The name of the runtime solib symlink of the shared library. */
RUNTIME_SOLIB_NAME("runtime_solib_name");

private final String variableName;

Expand All @@ -98,6 +100,7 @@ public static CcToolchainVariables setupVariables(
boolean isUsingLinkerNotArchiver,
PathFragment binDirectoryPath,
String outputFile,
String runtimeSolibName,
boolean isCreatingSharedLibrary,
String paramFile,
String thinltoParamFile,
Expand Down Expand Up @@ -166,6 +169,10 @@ public static CcToolchainVariables setupVariables(
buildVariables.addStringVariable(OUTPUT_EXECPATH.getVariableName(), outputFile);
}

if (runtimeSolibName != null && !isLtoIndexing) {
buildVariables.addStringVariable(RUNTIME_SOLIB_NAME.getVariableName(), runtimeSolibName);
}

if (isLtoIndexing) {
if (thinltoParamFile != null) {
// This is a lto-indexing action and we want it to populate param file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,12 @@ public interface CppConfigurationApi<InvalidConfigurationExceptionT extends Exce
+ " )<br/>"
+ ")</pre>")
Label customMalloc();

@StarlarkMethod(
name = "do_not_use_macos_set_install_name",
structField = true,
// Only for migration purposes. Intentionally not documented.
documented = false,
doc = "Accessor for <code>--incompatible_macos_set_install_name</code>.")
boolean macosSetInstallName();
}
37 changes: 37 additions & 0 deletions src/test/shell/bazel/cpp_darwin_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -123,5 +123,42 @@ EOF
return 0
}

function test_cc_test_with_explicit_install_name() {
mkdir -p cpp
cat > cpp/BUILD <<EOF
cc_library(
name = "foo",
srcs = ["foo.cc"],
hdrs = ["foo.h"],
)
cc_test(
name = "test",
srcs = ["test.cc"],
deps = [":foo"],
)
EOF
cat > cpp/foo.h <<EOF
int foo();
EOF
cat > cpp/foo.cc <<EOF
int foo() { return 0; }
EOF
cat > cpp/test.cc <<EOF
#include "cpp/foo.h"
int main() {
return foo();
}
EOF

bazel test --incompatible_macos_set_install_name //cpp:test || \
fail "bazel test //cpp:test failed"
# Ensure @rpath is correctly set in the binary.
./bazel-bin/cpp/test || \
fail "//cpp:test workspace execution failed, expected return 0, got $?"
cd bazel-bin
./cpp/test || \
fail "//cpp:test execution failed, expected 0, but $?"
}

run_suite "Tests for Bazel's C++ rules on Darwin"

24 changes: 24 additions & 0 deletions tools/osx/crosstool/cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -6065,6 +6065,28 @@ def _impl(ctx):
],
)

set_install_name = feature(
name = "set_install_name",
enabled = ctx.fragments.cpp.do_not_use_macos_set_install_name,
flag_sets = [
flag_set(
actions = [
ACTION_NAMES.cpp_link_dynamic_library,
ACTION_NAMES.cpp_link_nodeps_dynamic_library,
],
flag_groups = [
flag_group(
flags = [
"-install_name",
"@rpath/%{runtime_solib_name}",
],
expand_if_available = "runtime_solib_name",
),
],
),
],
)

if (ctx.attr.cpu == "ios_arm64" or
ctx.attr.cpu == "ios_arm64e" or
ctx.attr.cpu == "ios_armv7" or
Expand Down Expand Up @@ -6228,6 +6250,7 @@ def _impl(ctx):
supports_dynamic_linker_feature,
objcopy_embed_flags_feature,
dynamic_linking_mode_feature,
set_install_name,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this feature apply to dylibs built for iOS as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we should, done. PTAL

]
elif (ctx.attr.cpu == "armeabi-v7a"):
features = [
Expand Down Expand Up @@ -6406,4 +6429,5 @@ cc_toolchain_config = rule(
},
provides = [CcToolchainConfigInfo],
executable = True,
fragments = ["cpp"],
)