diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleBitcodeConverter.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleBitcodeConverter.java new file mode 100644 index 00000000000000..c6c4712b09b7e6 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleBitcodeConverter.java @@ -0,0 +1,103 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.apple; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Maps; +import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions.AppleBitcodeMode; +import com.google.devtools.common.options.Converter; +import com.google.devtools.common.options.OptionsParsingException; +import java.util.Map; + +/** + * Converts the {@code --apple_bitcode} command line option to a pair containing an optional + * platform type and the bitcode mode to apply to builds targeting that platform. + */ +public final class AppleBitcodeConverter + implements Converter> { + /** Used to convert Bitcode mode strings to their enum value. */ + private static final AppleBitcodeMode.Converter MODE_CONVERTER = new AppleBitcodeMode.Converter(); + + /** Used to convert Apple platform type strings to their enum value. */ + private static final AppleCommandLineOptions.PlatformTypeConverter PLATFORM_TYPE_CONVERTER = + new AppleCommandLineOptions.PlatformTypeConverter(); + + private static final String TYPE_DESCRIPTION = + String.format( + "'mode' or 'platform=mode', where 'mode' is %s, and 'platform' is %s", + MODE_CONVERTER.getTypeDescription(), PLATFORM_TYPE_CONVERTER.getTypeDescription()); + + @VisibleForTesting + public static final String INVALID_APPLE_BITCODE_OPTION_FORMAT = + "Apple Bitcode mode must be in the form " + TYPE_DESCRIPTION; + + @Override + public Map.Entry convert(String input) + throws OptionsParsingException { + ApplePlatform.PlatformType platformType; + AppleBitcodeMode mode; + + int pos = input.indexOf('='); + if (pos < 0) { + // If there was no '=', then parse it as a Bitcode mode and apply it to all platforms (by + // using a null key in the entry). + platformType = null; + mode = convertAppleBitcodeMode(input); + } else { + // If there was a '=', then parse the platform type from the left side, the Bitcode mode from + // the right side, and apply it to just that platform. + String platformTypeName = input.substring(0, pos); + String modeName = input.substring(pos + 1); + + platformType = convertPlatformType(platformTypeName); + mode = convertAppleBitcodeMode(modeName); + } + + return Maps.immutableEntry(platformType, mode); + } + + @Override + public String getTypeDescription() { + return TYPE_DESCRIPTION; + } + + /** + * Returns the {@code AppleBitcodeMode} value that is equivalent to the given string. + * + * @throws OptionsParsingException if the string was not a valid Apple Bitcode mode + */ + private static AppleBitcodeMode convertAppleBitcodeMode(String input) + throws OptionsParsingException { + try { + return MODE_CONVERTER.convert(input); + } catch (OptionsParsingException e) { + throw new OptionsParsingException(INVALID_APPLE_BITCODE_OPTION_FORMAT, e); + } + } + + /** + * Returns the {@code ApplePlatform.PlatformType} value that is equivalent to the given string. + * + * @throws OptionsParsingException if any of the strings was not a valid Apple platform type + */ + private static ApplePlatform.PlatformType convertPlatformType(String input) + throws OptionsParsingException { + try { + return PLATFORM_TYPE_CONVERTER.convert(input); + } catch (OptionsParsingException e) { + throw new OptionsParsingException(INVALID_APPLE_BITCODE_OPTION_FORMAT, e); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java index 72fbe36a9cd09b..cab9a1ab378eb8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java @@ -38,6 +38,7 @@ import com.google.protobuf.CodedOutputStream; import java.io.IOException; import java.util.List; +import java.util.Map; /** Command-line options for building for Apple platforms. */ public class AppleCommandLineOptions extends FragmentOptions { @@ -355,18 +356,21 @@ public DefaultProvisioningProfileConverter() { } @Option( - name = "apple_bitcode", - converter = AppleBitcodeMode.Converter.class, - // TODO(blaze-team): Default to embedded_markers when fully implemented. - defaultValue = "none", - documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS, - effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE}, - help = - "Specify the Apple bitcode mode for compile steps. " - + "Values: 'none', 'embedded_markers', 'embedded'." - ) - public AppleBitcodeMode appleBitcodeMode; - + name = "apple_bitcode", + allowMultiple = true, + converter = AppleBitcodeConverter.class, + defaultValue = "null", + documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS, + effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE}, + help = + "Specify the Apple bitcode mode for compile steps targeting device architectures. Values" + + " are of the form '[platform=]mode', where the platform (which must be 'ios'," + + " 'macos', 'tvos', or 'watchos') is optional. If provided, the bitcode mode is" + + " applied for that platform specifically; if omitted, it is applied for all" + + " platforms. The mode must be 'none', 'embedded_markers', or 'embedded'. This" + + " option may be provided multiple times.") + public List> appleBitcodeMode; + /** Returns whether the minimum OS version is explicitly set for the current platform. */ public DottedVersion getMinimumOsVersion() { DottedVersion.Option option; diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java index 9e4307649be25b..b403fefad6f02f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java @@ -33,7 +33,10 @@ import com.google.devtools.build.lib.rules.apple.ApplePlatform.PlatformType; import com.google.devtools.build.lib.skylarkbuildapi.apple.AppleConfigurationApi; import java.util.ArrayList; +import java.util.Arrays; +import java.util.EnumMap; import java.util.List; +import java.util.Map; import javax.annotation.Nullable; /** A configuration containing flags required for Apple platforms and tools. */ @@ -70,7 +73,7 @@ public class AppleConfiguration extends Fragment implements AppleConfigurationAp private final ImmutableList watchosCpus; private final ImmutableList tvosCpus; private final ImmutableList macosCpus; - private final AppleBitcodeMode bitcodeMode; + private final EnumMap platformBitcodeModes; private final Label xcodeConfigLabel; private final AppleCommandLineOptions options; @Nullable private final Label defaultProvisioningProfileLabel; @@ -95,7 +98,7 @@ private AppleConfiguration(AppleCommandLineOptions options, String iosCpu) { this.macosCpus = (options.macosCpus == null || options.macosCpus.isEmpty()) ? ImmutableList.of(AppleCommandLineOptions.DEFAULT_MACOS_CPU) : ImmutableList.copyOf(options.macosCpus); - this.bitcodeMode = options.appleBitcodeMode; + this.platformBitcodeModes = collectBitcodeModes(options.appleBitcodeMode); this.xcodeConfigLabel = Preconditions.checkNotNull(options.xcodeVersionConfig, "xcodeConfigLabel"); this.defaultProvisioningProfileLabel = options.defaultProvisioningProfile; @@ -357,11 +360,13 @@ public ImmutableList getIosMultiCpus() { */ @Override public AppleBitcodeMode getBitcodeMode() { - if (hasValidSingleArchPlatform() && getSingleArchPlatform().isDevice()) { - return bitcodeMode; - } else { - return AppleBitcodeMode.NONE; + if (hasValidSingleArchPlatform()) { + ApplePlatform platform = getSingleArchPlatform(); + if (platform.isDevice()) { + return platformBitcodeModes.get(applePlatformType); + } } + return AppleBitcodeMode.NONE; } /** @@ -433,6 +438,35 @@ static AppleConfiguration create(AppleCommandLineOptions appleOptions, String cp return new AppleConfiguration(appleOptions, iosCpuFromCpu(cpu)); } + /** + * Compute the platform-type-to-bitcode-mode mapping from the pairs that were passed on the + * command line. + */ + private static EnumMap collectBitcodeModes( + List> platformModeMappings) { + EnumMap modes = + new EnumMap<>(ApplePlatform.PlatformType.class); + ApplePlatform.PlatformType[] allPlatforms = ApplePlatform.PlatformType.values(); + + // Seed the map with the default mode for every key so that there is a valid mode for every + // platform. + // TODO(blaze-team): Default to embedded_markers when fully implemented. + Arrays.stream(allPlatforms).forEach(platform -> modes.put(platform, AppleBitcodeMode.NONE)); + + // Process the entries in order. If we encounter one with a null key, apply the mode to all + // platforms; otherwise, apply it only to that specific platform. This ensures that the later + // options override the earlier options. + for (Map.Entry entry : platformModeMappings) { + if (entry.getKey() == null) { + Arrays.stream(allPlatforms).forEach(platform -> modes.put(platform, entry.getValue())); + } else { + modes.put(entry.getKey(), entry.getValue()); + } + } + + return modes; + } + /** * Loads {@link AppleConfiguration} from build options. */ diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java index 2be2ae11df763b..41fdcd24b8be90 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.baseArtifactNames; +import static com.google.devtools.build.lib.rules.apple.AppleBitcodeConverter.INVALID_APPLE_BITCODE_OPTION_FORMAT; import static com.google.devtools.build.lib.rules.objc.CompilationSupport.ABSOLUTE_INCLUDES_PATH_FORMAT; import static com.google.devtools.build.lib.rules.objc.CompilationSupport.BOTH_MODULE_NAME_AND_MODULE_MAP_SPECIFIED; import static com.google.devtools.build.lib.rules.objc.CompilationSupport.FILE_IN_SRCS_AND_HDRS_WARNING_FORMAT; @@ -677,6 +678,138 @@ public void testCompilationActionsWithBitcode_simulator() throws Exception { assertThat(compileActionA.getArguments()).doesNotContain("-fembed-bitcode-marker"); } + @Test + public void testCompilationActionsWithEmbeddedBitcodeForMultiplePlatformsWithMatch() + throws Exception { + useConfiguration( + "--apple_platform_type=ios", + "--ios_multi_cpus=arm64", + "--apple_bitcode=ios=embedded", + "--apple_bitcode=watchos=embedded"); + createLibraryTargetWriter("//objc:lib") + .setAndCreateFiles("srcs", "a.m", "b.m", "private.h") + .setAndCreateFiles("hdrs", "c.h") + .write(); + + CommandAction compileActionA = compileAction("//objc:lib", "a.o"); + + assertThat(compileActionA.getArguments()).contains("-fembed-bitcode"); + } + + @Test + public void testCompilationActionsWithEmbeddedBitcodeForMultiplePlatformsWithoutMatch() + throws Exception { + useConfiguration( + "--apple_platform_type=ios", + "--ios_multi_cpus=arm64", + "--apple_bitcode=tvos=embedded", + "--apple_bitcode=watchos=embedded"); + createLibraryTargetWriter("//objc:lib") + .setAndCreateFiles("srcs", "a.m", "b.m", "private.h") + .setAndCreateFiles("hdrs", "c.h") + .write(); + + CommandAction compileActionA = compileAction("//objc:lib", "a.o"); + + assertThat(compileActionA.getArguments()).doesNotContain("-fembed-bitcode"); + assertThat(compileActionA.getArguments()).doesNotContain("-fembed-bitcode-marker"); + } + + @Test + public void testLaterBitcodeOptionsOverrideEarlierOptionsForSamePlatform() throws Exception { + useConfiguration( + "--apple_platform_type=ios", + "--ios_multi_cpus=arm64", + "--apple_bitcode=ios=embedded", + "--apple_bitcode=ios=embedded_markers"); + createLibraryTargetWriter("//objc:lib") + .setAndCreateFiles("srcs", "a.m", "b.m", "private.h") + .setAndCreateFiles("hdrs", "c.h") + .write(); + + CommandAction compileActionA = compileAction("//objc:lib", "a.o"); + + assertThat(compileActionA.getArguments()).doesNotContain("-fembed-bitcode"); + assertThat(compileActionA.getArguments()).contains("-fembed-bitcode-marker"); + } + + @Test + public void testLaterPlatformBitcodeOptionWithPlatformOverridesEarlierOptionWithoutPlatform() + throws Exception { + useConfiguration( + "--apple_platform_type=ios", + "--ios_multi_cpus=arm64", + "--apple_bitcode=ios=embedded", + "--apple_bitcode=embedded_markers"); + createLibraryTargetWriter("//objc:lib") + .setAndCreateFiles("srcs", "a.m", "b.m", "private.h") + .setAndCreateFiles("hdrs", "c.h") + .write(); + + CommandAction compileActionA = compileAction("//objc:lib", "a.o"); + + assertThat(compileActionA.getArguments()).doesNotContain("-fembed-bitcode"); + assertThat(compileActionA.getArguments()).contains("-fembed-bitcode-marker"); + } + + @Test + public void testLaterBitcodeOptionWithoutPlatformOverridesEarlierOptionWithPlatform() + throws Exception { + useConfiguration( + "--apple_platform_type=ios", + "--ios_multi_cpus=arm64", + "--apple_bitcode=embedded", + "--apple_bitcode=ios=embedded_markers"); + createLibraryTargetWriter("//objc:lib") + .setAndCreateFiles("srcs", "a.m", "b.m", "private.h") + .setAndCreateFiles("hdrs", "c.h") + .write(); + + CommandAction compileActionA = compileAction("//objc:lib", "a.o"); + + assertThat(compileActionA.getArguments()).doesNotContain("-fembed-bitcode"); + assertThat(compileActionA.getArguments()).contains("-fembed-bitcode-marker"); + } + + @Test + public void testAppleBitcode_invalidPlatformNameGivesError() throws Exception { + checkBitcodeModeError( + "--apple_platform_type=ios", + "--ios_multi_cpus=arm64", + "--apple_bitcode=ios=embedded", + "--apple_bitcode=nachos=embedded"); + } + + @Test + public void testAppleBitcode_invalidBitcodeModeGivesError() throws Exception { + checkBitcodeModeError( + "--apple_platform_type=ios", "--ios_multi_cpus=arm64", "--apple_bitcode=indebted"); + } + + @Test + public void testAppleBitcode_invalidBitcodeModeWithPlatformGivesError() throws Exception { + checkBitcodeModeError( + "--apple_platform_type=ios", "--ios_multi_cpus=arm64", "--apple_bitcode=ios=indebted"); + } + + @Test + public void testAppleBitcode_emptyBitcodeModeGivesError() throws Exception { + checkBitcodeModeError( + "--apple_platform_type=ios", "--ios_multi_cpus=arm64", "--apple_bitcode=ios="); + } + + @Test + public void testAppleBitcode_emptyValueGivesError() throws Exception { + checkBitcodeModeError( + "--apple_platform_type=ios", "--ios_multi_cpus=arm64", "--apple_bitcode="); + } + + private void checkBitcodeModeError(String... args) throws Exception { + OptionsParsingException thrown = + assertThrows(OptionsParsingException.class, () -> useConfiguration(args)); + assertThat(thrown).hasMessageThat().contains(INVALID_APPLE_BITCODE_OPTION_FORMAT); + } + @Test public void testModuleNameAttributeChangesName() throws Exception { RULE_TYPE.scratchTarget(scratch, "module_name", "'foo'");