Skip to content

Commit

Permalink
Rollback of commit 59b0195.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

Breaks Blaze nightly

*** Original change description ***

apple_binary extension_safe attribute results in configuration transition on dependencies

RELNOTES: apple_binary has a new "extension_safe" attribute to set extension compilation options on dependencies.

--
PiperOrigin-RevId: 148009147
MOS_MIGRATED_REVID=148009147
  • Loading branch information
@laurentlb @iirina
laurentlb authored and iirina committed Feb 20, 2017
1 parent 4d3bd44 commit df0a5f3
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 73 deletions.
12 changes: 3 additions & 9 deletions src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,18 +699,12 @@ public enum ConfigurationDistinguisher {
FRAMEWORK("framework"),
/** Split transition distinguisher for {@code apple_watch1_extension} rule. */
WATCH_OS1_EXTENSION("watch_os1_extension"),
/** Distinguisher for non-extension {@code apple_binary} rule with "ios" platform_type. */
/** Distinguisher for {@code apple_binary} rule with "ios" platform_type. */
APPLEBIN_IOS("applebin_ios"),
/** Distinguisher for non-extension {@code apple_binary} rule with "watchos" platform_type. */
/** Distinguisher for {@code apple_binary} rule with "watchos" platform_type. */
APPLEBIN_WATCHOS("applebin_watchos"),
/** Distinguisher for non-extension {@code apple_binary} rule with "tvos" platform_type. */
/** Distinguisher for {@code apple_binary} rule with "tvos" platform_type. */
APPLEBIN_TVOS("applebin_tvos"),
/** Distinguisher for extension {@code apple_binary} rule with "ios" platform_type. */
APPLEBIN_IOS_EXT("applebin_ios_ext"),
/** Distinguisher for extension {@code apple_binary} rule with "watchos" platform_type. */
APPLEBIN_WATCHOS_EXT("applebin_watchos_ext"),
/** Distinguisher for extension {@code apple_binary} rule with "tvos" platform_type. */
APPLEBIN_TVOS_EXT("applebin_tvos_ext"),
/**
* Distinguisher for the apple crosstool configuration. We use "apl" for output directory
* names instead of "apple_crosstool" to avoid oversized path names, which can be problematic
Expand Down
7 changes: 0 additions & 7 deletions src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinaryRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public class AppleBinaryRule implements RuleDefinition {

public static final String BINARY_TYPE_ATTR = "binary_type";
public static final String BUNDLE_LOADER_ATTR_NAME = "bundle_loader";
public static final String EXTENSION_SAFE_ATTR_NAME = "extension_safe";

private final ObjcProtoAspect objcProtoAspect;

Expand Down Expand Up @@ -119,12 +118,6 @@ the main() function.
attr(BINARY_TYPE_ATTR, STRING)
.value(AppleBinary.BinaryType.EXECUTABLE.toString())
.allowedValues(new AllowedValueSet(AppleBinary.BinaryType.getValues())))
/* <!-- #BLAZE_RULE(apple_binary).ATTRIBUTE(extension_safe) -->
Indicates whether this binary is for an extension. This will set certain compiler
options and restrictions on dependencies of this target.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr(EXTENSION_SAFE_ATTR_NAME, BOOLEAN).value(false)
.nonconfigurable("Determines the configuration transition on deps"))
.add(
attr(BUNDLE_LOADER_ATTR_NAME, LABEL)
.direct_compile_time_input()
Expand Down
73 changes: 16 additions & 57 deletions src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@

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

import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
import static com.google.devtools.build.lib.syntax.Type.STRING;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
Expand All @@ -34,7 +33,6 @@
import com.google.devtools.build.lib.rules.objc.ObjcRuleClasses.MultiArchPlatformRule;
import java.util.List;


/**
* {@link SplitTransitionProvider} implementation for multi-architecture apple rules which can
* accept different apple platform types (such as ios or watchos).
Expand All @@ -44,8 +42,7 @@ public class MultiArchSplitTransitionProvider implements SplitTransitionProvider
@VisibleForTesting
static final String UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT =
"Unsupported platform type \"%s\"";

private static final String EXTENSION_COPT = "-application-extension";

private static final ImmutableSet<PlatformType> SUPPORTED_PLATFORM_TYPES =
ImmutableSet.of(PlatformType.IOS, PlatformType.WATCHOS, PlatformType.TVOS);

Expand Down Expand Up @@ -83,23 +80,17 @@ public static PlatformType getPlatformType(String platformTypeString) {
}
}

private static final ImmutableSet<AppleBinaryTransition>
SPLIT_TRANSITIONS = ImmutableSet.of(
new AppleBinaryTransition(PlatformType.IOS, false),
new AppleBinaryTransition(PlatformType.WATCHOS, false),
new AppleBinaryTransition(PlatformType.TVOS, false),
new AppleBinaryTransition(PlatformType.IOS, true),
new AppleBinaryTransition(PlatformType.WATCHOS, true),
new AppleBinaryTransition(PlatformType.TVOS, true));
private static final ImmutableMap<PlatformType, AppleBinaryTransition>
SPLIT_TRANSITIONS_BY_TYPE = ImmutableMap.<PlatformType, AppleBinaryTransition>builder()
.put(PlatformType.IOS, new AppleBinaryTransition(PlatformType.IOS))
.put(PlatformType.WATCHOS, new AppleBinaryTransition(PlatformType.WATCHOS))
.put(PlatformType.TVOS, new AppleBinaryTransition(PlatformType.TVOS))
.build();

@Override
public SplitTransition<?> apply(Rule fromRule) {
NonconfigurableAttributeMapper attrMapper = NonconfigurableAttributeMapper.of(fromRule);
String platformTypeString =
attrMapper.get(MultiArchPlatformRule.PLATFORM_TYPE_ATTR_NAME, STRING);

boolean isExtension = attrMapper.has(AppleBinaryRule.EXTENSION_SAFE_ATTR_NAME, BOOLEAN)
&& attrMapper.get(AppleBinaryRule.EXTENSION_SAFE_ATTR_NAME, BOOLEAN);
String platformTypeString = NonconfigurableAttributeMapper.of(fromRule)
.get(MultiArchPlatformRule.PLATFORM_TYPE_ATTR_NAME, STRING);
PlatformType platformType;
try {
platformType = getPlatformType(platformTypeString);
Expand All @@ -110,7 +101,7 @@ public SplitTransition<?> apply(Rule fromRule) {
platformType = PlatformType.IOS;
}

return new AppleBinaryTransition(platformType, isExtension);
return SPLIT_TRANSITIONS_BY_TYPE.get(platformType);
}

/**
Expand All @@ -119,7 +110,7 @@ public SplitTransition<?> apply(Rule fromRule) {
*/
public static List<SplitTransition<BuildOptions>> getPotentialSplitTransitions() {
return ImmutableList.<SplitTransition<BuildOptions>>copyOf(
SPLIT_TRANSITIONS.asList());
SPLIT_TRANSITIONS_BY_TYPE.values());
}

/**
Expand All @@ -130,11 +121,9 @@ public static List<SplitTransition<BuildOptions>> getPotentialSplitTransitions()
protected static class AppleBinaryTransition implements SplitTransition<BuildOptions> {

private final PlatformType platformType;
private final boolean isExtension;

public AppleBinaryTransition(PlatformType platformType, boolean isExtension) {
public AppleBinaryTransition(PlatformType platformType) {
this.platformType = platformType;
this.isExtension = isExtension;
}

@Override
Expand All @@ -144,45 +133,30 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
switch (platformType) {
case IOS:
cpus = buildOptions.get(AppleCommandLineOptions.class).iosMultiCpus;
if (cpus.isEmpty()) {
cpus = ImmutableList.of(buildOptions.get(AppleCommandLineOptions.class).iosCpu);
}
configurationDistinguisher = isExtension
? ConfigurationDistinguisher.APPLEBIN_IOS_EXT
: ConfigurationDistinguisher.APPLEBIN_IOS;
configurationDistinguisher = ConfigurationDistinguisher.APPLEBIN_IOS;
break;
case WATCHOS:
cpus = buildOptions.get(AppleCommandLineOptions.class).watchosCpus;
if (cpus.isEmpty()) {
cpus = ImmutableList.of(AppleCommandLineOptions.DEFAULT_WATCHOS_CPU);
}
configurationDistinguisher = isExtension
? ConfigurationDistinguisher.APPLEBIN_WATCHOS_EXT
: ConfigurationDistinguisher.APPLEBIN_WATCHOS;
configurationDistinguisher = ConfigurationDistinguisher.APPLEBIN_WATCHOS;
break;
case TVOS:
cpus = buildOptions.get(AppleCommandLineOptions.class).tvosCpus;
if (cpus.isEmpty()) {
cpus = ImmutableList.of(AppleCommandLineOptions.DEFAULT_TVOS_CPU);
}
configurationDistinguisher = isExtension
? ConfigurationDistinguisher.APPLEBIN_TVOS_EXT
: ConfigurationDistinguisher.APPLEBIN_TVOS;
configurationDistinguisher = ConfigurationDistinguisher.APPLEBIN_TVOS;
break;
default:
throw new IllegalArgumentException("Unsupported platform type " + platformType);
}

List<String> copts = buildOptions.get(ObjcCommandLineOptions.class).copts;
if (isExtension && !copts.contains(EXTENSION_COPT)) {
copts = ImmutableList.<String>builder()
.addAll(copts).add(EXTENSION_COPT).build();
}
ImmutableList.Builder<BuildOptions> splitBuildOptions = ImmutableList.builder();
for (String cpu : cpus) {
BuildOptions splitOptions = buildOptions.clone();

splitOptions.get(ObjcCommandLineOptions.class).copts = copts;
splitOptions.get(AppleCommandLineOptions.class).applePlatformType = platformType;
splitOptions.get(AppleCommandLineOptions.class).appleSplitCpu = cpu;
// If the new configuration does not use the apple crosstool, then it needs ios_cpu to be
Expand Down Expand Up @@ -210,20 +184,5 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
public boolean defaultsToSelf() {
return true;
}

@Override
public int hashCode() {
return Objects.hashCode(platformType, isExtension);
}

@Override
public boolean equals(Object other) {
if (!(other instanceof AppleBinaryTransition)) {
return false;
}
AppleBinaryTransition that = (AppleBinaryTransition) other;
return Objects.equal(platformType, that.platformType)
&& Objects.equal(isExtension, that.isExtension);
}
}
}

0 comments on commit df0a5f3

Please sign in to comment.