Skip to content

Commit

Permalink
Rollback the copt-changing functionality of the extension_safe attribute
Browse files Browse the repository at this point in the history
This change impacted build times of builds with libraries that needed to be built both extension-safe and non-extension-safe.
This also caused duplicate symbol issues in such builds.

The extension_safe attribute will now be a no-op, with subsequent removal to follow.

RELNOTES: The extension_safe attribute of apple_binary no longer validates transitive dependencies are compiled against extension_safe APIs.
PiperOrigin-RevId: 159142000
  • Loading branch information
c-parsons authored and meteorcloudy committed Jun 16, 2017
1 parent daa1777 commit 8effaa7
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -679,22 +679,14 @@ 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 non-extension {@code apple_binary} rule with "macos" platform_type. */
/** Distinguisher for {@code apple_binary} rule with "macos" platform_type. */
APPLEBIN_MACOS("applebin_macos"),
/** 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 extension {@code apple_binary} rule with "macos" platform_type. */
APPLEBIN_MACOS_EXT("applebin_macos_ext"),

/**
* Distinguisher for the apple crosstool configuration. We use "apl" for output directory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ the main() function.
.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.
This attribute is deprecated and will be removed soon. It currently has no effect.
"Extension-safe" link options may be added using the <code>linkopts</code> attribute.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr(EXTENSION_SAFE_ATTR_NAME, BOOLEAN).value(false)
.nonconfigurable("Determines the configuration transition on deps"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

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;
Expand Down Expand Up @@ -52,7 +51,6 @@ public class MultiArchSplitTransitionProvider implements SplitTransitionProvider
"Invalid version string \"%s\". Version must be of the form 'x.y' without alphabetic "
+ "characters, such as '4.3'.";

private static final String EXTENSION_COPT = "-fapplication-extension";
private static final ImmutableSet<PlatformType> SUPPORTED_PLATFORM_TYPES =
ImmutableSet.of(
PlatformType.IOS, PlatformType.WATCHOS, PlatformType.TVOS, PlatformType.MACOS);
Expand Down Expand Up @@ -122,8 +120,6 @@ public SplitTransition<?> apply(Rule fromRule) {
attrMapper.get(MultiArchPlatformRule.PLATFORM_TYPE_ATTR_NAME, STRING);
String minimumOsVersionString =
attrMapper.get(MultiArchPlatformRule.MINIMUM_OS_VERSION, STRING);
boolean isExtension = attrMapper.has(AppleBinaryRule.EXTENSION_SAFE_ATTR_NAME, BOOLEAN)
&& attrMapper.get(AppleBinaryRule.EXTENSION_SAFE_ATTR_NAME, BOOLEAN);
PlatformType platformType;
Optional<DottedVersion> minimumOsVersion;
try {
Expand All @@ -142,7 +138,7 @@ public SplitTransition<?> apply(Rule fromRule) {
minimumOsVersion = Optional.absent();
}

return new AppleBinaryTransition(platformType, minimumOsVersion, isExtension);
return new AppleBinaryTransition(platformType, minimumOsVersion);
}

/**
Expand All @@ -155,14 +151,11 @@ protected static class AppleBinaryTransition implements SplitTransition<BuildOpt
private final PlatformType platformType;
// TODO(b/37096178): This should be a mandatory attribute.
private final Optional<DottedVersion> minimumOsVersion;
private final boolean isExtension;

public AppleBinaryTransition(PlatformType platformType,
Optional<DottedVersion> minimumOsVersion,
boolean isExtension) {
Optional<DottedVersion> minimumOsVersion) {
this.platformType = platformType;
this.minimumOsVersion = minimumOsVersion;
this.isExtension = isExtension;
}

@Override
Expand All @@ -176,9 +169,7 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
if (cpus.isEmpty()) {
cpus = ImmutableList.of(buildOptions.get(AppleCommandLineOptions.class).iosCpu);
}
configurationDistinguisher = isExtension
? ConfigurationDistinguisher.APPLEBIN_IOS_EXT
: ConfigurationDistinguisher.APPLEBIN_IOS;
configurationDistinguisher = ConfigurationDistinguisher.APPLEBIN_IOS;
actualMinimumOsVersion = minimumOsVersion.isPresent() ? minimumOsVersion.get()
: buildOptions.get(AppleCommandLineOptions.class).iosMinimumOs;
break;
Expand All @@ -187,9 +178,7 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
if (cpus.isEmpty()) {
cpus = ImmutableList.of(AppleCommandLineOptions.DEFAULT_WATCHOS_CPU);
}
configurationDistinguisher = isExtension
? ConfigurationDistinguisher.APPLEBIN_WATCHOS_EXT
: ConfigurationDistinguisher.APPLEBIN_WATCHOS;
configurationDistinguisher = ConfigurationDistinguisher.APPLEBIN_WATCHOS;
actualMinimumOsVersion = minimumOsVersion.isPresent() ? minimumOsVersion.get()
: buildOptions.get(AppleCommandLineOptions.class).watchosMinimumOs;
break;
Expand All @@ -198,9 +187,7 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
if (cpus.isEmpty()) {
cpus = ImmutableList.of(AppleCommandLineOptions.DEFAULT_TVOS_CPU);
}
configurationDistinguisher = isExtension
? ConfigurationDistinguisher.APPLEBIN_TVOS_EXT
: ConfigurationDistinguisher.APPLEBIN_TVOS;
configurationDistinguisher = ConfigurationDistinguisher.APPLEBIN_TVOS;
actualMinimumOsVersion = minimumOsVersion.isPresent() ? minimumOsVersion.get()
: buildOptions.get(AppleCommandLineOptions.class).tvosMinimumOs;
break;
Expand All @@ -209,25 +196,17 @@ public final List<BuildOptions> split(BuildOptions buildOptions) {
if (cpus.isEmpty()) {
cpus = ImmutableList.of(AppleCommandLineOptions.DEFAULT_MACOS_CPU);
}
configurationDistinguisher = isExtension
? ConfigurationDistinguisher.APPLEBIN_MACOS_EXT
: ConfigurationDistinguisher.APPLEBIN_MACOS;
configurationDistinguisher = ConfigurationDistinguisher.APPLEBIN_MACOS;
actualMinimumOsVersion = minimumOsVersion.isPresent() ? minimumOsVersion.get()
: buildOptions.get(AppleCommandLineOptions.class).macosMinimumOs;
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;

AppleCommandLineOptions appleCommandLineOptions =
splitOptions.get(AppleCommandLineOptions.class);
Expand Down

0 comments on commit 8effaa7

Please sign in to comment.