Skip to content

Commit

Permalink
Support features in apple_common.link_multi_arch_binary
Browse files Browse the repository at this point in the history
This will be used to convert some linkopts in apple rules to features, which in
turn will allow the crosstool to select linking options based on the kind of
linking we're doing (executable, bundle, relocatible, or dylib).

Note we couldn't pass in a feature_configuration from the starlark, because it
is before the split transition.  We need to pass feature strings into native
code, and have it apply those features after the transition.

PiperOrigin-RevId: 542032473
Change-Id: I0b769744db5d3286e7af8d9af173151d0b838738
  • Loading branch information
googlewalt authored and copybara-github committed Jun 20, 2023
1 parent 4d14d3f commit 1f8951c
Show file tree
Hide file tree
Showing 8 changed files with 141 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ public static AppleLinkingOutputs linkMultiArchBinary(
ImmutableList<TransitiveInfoCollection> avoidDeps,
Iterable<String> extraLinkopts,
Iterable<Artifact> extraLinkInputs,
Iterable<String> extraRequestedFeatures,
Iterable<String> extraDisabledFeatures,
boolean isStampingEnabled)
throws InterruptedException, RuleErrorException, ActionConflictException {
Map<Optional<String>, List<ConfiguredTargetAndData>> splitDeps =
Expand Down Expand Up @@ -137,6 +139,8 @@ public static AppleLinkingOutputs linkMultiArchBinary(
dependencySpecificConfiguration,
new ExtraLinkArgs(allLinkopts.build()),
allLinkInputs.build(),
extraRequestedFeatures,
extraDisabledFeatures,
isStampingEnabled,
propagatedDeps,
outputGroupCollector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ public StructImpl linkMultiArchBinary(
Object avoidDeps,
Sequence<?> extraLinkopts,
Sequence<?> extraLinkInputs,
Sequence<?> extraRequestedFeatures,
Sequence<?> extraDisabledFeatures,
StarlarkInt stamp,
StarlarkThread thread)
throws EvalException, InterruptedException {
Expand All @@ -277,6 +279,8 @@ public StructImpl linkMultiArchBinary(
avoidDepsList,
ImmutableList.copyOf(Sequence.cast(extraLinkopts, String.class, "extra_linkopts")),
Sequence.cast(extraLinkInputs, Artifact.class, "extra_link_inputs"),
Sequence.cast(extraRequestedFeatures, String.class, "extra_requested_features"),
Sequence.cast(extraDisabledFeatures, String.class, "extra_disabled_features"),
isStampingEnabled);
return createStarlarkLinkingOutputs(linkingOutputs, thread);
} catch (RuleErrorException | ActionConflictException exception) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,8 @@ public CompilationSupport registerLinkActions(
J2ObjcEntryClassProvider j2ObjcEntryClassProvider,
ExtraLinkArgs extraLinkArgs,
Iterable<Artifact> extraLinkInputs,
Iterable<String> extraRequestedFeatures,
Iterable<String> extraDisabledFeatures,
boolean isStampingEnabled)
throws InterruptedException, RuleErrorException {
ObjcProvider objcProviderWithLinkingInfo = null;
Expand Down Expand Up @@ -585,12 +587,22 @@ public CompilationSupport registerLinkActions(
// CppLinkAction too, so create it now.
Artifact inputFileList = intermediateArtifacts.linkerObjList();

ImmutableSet<String> allRequestedFeatures =
new ImmutableSet.Builder<String>()
.addAll(ruleContext.getFeatures())
.addAll(extraRequestedFeatures)
.build();
ImmutableSet<String> allDisabledFeatures =
new ImmutableSet.Builder<String>()
.addAll(ruleContext.getDisabledFeatures())
.addAll(extraDisabledFeatures)
.build();
FeatureConfiguration featureConfiguration =
CcCommon.configureFeaturesOrReportRuleError(
ruleContext,
buildConfiguration,
ruleContext.getFeatures(),
ruleContext.getDisabledFeatures(),
allRequestedFeatures,
allDisabledFeatures,
Language.OBJC,
toolchain,
cppSemantics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ public Artifact registerConfigurationSpecificLinkActions(
DependencySpecificConfiguration dependencySpecificConfiguration,
ExtraLinkArgs extraLinkArgs,
Iterable<Artifact> extraLinkInputs,
Iterable<String> extraRequestedFeatures,
Iterable<String> extraDisabledFeatures,
boolean isStampingEnabled,
Iterable<? extends TransitiveInfoCollection> infoCollections,
Map<String, NestedSet<Artifact>> outputMapCollector)
Expand Down Expand Up @@ -165,6 +167,8 @@ public Artifact registerConfigurationSpecificLinkActions(
j2ObjcEntryClassProvider,
extraLinkArgs,
extraLinkInputs,
extraRequestedFeatures,
extraDisabledFeatures,
isStampingEnabled)
.validateAttributes();
ruleContext.assertNoErrors();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,20 @@ AppleExecutableBinaryApi newExecutableBinaryProvider(
positional = false,
defaultValue = "[]",
doc = "Extra files to pass to the linker action."),
@Param(
name = "extra_requested_features",
allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)},
named = true,
positional = false,
defaultValue = "[]",
doc = "Extra requested features to be passed to the linker action."),
@Param(
name = "extra_disabled_features",
allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)},
named = true,
positional = false,
defaultValue = "[]",
doc = "Extra disabled features to be passed to the linker action."),
@Param(
name = "stamp",
named = true,
Expand All @@ -409,6 +423,8 @@ StructApi linkMultiArchBinary(
Object avoidDeps, // Sequence<TransitiveInfoCollection> expected.
Sequence<?> extraLinkopts, // <String> expected.
Sequence<?> extraLinkInputs, // <? extends FileApi> expected.
Sequence<?> extraRequestedFeatures, // <String> expected.
Sequence<?> extraDisabledFeatures, // <String> expected.
StarlarkInt stamp,
StarlarkThread thread)
throws EvalException, InterruptedException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,39 @@ _parse_headers_feature = feature(
name = "parse_headers",
)

_special_linking_feature = feature(
name = "special_linking_feature",
)

_special_linking_flags_feature = feature(
"special_linking_flags_feature",
enabled = True,
flag_sets = [
flag_set(
actions = _ALL_LINK_ACTIONS,
flag_groups = [flag_group(flags = ["--special_linking_flag"])],
with_features = [with_feature_set(features = ["special_linking_feature"])],
),
],
)

_default_enabled_linking_feature = feature(
name = "default_enabled_linking_feature",
enabled = True,
)

_default_enabled_linking_flags_feature = feature(
"default_enabled_linking_flags_feature",
enabled = True,
flag_sets = [
flag_set(
actions = _ALL_LINK_ACTIONS,
flag_groups = [flag_group(flags = ["--default_enabled_linking_flag"])],
with_features = [with_feature_set(features = ["default_enabled_linking_feature"])],
),
],
)

_feature_name_to_feature = {
"archive_param_file": _archive_param_file_feature,
"default_feature": _default_feature,
Expand All @@ -89,6 +122,10 @@ _feature_name_to_feature = {
"supports_interface_shared_libraries": _supports_interface_shared_libraries_feature,
"supports_dynamic_linker": _supports_dynamic_linker_feature,
"parse_headers": _parse_headers_feature,
"special_linking_feature": _special_linking_feature,
"special_linking_flags_feature": _special_linking_flags_feature,
"default_enabled_linking_feature": _default_enabled_linking_feature,
"default_enabled_linking_flags_feature": _default_enabled_linking_flags_feature,
}

_action_name_to_action = {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1330,4 +1330,62 @@ public void testRuntimeLibPostMigration() throws Exception {
useConfiguration(linkingInfoMigrationFlag(/* linkingInfoMigration= */ true));
checkRuntimeLib();
}

@Test
public void testLinkMultiArchBinaryCanEnableFeature() throws Exception {
MockObjcSupport.setupCcToolchainConfig(
mockToolsConfig,
MockObjcSupport.darwinX86_64()
.withFeatures("special_linking_feature", "special_linking_flags_feature"));
scratch.file(
"package/BUILD",
"load('//test_starlark:apple_binary_starlark.bzl', 'apple_binary_starlark')",
"objc_library(",
" name = 'lib',",
" srcs = ['a.m'],",
")",
"apple_binary_starlark(name = 'test_with_feature',",
" deps = [ ':lib' ],",
" extra_requested_features = [ 'special_linking_feature' ],",
" platform_type = 'macos')",
"apple_binary_starlark(name = 'test_without_feature',",
" deps = [ ':lib' ],",
" platform_type = 'macos')");

CommandAction actionWithFeature = linkAction("//package:test_with_feature");
CommandAction actionWithoutFeature = linkAction("//package:test_without_feature");

assertThat(actionWithFeature.getArguments()).contains("--special_linking_flag");
assertThat(actionWithoutFeature.getArguments()).doesNotContain("--special_linking_flag");
}

@Test
public void testLinkMultiArchBinaryCanDisableFeature() throws Exception {
MockObjcSupport.setupCcToolchainConfig(
mockToolsConfig,
MockObjcSupport.darwinX86_64()
.withFeatures(
"default_enabled_linking_feature", "default_enabled_linking_flags_feature"));
scratch.file(
"package/BUILD",
"load('//test_starlark:apple_binary_starlark.bzl', 'apple_binary_starlark')",
"objc_library(",
" name = 'lib',",
" srcs = ['a.m'],",
")",
"apple_binary_starlark(name = 'test_disabled_feature',",
" deps = [ ':lib' ],",
" extra_disabled_features = [ 'default_enabled_linking_feature' ],",
" platform_type = 'macos')",
"apple_binary_starlark(name = 'test_enabled_feature',",
" deps = [ ':lib' ],",
" platform_type = 'macos')");

CommandAction actionDisabledFeature = linkAction("//package:test_disabled_feature");
CommandAction actionEnabledFeature = linkAction("//package:test_enabled_feature");

assertThat(actionDisabledFeature.getArguments())
.doesNotContain("--default_enabled_linking_flag");
assertThat(actionEnabledFeature.getArguments()).contains("--default_enabled_linking_flag");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,8 @@ protected static void addAppleBinaryStarlarkRule(Scratch scratch) throws Excepti
" avoid_deps = all_avoid_deps,",
" extra_linkopts = linkopts,",
" extra_link_inputs = link_inputs,",
" extra_requested_features = ctx.attr.extra_requested_features,",
" extra_disabled_features = ctx.attr.extra_disabled_features,",
" stamp = ctx.attr.stamp,",
" )",
" processed_binary = ctx.actions.declare_file('{}_lipobin'.format(ctx.label.name))",
Expand Down Expand Up @@ -660,6 +662,8 @@ protected static void addAppleBinaryStarlarkRule(Scratch scratch) throws Excepti
" cfg=apple_common.multi_arch_split,",
" ),",
" 'linkopts': attr.string_list(),",
" 'extra_requested_features': attr.string_list(),",
" 'extra_disabled_features': attr.string_list(),",
" 'platform_type': attr.string(),",
" 'minimum_os_version': attr.string(),",
" 'stamp': attr.int(values=[-1,0,1],default=-1),",
Expand Down

0 comments on commit 1f8951c

Please sign in to comment.