Skip to content

Commit 32d6c49

Browse files
author
Luca Di Grazia
committed
Refactor AppleCcToolchain to not pass ruleContext around too much
This is a preparation for moving variables away from toolchains to rules (because toolchains don't get analyzed in the same configuration as rules). bazelbuild/bazel#6516 RELNOTES: None PiperOrigin-RevId: 240759048
1 parent 9b12936 commit 32d6c49

File tree

7 files changed

+122
-65
lines changed

7 files changed

+122
-65
lines changed

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public static String sdkFrameworkDir(
106106
case IOS_SIMULATOR:
107107
if (xcodeConfig
108108
.getSdkVersionForPlatform(targetPlatform)
109-
.compareTo(DottedVersion.fromStringUnchecked("9.0"))
109+
.compareTo(DottedVersion.fromString("9.0"))
110110
>= 0) {
111111
relativePath = SYSTEM_FRAMEWORK_PATH;
112112
} else {

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@
3737
* Implementation for the {@code xcode_config} rule.
3838
*/
3939
public class XcodeConfig implements RuleConfiguredTargetFactory {
40-
private static final DottedVersion MINIMUM_BITCODE_XCODE_VERSION =
41-
DottedVersion.fromStringUnchecked("7");
40+
private static final DottedVersion MINIMUM_BITCODE_XCODE_VERSION = DottedVersion.fromString("7");
4241

4342
/**
4443
* An exception that signals that an Xcode config setup was invalid.
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2015 The Bazel Authors. All rights reserved.
1+
// Copyright 2017 The Bazel Authors. All rights reserved.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -11,43 +11,115 @@
1111
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
14-
1514
package com.google.devtools.build.lib.rules.apple;
1615

17-
import com.google.common.base.Optional;
18-
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
16+
import com.google.common.base.Preconditions;
17+
import com.google.devtools.build.lib.analysis.RuleContext;
18+
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
1919
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
20+
import com.google.devtools.build.lib.packages.NativeInfo;
21+
import com.google.devtools.build.lib.packages.NativeProvider;
22+
import com.google.devtools.build.lib.skylarkbuildapi.apple.XcodeConfigProviderApi;
23+
import javax.annotation.Nullable;
2024

2125
/**
22-
* Provides a version of xcode based on a combination of the {@code --xcode_version} build flag
23-
* and a {@code xcode_config} target. This version of xcode should be used for selecting apple
24-
* toolchains and SDKs.
26+
* The set of Apple versions computed from command line options and the {@code xcode_config} rule.
2527
*/
2628
@Immutable
27-
public final class XcodeConfigProvider implements TransitiveInfoProvider {
28-
private final Optional<DottedVersion> xcodeVersion;
29-
30-
XcodeConfigProvider(DottedVersion xcodeVersion) {
31-
this.xcodeVersion = Optional.of(xcodeVersion);
29+
public class XcodeConfigProvider extends NativeInfo
30+
implements XcodeConfigProviderApi<ApplePlatform, ApplePlatform.PlatformType> {
31+
/** Skylark name for this provider. */
32+
public static final String SKYLARK_NAME = "XcodeVersionConfig";
33+
34+
/** Provider identifier for {@link XcodeConfigProvider}. */
35+
public static final NativeProvider<XcodeConfigProvider> PROVIDER =
36+
new NativeProvider<XcodeConfigProvider>(XcodeConfigProvider.class, SKYLARK_NAME) {};
37+
38+
private final DottedVersion iosSdkVersion;
39+
private final DottedVersion iosMinimumOsVersion;
40+
private final DottedVersion watchosSdkVersion;
41+
private final DottedVersion watchosMinimumOsVersion;
42+
private final DottedVersion tvosSdkVersion;
43+
private final DottedVersion tvosMinimumOsVersion;
44+
private final DottedVersion macosSdkVersion;
45+
private final DottedVersion macosMinimumOsVersion;
46+
@Nullable private final DottedVersion xcodeVersion;
47+
48+
public XcodeConfigProvider(
49+
DottedVersion iosSdkVersion, DottedVersion iosMinimumOsVersion,
50+
DottedVersion watchosSdkVersion, DottedVersion watchosMinimumOsVersion,
51+
DottedVersion tvosSdkVersion, DottedVersion tvosMinimumOsVersion,
52+
DottedVersion macosSdkVersion, DottedVersion macosMinimumOsVersion,
53+
DottedVersion xcodeVersion) {
54+
super(PROVIDER);
55+
this.iosSdkVersion = Preconditions.checkNotNull(iosSdkVersion);
56+
this.iosMinimumOsVersion = Preconditions.checkNotNull(iosMinimumOsVersion);
57+
this.watchosSdkVersion = Preconditions.checkNotNull(watchosSdkVersion);
58+
this.watchosMinimumOsVersion = Preconditions.checkNotNull(watchosMinimumOsVersion);
59+
this.tvosSdkVersion = Preconditions.checkNotNull(tvosSdkVersion);
60+
this.tvosMinimumOsVersion = Preconditions.checkNotNull(tvosMinimumOsVersion);
61+
this.macosSdkVersion = Preconditions.checkNotNull(macosSdkVersion);
62+
this.macosMinimumOsVersion = Preconditions.checkNotNull(macosMinimumOsVersion);
63+
this.xcodeVersion = xcodeVersion;
3264
}
33-
34-
private XcodeConfigProvider() {
35-
this.xcodeVersion = Optional.absent();
65+
66+
/**
67+
* Returns the value of the xcode version, if available. This is determined based on a combination
68+
* of the {@code --xcode_version} build flag and the {@code xcode_config} target defined in the
69+
* {@code --xcode_version_config} flag. Returns null if no xcode is available.
70+
*/
71+
@Override
72+
public DottedVersion getXcodeVersion() {
73+
return xcodeVersion;
3674
}
37-
75+
3876
/**
39-
* Returns a {@link XcodeConfigProvider} with no xcode version specified. The host system
40-
* default xcode should be used. See {@link #getXcodeVersion}.
77+
* Returns the minimum compatible OS version for target simulator and devices for a particular
78+
* platform type.
4179
*/
42-
static XcodeConfigProvider hostSystemDefault() {
43-
return new XcodeConfigProvider();
80+
@Override
81+
public DottedVersion getMinimumOsForPlatformType(ApplePlatform.PlatformType platformType) {
82+
// TODO(b/37240784): Look into using only a single minimum OS flag tied to the current
83+
// apple_platform_type.
84+
switch (platformType) {
85+
case IOS:
86+
return iosMinimumOsVersion;
87+
case TVOS:
88+
return tvosMinimumOsVersion;
89+
case WATCHOS:
90+
return watchosMinimumOsVersion;
91+
case MACOS:
92+
return macosMinimumOsVersion;
93+
default:
94+
throw new IllegalArgumentException("Unhandled platform type: " + platformType);
95+
}
4496
}
4597

4698
/**
47-
* Returns either an explicit xcode version which should be used in actions which require an
48-
* apple toolchain, or {@link Optional#absent} if the host system default should be used.
99+
* Returns the SDK version for a platform (whether they be for simulator or device). This is
100+
* directly derived from command line args.
49101
*/
50-
public Optional<DottedVersion> getXcodeVersion() {
51-
return xcodeVersion;
102+
@Override
103+
public DottedVersion getSdkVersionForPlatform(ApplePlatform platform) {
104+
switch (platform) {
105+
case IOS_DEVICE:
106+
case IOS_SIMULATOR:
107+
return iosSdkVersion;
108+
case TVOS_DEVICE:
109+
case TVOS_SIMULATOR:
110+
return tvosSdkVersion;
111+
case WATCHOS_DEVICE:
112+
case WATCHOS_SIMULATOR:
113+
return watchosSdkVersion;
114+
case MACOS:
115+
return macosSdkVersion;
116+
default:
117+
throw new IllegalArgumentException("Unhandled platform: " + platform);
118+
}
119+
}
120+
121+
public static XcodeConfigProvider fromRuleContext(RuleContext ruleContext) {
122+
return ruleContext.getPrerequisite(
123+
XcodeConfigRule.XCODE_CONFIG_ATTR_NAME, Mode.TARGET, XcodeConfigProvider.PROVIDER);
52124
}
53125
}

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java

Lines changed: 13 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,13 @@
1616
import com.google.common.annotations.VisibleForTesting;
1717
import com.google.common.collect.ImmutableMap;
1818
import com.google.devtools.build.lib.analysis.RuleContext;
19-
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options;
20-
import com.google.devtools.build.lib.analysis.config.BuildOptions;
2119
import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
2220
import com.google.devtools.build.lib.rules.apple.ApplePlatform;
2321
import com.google.devtools.build.lib.rules.apple.AppleToolchain;
2422
import com.google.devtools.build.lib.rules.apple.XcodeConfig;
2523
import com.google.devtools.build.lib.rules.apple.XcodeConfigProvider;
2624
import com.google.devtools.build.lib.rules.cpp.CcToolchain;
2725
import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables;
28-
import java.io.Serializable;
2926
import java.util.LinkedHashMap;
3027
import java.util.Map;
3128

@@ -54,27 +51,18 @@ public class AppleCcToolchain extends CcToolchain {
5451
public static final String APPLE_SDK_PLATFORM_VALUE_KEY = "apple_sdk_platform_value";
5552

5653
@Override
57-
protected AdditionalBuildVariablesComputer getAdditionalBuildVariablesComputer(
58-
RuleContext ruleContextPossiblyInHostConfiguration) {
59-
// xcode config is shared between target and host configuration therefore we can use it.
60-
XcodeConfigProvider xcodeConfig =
61-
XcodeConfig.getXcodeConfigProvider(ruleContextPossiblyInHostConfiguration);
62-
return getAdditionalBuildVariablesComputer(xcodeConfig);
63-
}
64-
65-
/** Returns {@link AdditionalBuildVariablesComputer} lambda without capturing instance state. */
66-
private static AdditionalBuildVariablesComputer getAdditionalBuildVariablesComputer(
67-
XcodeConfigProvider xcodeConfig) {
68-
return (AdditionalBuildVariablesComputer & Serializable)
69-
(BuildOptions buildOptions) -> computeCcToolchainVariables(xcodeConfig, buildOptions);
70-
}
54+
protected CcToolchainVariables getAdditionalBuildVariables(RuleContext ruleContext)
55+
throws RuleErrorException {
56+
ApplePlatform platform =
57+
ruleContext.getFragment(AppleConfiguration.class).getSingleArchPlatform();
58+
XcodeConfigProvider xcodeConfig = XcodeConfig.getXcodeConfigProvider(ruleContext);
59+
String cpu = ruleContext.getConfiguration().getCpu();
7160

72-
private static CcToolchainVariables computeCcToolchainVariables(
73-
XcodeConfigProvider xcodeConfig, BuildOptions buildOptions) {
74-
AppleConfiguration.Loader appleConfigurationLoader = new AppleConfiguration.Loader();
75-
AppleConfiguration appleConfiguration = appleConfigurationLoader.create(buildOptions);
76-
ApplePlatform platform = appleConfiguration.getSingleArchPlatform();
77-
String cpu = buildOptions.get(Options.class).cpu;
61+
if (xcodeConfig.getXcodeVersion() == null) {
62+
ruleContext.throwWithRuleError(
63+
"Xcode version must be specified to use an Apple CROSSTOOL. If your Xcode version has "
64+
+ "changed recently, try: \"bazel clean --expunge\" to re-run Xcode configuration");
65+
}
7866

7967
Map<String, String> appleEnv = getEnvironmentBuildVariables(xcodeConfig, cpu);
8068
CcToolchainVariables.Builder variables = new CcToolchainVariables.Builder();
@@ -128,9 +116,8 @@ private static ImmutableMap<String, String> getEnvironmentBuildVariables(
128116
builder.putAll(AppleConfiguration.getXcodeVersionEnv(xcodeConfig.getXcodeVersion()));
129117
if (ApplePlatform.isApplePlatform(cpu)) {
130118
ApplePlatform platform = ApplePlatform.forTargetCpu(cpu);
131-
builder.putAll(
132-
AppleConfiguration.appleTargetPlatformEnv(
133-
platform, xcodeConfig.getSdkVersionForPlatform(platform)));
119+
builder.putAll(AppleConfiguration.appleTargetPlatformEnv(
120+
platform, xcodeConfig.getSdkVersionForPlatform(platform)));
134121
}
135122
return ImmutableMap.copyOf(builder);
136123
}
@@ -139,13 +126,4 @@ private static ImmutableMap<String, String> getEnvironmentBuildVariables(
139126
protected boolean isAppleToolchain() {
140127
return true;
141128
}
142-
143-
@Override
144-
protected void validateToolchain(RuleContext ruleContext) throws RuleErrorException {
145-
if (XcodeConfig.getXcodeConfigProvider(ruleContext).getXcodeVersion() == null) {
146-
ruleContext.throwWithRuleError(
147-
"Xcode version must be specified to use an Apple CROSSTOOL. If your Xcode version has "
148-
+ "changed recently, try: \"bazel clean --expunge\" to re-run Xcode configuration");
149-
}
150-
}
151129
}

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,13 @@ private FeatureConfiguration getFeatureConfiguration(
570570
if (objcProvider.is(Flag.USES_OBJC)) {
571571
activatedCrosstoolSelectables.add(CONTAINS_OBJC);
572572
}
573+
CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
574+
if (cppConfiguration.fissionIsActiveForCurrentCompilationMode()
575+
&& toolchain.supportsFission()
576+
&& !cppConfiguration.disableLegacyCrosstoolFields()) {
577+
activatedCrosstoolSelectables.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
578+
}
579+
573580
// Add a feature identifying the Xcode version so CROSSTOOL authors can enable flags for
574581
// particular versions of Xcode. To ensure consistency across platforms, use exactly two
575582
// components in the version number.
@@ -581,7 +588,6 @@ private FeatureConfiguration getFeatureConfiguration(
581588

582589
activatedCrosstoolSelectables.addAll(ruleContext.getFeatures());
583590

584-
CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class);
585591
activatedCrosstoolSelectables.addAll(CcCommon.getCoverageFeatures(cppConfiguration));
586592

587593
try {

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ protected void checkObjcCompileActions(
530530
throws Exception {
531531
CommandAction compileAction = getObjcCompileAction(archiveFile, objFileName);
532532
assertThat(Artifact.toRootRelativePaths(compileAction.getPossibleInputsForTesting()))
533-
.containsAtLeastElementsIn(compilationInputExecPaths);
533+
.containsAllIn(compilationInputExecPaths);
534534
}
535535

536536
protected CommandAction getObjcCompileAction(Artifact archiveFile, String objFileName)
@@ -951,7 +951,7 @@ public void testJ2ObjcLibraryDepThroughSkylarkRule() throws Exception {
951951
"examples/fake_rule.bzl",
952952
"def _fake_rule_impl(ctx):",
953953
" myProvider = ctx.attr.deps[0][JavaInfo]",
954-
" return myProvider",
954+
" return struct(providers = [myProvider])",
955955
"",
956956
"fake_rule = rule(",
957957
" implementation = _fake_rule_impl,",

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.devtools.build.lib.rules.cpp.CppLinkAction;
3333
import com.google.devtools.build.lib.rules.cpp.Link;
3434
import com.google.devtools.build.lib.rules.cpp.LinkBuildVariablesTestCase;
35+
import com.google.devtools.build.lib.testutil.TestConstants;
3536
import java.io.IOException;
3637
import org.junit.Before;
3738
import org.junit.Test;
@@ -63,6 +64,7 @@ public void initializeMockClient() throws IOException {
6364
@Override
6465
protected void useConfiguration(String... args) throws Exception {
6566
ImmutableList<String> extraArgs = ImmutableList.<String>builder()
67+
.addAll(TestConstants.OSX_CROSSTOOL_FLAGS)
6668
.add("--xcode_version_config=" + MockObjcSupport.XCODE_VERSION_CONFIG)
6769
.add("--apple_crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)
6870
.add("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)

0 commit comments

Comments
 (0)