Skip to content

Commit

Permalink
Refactor ios_split_cpu to be apple_split_cpu, in preparation for usin…
Browse files Browse the repository at this point in the history
…g it to specify platform mutli_cpu splits other than on ios_multi_cpu (such as a split on watchos)

No users should be manually specifying this flag, so this shouldn't break anyone...

--
MOS_MIGRATED_REVID=122037152
  • Loading branch information
c-parsons authored and aehlig committed May 11, 2016
1 parent 4caa04f commit 5d80fdd
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
import com.google.devtools.build.lib.rules.apple.Platform.PlatformType;
import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
import com.google.devtools.common.options.EnumConverter;
import com.google.devtools.common.options.Option;
Expand Down Expand Up @@ -92,6 +94,35 @@ public class AppleCommandLineOptions extends FragmentOptions {
help = "Specifies to target CPU of iOS compilation.")
public String iosCpu;

@Option(name = "apple_platform_type",
defaultValue = "IOS",
category = "undocumented",
converter = PlatformTypeConverter.class,
help =
"Don't set this value from the command line - it is derived from other flags and "
+ "configuration transitions derived from rule attributes")
public PlatformType applePlatformType;

@Option(name = "apple_split_cpu",
defaultValue = "",
category = "undocumented",
help =
"Don't set this value from the command line - it is derived from other flags and "
+ "configuration transitions derived from rule attributes")
public String appleSplitCpu;

// This option exists because two configurations are not allowed to have the same cache key
// (partially derived from options). Since we have multiple transitions that may result in the
// same configuration values at runtime we need an artificial way to distinguish between them.
// This option must only be set by those transitions for this purpose.
// TODO(bazel-team): Remove this once we have dynamic configurations but make sure that different
// configurations (e.g. by min os version) always use different output paths.
@Option(name = "apple configuration distinguisher",
defaultValue = "UNKNOWN",
converter = ConfigurationDistinguisherConverter.class,
category = "undocumented")
public ConfigurationDistinguisher configurationDistinguisher;

@Option(name = "ios_multi_cpus",
converter = CommaSeparatedOptionListConverter.class,
defaultValue = "",
Expand Down Expand Up @@ -225,4 +256,20 @@ public FragmentOptions getHost(boolean fallback) {

return host;
}

/** Converter for the Apple configuration distinguisher. */
public static final class ConfigurationDistinguisherConverter
extends EnumConverter<ConfigurationDistinguisher> {
public ConfigurationDistinguisherConverter() {
super(ConfigurationDistinguisher.class, "Apple rule configuration distinguisher");
}
}

/** Flag converter for {@link PlatformType}. */
public static final class PlatformTypeConverter
extends EnumConverter<PlatformType> {
public PlatformTypeConverter() {
super(PlatformType.class, "Apple platform type");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.google.devtools.build.lib.rules.apple;

import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand All @@ -27,10 +28,14 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions.AppleBitcodeMode;
import com.google.devtools.build.lib.rules.apple.Platform.PlatformType;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.util.Preconditions;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Map;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -65,6 +70,9 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
private final DottedVersion tvOsSdkVersion;
private final DottedVersion macOsXSdkVersion;
private final String iosCpu;
private final String appleSplitCpu;
private final PlatformType applePlatformType;
private final ConfigurationDistinguisher configurationDistinguisher;
private final Optional<DottedVersion> xcodeVersion;
private final ImmutableList<String> iosMultiCpus;
private final AppleBitcodeMode bitcodeMode;
Expand All @@ -87,6 +95,10 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {

this.xcodeVersion = Preconditions.checkNotNull(xcodeVersionOverride);
this.iosCpu = Preconditions.checkNotNull(appleOptions.iosCpu, "iosCpu");
this.appleSplitCpu = Preconditions.checkNotNull(appleOptions.appleSplitCpu, "appleSplitCpu");
this.applePlatformType =
Preconditions.checkNotNull(appleOptions.applePlatformType, "applePlatformType");
this.configurationDistinguisher = appleOptions.configurationDistinguisher;
this.iosMultiCpus = ImmutableList.copyOf(
Preconditions.checkNotNull(appleOptions.iosMultiCpus, "iosMultiCpus"));
this.bitcodeMode = appleOptions.appleBitcodeMode;
Expand Down Expand Up @@ -290,6 +302,35 @@ public Label getXcodeConfigLabel() {
return xcodeConfigLabel;
}

/**
* Returns the unique identifier distinguishing configurations that are otherwise the same.
*
* <p>Use this value for situations in which two configurations create two outputs that are the
* same but are not collapsed due to their different configuration owners.
*/
public ConfigurationDistinguisher getConfigurationDistinguisher() {
return configurationDistinguisher;
}


@Nullable
@Override
public String getOutputDirectoryName() {
List<String> components = new ArrayList<>();
if (!appleSplitCpu.isEmpty()) {
components.add(applePlatformType.toString().toLowerCase());
components.add(appleSplitCpu);
}
if (configurationDistinguisher != ConfigurationDistinguisher.UNKNOWN) {
components.add(configurationDistinguisher.toString().toLowerCase(Locale.US));
}

if (components.isEmpty()) {
return null;
}
return Joiner.on('-').join(components);
}

/**
* Loads {@link AppleConfiguration} from build options.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,16 @@ public static Platform forTargetCpu(String targetCpu) {
public static boolean isApplePlatform(String targetCpu) {
return forTargetCpuNullable(targetCpu) != null;
}

/**
* Value used to describe Apple platform "type". A {@link Platform} is implied from a platform
* type (for example, watchOS) together with a cpu value (for example, armv7).
*/
// TODO(cparsons): Use these values in static retrieval methods in this class.
public enum PlatformType {
IOS,
WATCHOS,
TVOS,
MACOSX
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
import com.google.devtools.build.lib.rules.apple.Platform.PlatformType;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs;
import com.google.devtools.build.lib.rules.objc.ObjcCommon.CompilationAttributes;
Expand Down Expand Up @@ -182,7 +183,7 @@ private Set<BuildConfiguration> getChildConfigurations(RuleContext ruleContext)

return configToProvider.keySet();
}

/**
* {@link SplitTransitionProvider} implementation for the apple binary rule.
*/
Expand All @@ -196,7 +197,7 @@ public SplitTransition<?> apply(Rule fromRule) {
// TODO(cparsons): Support different split transitions based on rule attribute.
return IOS_MULTI_CPUS_SPLIT_TRANSITION;
}

public List<SplitTransition<BuildOptions>> getPotentialSplitTransitions() {
return ImmutableList.<SplitTransition<BuildOptions>>of(IOS_MULTI_CPUS_SPLIT_TRANSITION);
}
Expand All @@ -217,7 +218,8 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
BuildOptions splitOptions = buildOptions.clone();

splitOptions.get(AppleCommandLineOptions.class).iosMultiCpus = ImmutableList.of();
splitOptions.get(ObjcCommandLineOptions.class).iosSplitCpu = iosCpu;
splitOptions.get(AppleCommandLineOptions.class).applePlatformType = PlatformType.IOS;
splitOptions.get(AppleCommandLineOptions.class).appleSplitCpu = iosCpu;
splitOptions.get(AppleCommandLineOptions.class).iosCpu = iosCpu;
if (splitOptions.get(ObjcCommandLineOptions.class).enableCcDeps) {
// Only set the (CC-compilation) CPU for dependencies if explicitly required by the user.
Expand All @@ -226,7 +228,7 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
// set if this feature is explicitly requested.
splitOptions.get(BuildConfiguration.Options.class).cpu = "ios_" + iosCpu;
}
splitOptions.get(ObjcCommandLineOptions.class).configurationDistinguisher =
splitOptions.get(AppleCommandLineOptions.class).configurationDistinguisher =
ConfigurationDistinguisher.APPLEBIN_IOS;
splitBuildOptions.add(splitOptions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
import com.google.devtools.build.lib.rules.apple.DottedVersion;
import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition;
Expand Down Expand Up @@ -119,7 +120,7 @@ protected ImmutableList<BuildOptions> defaultOptions(BuildOptions originalOption

BuildOptions splitOptions = originalOptions.clone();
setMinimumOsVersion(splitOptions, newMinimumVersion);
splitOptions.get(ObjcCommandLineOptions.class).configurationDistinguisher =
splitOptions.get(AppleCommandLineOptions.class).configurationDistinguisher =
getConfigurationDistinguisher();
return ImmutableList.of(splitOptions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
import com.google.devtools.build.lib.rules.apple.DottedVersion;
import com.google.devtools.build.lib.rules.apple.DottedVersionConverter;
import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
import com.google.devtools.common.options.EnumConverter;
import com.google.devtools.common.options.Option;

import java.util.List;
Expand Down Expand Up @@ -91,13 +89,6 @@ public class ObjcCommandLineOptions extends FragmentOptions {
help = "Enable checking for memory leaks in ios_test targets.")
public boolean runMemleaks;

@Option(name = "ios_split_cpu",
defaultValue = "",
category = "undocumented",
help =
"Don't set this value from the command line - it is derived from ios_multi_cpus only.")
public String iosSplitCpu;

@Option(name = "experimental_enable_objc_cc_deps",
defaultValue = "false",
category = "undocumented",
Expand Down Expand Up @@ -139,19 +130,6 @@ public class ObjcCommandLineOptions extends FragmentOptions {
)
public boolean appleGenerateDsym;

// This option exists because two configurations are not allowed to have the same cache key
// (partially derived from options). Since we have multiple transitions (see
// getPotentialSplitTransitions below) that may result in the same configuration values at runtime
// we need an artificial way to distinguish between them. This option must only be set by those
// transitions for this purpose.
// TODO(bazel-team): Remove this once we have dynamic configurations but make sure that different
// configurations (e.g. by min os version) always use different output paths.
@Option(name = "iOS configuration distinguisher",
defaultValue = "UNKNOWN",
converter = ConfigurationDistinguisherConverter.class,
category = "undocumented")
public ConfigurationDistinguisher configurationDistinguisher;

@Option(
name = "ios_signing_cert_name",
defaultValue = "null",
Expand Down Expand Up @@ -243,12 +221,4 @@ public List<SplitTransition<BuildOptions>> getPotentialSplitTransitions() {
.addAll(AppleBinary.SPLIT_TRANSITION_PROVIDER.getPotentialSplitTransitions())
.build();
}

/** Converter for the iOS configuration distinguisher. */
public static final class ConfigurationDistinguisherConverter
extends EnumConverter<ConfigurationDistinguisher> {
public ConfigurationDistinguisherConverter() {
super(ConfigurationDistinguisher.class, "Objective C configuration distinguisher");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,18 @@
package com.google.devtools.build.lib.rules.objc;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.CompilationMode;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.rules.apple.AppleConfiguration.ConfigurationDistinguisher;
import com.google.devtools.build.lib.rules.apple.DottedVersion;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.Path;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;

import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -64,11 +58,9 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
private final boolean runMemleaks;
private final ImmutableList<String> copts;
private final CompilationMode compilationMode;
private final String iosSplitCpu;
private final ImmutableList<String> fastbuildOptions;
private final boolean enableBinaryStripping;
private final boolean moduleMapsEnabled;
private final ConfigurationDistinguisher configurationDistinguisher;
@Nullable private final String signingCertName;
@Nullable private final Path clientWorkspaceRoot;
private final String xcodeOverrideWorkspaceRoot;
Expand All @@ -92,11 +84,9 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
this.runMemleaks = objcOptions.runMemleaks;
this.copts = ImmutableList.copyOf(objcOptions.copts);
this.compilationMode = Preconditions.checkNotNull(options.compilationMode, "compilationMode");
this.iosSplitCpu = Preconditions.checkNotNull(objcOptions.iosSplitCpu, "iosSplitCpu");
this.fastbuildOptions = ImmutableList.copyOf(objcOptions.fastbuildOptions);
this.enableBinaryStripping = objcOptions.enableBinaryStripping;
this.moduleMapsEnabled = objcOptions.enableModuleMaps;
this.configurationDistinguisher = objcOptions.configurationDistinguisher;
this.clientWorkspaceRoot = directories != null ? directories.getWorkspace() : null;
this.signingCertName = objcOptions.iosSigningCertName;
this.xcodeOverrideWorkspaceRoot = objcOptions.xcodeOverrideWorkspaceRoot;
Expand Down Expand Up @@ -221,33 +211,6 @@ public boolean moduleMapsEnabled() {
return moduleMapsEnabled;
}

/**
* Returns the unique identifier distinguishing configurations that are otherwise the same.
*
* <p>Use this value for situations in which two configurations create two outputs that are the
* same but are not collapsed due to their different configuration owners.
*/
public ConfigurationDistinguisher getConfigurationDistinguisher() {
return configurationDistinguisher;
}

@Nullable
@Override
public String getOutputDirectoryName() {
List<String> components = new ArrayList<>();
if (!iosSplitCpu.isEmpty()) {
components.add("ios-" + iosSplitCpu);
}
if (configurationDistinguisher != ConfigurationDistinguisher.UNKNOWN) {
components.add(configurationDistinguisher.toString().toLowerCase(Locale.US));
}

if (components.isEmpty()) {
return null;
}
return Joiner.on('-').join(components);
}

/**
* Returns whether to perform symbol and dead-code strippings on linked binaries. The strippings
* are performed iff --compilation_mode=opt and --objc_enable_binary_stripping are specified.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import com.google.devtools.build.lib.rules.apple.AppleToolchain;
import com.google.devtools.build.lib.rules.apple.DottedVersion;
import com.google.devtools.build.lib.rules.apple.Platform;
import com.google.devtools.build.lib.rules.apple.Platform.PlatformType;
import com.google.devtools.build.lib.rules.objc.BundleSupport.ExtraActoolArgs;
import com.google.devtools.build.lib.rules.objc.Bundling.Builder;
import com.google.devtools.build.lib.shell.ShellUtils;
Expand Down Expand Up @@ -1333,7 +1334,7 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
BuildOptions splitOptions = buildOptions.clone();
setArchitectureOptions(splitOptions, iosCpu);
setAdditionalOptions(splitOptions, buildOptions);
splitOptions.get(ObjcCommandLineOptions.class).configurationDistinguisher =
splitOptions.get(AppleCommandLineOptions.class).configurationDistinguisher =
getConfigurationDistinguisher();
splitBuildOptions.add(splitOptions);
}
Expand All @@ -1360,7 +1361,8 @@ protected ImmutableList<BuildOptions> defaultOptions(BuildOptions originalOption
protected void setAdditionalOptions(BuildOptions splitOptions, BuildOptions originalOptions) {}

private void setArchitectureOptions(BuildOptions splitOptions, String iosCpu) {
splitOptions.get(ObjcCommandLineOptions.class).iosSplitCpu = iosCpu;
splitOptions.get(AppleCommandLineOptions.class).applePlatformType = PlatformType.IOS;
splitOptions.get(AppleCommandLineOptions.class).appleSplitCpu = iosCpu;
splitOptions.get(AppleCommandLineOptions.class).iosCpu = iosCpu;
if (splitOptions.get(ObjcCommandLineOptions.class).enableCcDeps) {
// Only set the (CC-compilation) CPU for dependencies if explicitly required by the user.
Expand Down
Loading

0 comments on commit 5d80fdd

Please sign in to comment.