From 32d6c49357aaf151b5d67570196b04927696a79e Mon Sep 17 00:00:00 2001 From: Luca Di Grazia Date: Sun, 4 Sep 2022 20:09:44 +0200 Subject: [PATCH] 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). https://github.com/bazelbuild/bazel/issues/6516 RELNOTES: None PiperOrigin-RevId: 240759048 --- .../build/lib/rules/apple/AppleToolchain.java | 2 +- .../build/lib/rules/apple/XcodeConfig.java | 3 +- .../lib/rules/apple/XcodeConfigProvider.java | 120 ++++++++++++++---- .../lib/rules/apple/cpp/AppleCcToolchain.java | 48 ++----- .../lib/rules/objc/CompilationSupport.java | 8 +- .../rules/objc/BazelJ2ObjcLibraryTest.java | 4 +- .../rules/objc/ObjcBuildVariablesTest.java | 2 + 7 files changed, 122 insertions(+), 65 deletions(-) diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java index 99a47c79664..a19fa351990 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/AppleToolchain.java @@ -106,7 +106,7 @@ public static String sdkFrameworkDir( case IOS_SIMULATOR: if (xcodeConfig .getSdkVersionForPlatform(targetPlatform) - .compareTo(DottedVersion.fromStringUnchecked("9.0")) + .compareTo(DottedVersion.fromString("9.0")) >= 0) { relativePath = SYSTEM_FRAMEWORK_PATH; } else { diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java index 19a2a99e25a..445db9b1c0b 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java @@ -37,8 +37,7 @@ * Implementation for the {@code xcode_config} rule. */ public class XcodeConfig implements RuleConfiguredTargetFactory { - private static final DottedVersion MINIMUM_BITCODE_XCODE_VERSION = - DottedVersion.fromStringUnchecked("7"); + private static final DottedVersion MINIMUM_BITCODE_XCODE_VERSION = DottedVersion.fromString("7"); /** * An exception that signals that an Xcode config setup was invalid. diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfigProvider.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfigProvider.java index 5241e048679..2607806d09d 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfigProvider.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfigProvider.java @@ -1,4 +1,4 @@ -// Copyright 2015 The Bazel Authors. All rights reserved. +// Copyright 2017 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. @@ -11,43 +11,115 @@ // 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.base.Optional; -import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; +import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.NativeInfo; +import com.google.devtools.build.lib.packages.NativeProvider; +import com.google.devtools.build.lib.skylarkbuildapi.apple.XcodeConfigProviderApi; +import javax.annotation.Nullable; /** - * Provides a version of xcode based on a combination of the {@code --xcode_version} build flag - * and a {@code xcode_config} target. This version of xcode should be used for selecting apple - * toolchains and SDKs. + * The set of Apple versions computed from command line options and the {@code xcode_config} rule. */ @Immutable -public final class XcodeConfigProvider implements TransitiveInfoProvider { - private final Optional xcodeVersion; - - XcodeConfigProvider(DottedVersion xcodeVersion) { - this.xcodeVersion = Optional.of(xcodeVersion); +public class XcodeConfigProvider extends NativeInfo + implements XcodeConfigProviderApi { + /** Skylark name for this provider. */ + public static final String SKYLARK_NAME = "XcodeVersionConfig"; + + /** Provider identifier for {@link XcodeConfigProvider}. */ + public static final NativeProvider PROVIDER = + new NativeProvider(XcodeConfigProvider.class, SKYLARK_NAME) {}; + + private final DottedVersion iosSdkVersion; + private final DottedVersion iosMinimumOsVersion; + private final DottedVersion watchosSdkVersion; + private final DottedVersion watchosMinimumOsVersion; + private final DottedVersion tvosSdkVersion; + private final DottedVersion tvosMinimumOsVersion; + private final DottedVersion macosSdkVersion; + private final DottedVersion macosMinimumOsVersion; + @Nullable private final DottedVersion xcodeVersion; + + public XcodeConfigProvider( + DottedVersion iosSdkVersion, DottedVersion iosMinimumOsVersion, + DottedVersion watchosSdkVersion, DottedVersion watchosMinimumOsVersion, + DottedVersion tvosSdkVersion, DottedVersion tvosMinimumOsVersion, + DottedVersion macosSdkVersion, DottedVersion macosMinimumOsVersion, + DottedVersion xcodeVersion) { + super(PROVIDER); + this.iosSdkVersion = Preconditions.checkNotNull(iosSdkVersion); + this.iosMinimumOsVersion = Preconditions.checkNotNull(iosMinimumOsVersion); + this.watchosSdkVersion = Preconditions.checkNotNull(watchosSdkVersion); + this.watchosMinimumOsVersion = Preconditions.checkNotNull(watchosMinimumOsVersion); + this.tvosSdkVersion = Preconditions.checkNotNull(tvosSdkVersion); + this.tvosMinimumOsVersion = Preconditions.checkNotNull(tvosMinimumOsVersion); + this.macosSdkVersion = Preconditions.checkNotNull(macosSdkVersion); + this.macosMinimumOsVersion = Preconditions.checkNotNull(macosMinimumOsVersion); + this.xcodeVersion = xcodeVersion; } - - private XcodeConfigProvider() { - this.xcodeVersion = Optional.absent(); + + /** + * Returns the value of the xcode version, if available. This is determined based on a combination + * of the {@code --xcode_version} build flag and the {@code xcode_config} target defined in the + * {@code --xcode_version_config} flag. Returns null if no xcode is available. + */ + @Override + public DottedVersion getXcodeVersion() { + return xcodeVersion; } - + /** - * Returns a {@link XcodeConfigProvider} with no xcode version specified. The host system - * default xcode should be used. See {@link #getXcodeVersion}. + * Returns the minimum compatible OS version for target simulator and devices for a particular + * platform type. */ - static XcodeConfigProvider hostSystemDefault() { - return new XcodeConfigProvider(); + @Override + public DottedVersion getMinimumOsForPlatformType(ApplePlatform.PlatformType platformType) { + // TODO(b/37240784): Look into using only a single minimum OS flag tied to the current + // apple_platform_type. + switch (platformType) { + case IOS: + return iosMinimumOsVersion; + case TVOS: + return tvosMinimumOsVersion; + case WATCHOS: + return watchosMinimumOsVersion; + case MACOS: + return macosMinimumOsVersion; + default: + throw new IllegalArgumentException("Unhandled platform type: " + platformType); + } } /** - * Returns either an explicit xcode version which should be used in actions which require an - * apple toolchain, or {@link Optional#absent} if the host system default should be used. + * Returns the SDK version for a platform (whether they be for simulator or device). This is + * directly derived from command line args. */ - public Optional getXcodeVersion() { - return xcodeVersion; + @Override + public DottedVersion getSdkVersionForPlatform(ApplePlatform platform) { + switch (platform) { + case IOS_DEVICE: + case IOS_SIMULATOR: + return iosSdkVersion; + case TVOS_DEVICE: + case TVOS_SIMULATOR: + return tvosSdkVersion; + case WATCHOS_DEVICE: + case WATCHOS_SIMULATOR: + return watchosSdkVersion; + case MACOS: + return macosSdkVersion; + default: + throw new IllegalArgumentException("Unhandled platform: " + platform); + } + } + + public static XcodeConfigProvider fromRuleContext(RuleContext ruleContext) { + return ruleContext.getPrerequisite( + XcodeConfigRule.XCODE_CONFIG_ATTR_NAME, Mode.TARGET, XcodeConfigProvider.PROVIDER); } } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java index 1c601a04d1d..85f36151d9d 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java @@ -16,8 +16,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options; -import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.rules.apple.AppleConfiguration; import com.google.devtools.build.lib.rules.apple.ApplePlatform; import com.google.devtools.build.lib.rules.apple.AppleToolchain; @@ -25,7 +23,6 @@ import com.google.devtools.build.lib.rules.apple.XcodeConfigProvider; import com.google.devtools.build.lib.rules.cpp.CcToolchain; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables; -import java.io.Serializable; import java.util.LinkedHashMap; import java.util.Map; @@ -54,27 +51,18 @@ public class AppleCcToolchain extends CcToolchain { public static final String APPLE_SDK_PLATFORM_VALUE_KEY = "apple_sdk_platform_value"; @Override - protected AdditionalBuildVariablesComputer getAdditionalBuildVariablesComputer( - RuleContext ruleContextPossiblyInHostConfiguration) { - // xcode config is shared between target and host configuration therefore we can use it. - XcodeConfigProvider xcodeConfig = - XcodeConfig.getXcodeConfigProvider(ruleContextPossiblyInHostConfiguration); - return getAdditionalBuildVariablesComputer(xcodeConfig); - } - - /** Returns {@link AdditionalBuildVariablesComputer} lambda without capturing instance state. */ - private static AdditionalBuildVariablesComputer getAdditionalBuildVariablesComputer( - XcodeConfigProvider xcodeConfig) { - return (AdditionalBuildVariablesComputer & Serializable) - (BuildOptions buildOptions) -> computeCcToolchainVariables(xcodeConfig, buildOptions); - } + protected CcToolchainVariables getAdditionalBuildVariables(RuleContext ruleContext) + throws RuleErrorException { + ApplePlatform platform = + ruleContext.getFragment(AppleConfiguration.class).getSingleArchPlatform(); + XcodeConfigProvider xcodeConfig = XcodeConfig.getXcodeConfigProvider(ruleContext); + String cpu = ruleContext.getConfiguration().getCpu(); - private static CcToolchainVariables computeCcToolchainVariables( - XcodeConfigProvider xcodeConfig, BuildOptions buildOptions) { - AppleConfiguration.Loader appleConfigurationLoader = new AppleConfiguration.Loader(); - AppleConfiguration appleConfiguration = appleConfigurationLoader.create(buildOptions); - ApplePlatform platform = appleConfiguration.getSingleArchPlatform(); - String cpu = buildOptions.get(Options.class).cpu; + if (xcodeConfig.getXcodeVersion() == null) { + ruleContext.throwWithRuleError( + "Xcode version must be specified to use an Apple CROSSTOOL. If your Xcode version has " + + "changed recently, try: \"bazel clean --expunge\" to re-run Xcode configuration"); + } Map appleEnv = getEnvironmentBuildVariables(xcodeConfig, cpu); CcToolchainVariables.Builder variables = new CcToolchainVariables.Builder(); @@ -128,9 +116,8 @@ private static ImmutableMap getEnvironmentBuildVariables( builder.putAll(AppleConfiguration.getXcodeVersionEnv(xcodeConfig.getXcodeVersion())); if (ApplePlatform.isApplePlatform(cpu)) { ApplePlatform platform = ApplePlatform.forTargetCpu(cpu); - builder.putAll( - AppleConfiguration.appleTargetPlatformEnv( - platform, xcodeConfig.getSdkVersionForPlatform(platform))); + builder.putAll(AppleConfiguration.appleTargetPlatformEnv( + platform, xcodeConfig.getSdkVersionForPlatform(platform))); } return ImmutableMap.copyOf(builder); } @@ -139,13 +126,4 @@ private static ImmutableMap getEnvironmentBuildVariables( protected boolean isAppleToolchain() { return true; } - - @Override - protected void validateToolchain(RuleContext ruleContext) throws RuleErrorException { - if (XcodeConfig.getXcodeConfigProvider(ruleContext).getXcodeVersion() == null) { - ruleContext.throwWithRuleError( - "Xcode version must be specified to use an Apple CROSSTOOL. If your Xcode version has " - + "changed recently, try: \"bazel clean --expunge\" to re-run Xcode configuration"); - } - } } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index 0ef892d1a39..30cb3f32c77 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java @@ -570,6 +570,13 @@ private FeatureConfiguration getFeatureConfiguration( if (objcProvider.is(Flag.USES_OBJC)) { activatedCrosstoolSelectables.add(CONTAINS_OBJC); } + CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); + if (cppConfiguration.fissionIsActiveForCurrentCompilationMode() + && toolchain.supportsFission() + && !cppConfiguration.disableLegacyCrosstoolFields()) { + activatedCrosstoolSelectables.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO); + } + // Add a feature identifying the Xcode version so CROSSTOOL authors can enable flags for // particular versions of Xcode. To ensure consistency across platforms, use exactly two // components in the version number. @@ -581,7 +588,6 @@ private FeatureConfiguration getFeatureConfiguration( activatedCrosstoolSelectables.addAll(ruleContext.getFeatures()); - CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); activatedCrosstoolSelectables.addAll(CcCommon.getCoverageFeatures(cppConfiguration)); try { diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java index 7bf46428483..a773056156f 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java @@ -530,7 +530,7 @@ protected void checkObjcCompileActions( throws Exception { CommandAction compileAction = getObjcCompileAction(archiveFile, objFileName); assertThat(Artifact.toRootRelativePaths(compileAction.getPossibleInputsForTesting())) - .containsAtLeastElementsIn(compilationInputExecPaths); + .containsAllIn(compilationInputExecPaths); } protected CommandAction getObjcCompileAction(Artifact archiveFile, String objFileName) @@ -951,7 +951,7 @@ public void testJ2ObjcLibraryDepThroughSkylarkRule() throws Exception { "examples/fake_rule.bzl", "def _fake_rule_impl(ctx):", " myProvider = ctx.attr.deps[0][JavaInfo]", - " return myProvider", + " return struct(providers = [myProvider])", "", "fake_rule = rule(", " implementation = _fake_rule_impl,", diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java index 92e033a48b5..f697bca93ba 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.rules.cpp.CppLinkAction; import com.google.devtools.build.lib.rules.cpp.Link; import com.google.devtools.build.lib.rules.cpp.LinkBuildVariablesTestCase; +import com.google.devtools.build.lib.testutil.TestConstants; import java.io.IOException; import org.junit.Before; import org.junit.Test; @@ -63,6 +64,7 @@ public void initializeMockClient() throws IOException { @Override protected void useConfiguration(String... args) throws Exception { ImmutableList extraArgs = ImmutableList.builder() + .addAll(TestConstants.OSX_CROSSTOOL_FLAGS) .add("--xcode_version_config=" + MockObjcSupport.XCODE_VERSION_CONFIG) .add("--apple_crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL) .add("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)