From a76763c4125fbbea17f415a40d9efca846b0b6f0 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 2 Aug 2023 09:41:48 -0700 Subject: [PATCH] Remove apple_common.multi_arch_split Remove the apple_binary and apple_static_library tests. The tests involved hardcoded Starlark implementation of apple_binary that diverged from the implementation that is used. Add a simple hardcoded implementation of the apple_platform_split_transition to tests. This keep the coverage of objc rules that are in builtins. (Also the implementations of multi_arch_split has diverged from apple_platform_split_transition). RELNOTES[INC]: Removed multi_arch_split, use transition_support.apple_platform_split_transition from rules_apple instead. https://github.com/bazelbuild/rules_apple/blob/master/apple/internal/transition_support.bzl#L608 PiperOrigin-RevId: 553171946 Change-Id: I874e8b9e39b291774a74b0192926521a3c88c103 --- .../lib/rules/objc/AppleStarlarkCommon.java | 6 - .../MultiArchSplitTransitionProvider.java | 206 --- .../starlarkbuildapi/objc/AppleCommonApi.java | 22 - .../objc/AppleBinaryStarlarkApiTest.java | 1261 ----------------- .../rules/objc/AppleDynamicLibraryTest.java | 221 --- .../rules/objc/AppleStaticLibraryTest.java | 648 --------- .../rules/objc/BazelJ2ObjcLibraryTest.java | 53 +- .../rules/objc/ObjcBuildVariablesTest.java | 125 -- .../lib/rules/objc/ObjcRuleTestCase.java | 79 +- .../lib/rules/objc/ObjcStarlarkTest.java | 100 +- ...arlarkRuleImplementationFunctionsTest.java | 2 +- src/test/shell/bazel/apple/BUILD | 8 - src/test/shell/bazel/apple/apple_common.sh | 193 --- .../shell/bazel/apple/bazel_apple_test.sh | 3 - src/test/shell/bazel/apple/bazel_objc_test.sh | 3 - 15 files changed, 98 insertions(+), 2832 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java delete mode 100644 src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryStarlarkApiTest.java delete mode 100644 src/test/java/com/google/devtools/build/lib/rules/objc/AppleDynamicLibraryTest.java delete mode 100644 src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java delete mode 100644 src/test/shell/bazel/apple/apple_common.sh diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java index a149f17500ca80..9a7ed73edbc200 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStarlarkCommon.java @@ -44,7 +44,6 @@ import com.google.devtools.build.lib.rules.cpp.CppSemantics; import com.google.devtools.build.lib.rules.cpp.UserVariablesExtension; import com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag; -import com.google.devtools.build.lib.starlarkbuildapi.SplitTransitionProviderApi; import com.google.devtools.build.lib.starlarkbuildapi.objc.AppleCommonApi; import java.util.Map; import javax.annotation.Nullable; @@ -169,11 +168,6 @@ public ImmutableMap getTargetAppleEnvironment( platform, xcodeConfig.getSdkVersionForPlatform(platform)); } - @Override - public SplitTransitionProviderApi getMultiArchSplitProvider() { - return new MultiArchSplitTransitionProvider(); - } - @Override // This method is registered statically for Starlark, and never called directly. public ObjcProvider newObjcProvider(Dict kwargs, StarlarkThread thread) diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java deleted file mode 100644 index 8e538d43cb3894..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java +++ /dev/null @@ -1,206 +0,0 @@ -// Copyright 2016 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. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// 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.objc; - -import static com.google.devtools.build.lib.packages.Type.STRING; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Optional; -import com.google.common.base.Strings; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.analysis.PlatformOptions; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.BuildOptionsView; -import com.google.devtools.build.lib.analysis.config.CoreOptions; -import com.google.devtools.build.lib.analysis.config.FragmentOptions; -import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition; -import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.events.EventHandler; -import com.google.devtools.build.lib.packages.AttributeTransitionData; -import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions; -import com.google.devtools.build.lib.rules.apple.ApplePlatform; -import com.google.devtools.build.lib.rules.apple.ApplePlatform.PlatformType; -import com.google.devtools.build.lib.rules.apple.DottedVersion; -import com.google.devtools.build.lib.rules.cpp.CppOptions; -import com.google.devtools.build.lib.starlarkbuildapi.SplitTransitionProviderApi; -import java.util.List; -import java.util.Map; -import net.starlark.java.eval.Printer; -import net.starlark.java.eval.StarlarkValue; - -/** - * {@link TransitionFactory} implementation for multi-architecture apple rules which can accept - * different apple platform types (such as ios or watchos). - */ -// TODO(https://github.com/bazelbuild/bazel/pull/7825): Rename to MultiArchSplitTransitionFactory. -public class MultiArchSplitTransitionProvider - implements TransitionFactory, - SplitTransitionProviderApi, - StarlarkValue { - - @VisibleForTesting - static final String UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT = - "Unsupported platform type \"%s\""; - - @VisibleForTesting - static final String INVALID_VERSION_STRING_ERROR_FORMAT = - "Invalid version string \"%s\". Version must be of the form 'x.y' without alphabetic " - + "characters, such as '4.3'."; - - private static final ImmutableSet SUPPORTED_PLATFORM_TYPES = - ImmutableSet.of( - PlatformType.IOS, - PlatformType.WATCHOS, - PlatformType.TVOS, - PlatformType.MACOS, - PlatformType.CATALYST); - - /** - * Returns the apple platform type in the current rule context. - * - * @throws RuleErrorException if the platform type attribute in the current rulecontext is - * an invalid value - */ - public static PlatformType getPlatformType(RuleContext ruleContext) throws RuleErrorException { - String attributeValue = - ruleContext.attributes().get(ObjcRuleClasses.PLATFORM_TYPE_ATTR_NAME, STRING); - try { - return getPlatformType(attributeValue); - } catch ( - @SuppressWarnings("UnusedException") - ApplePlatform.UnsupportedPlatformTypeException exception) { - throw ruleContext.throwWithAttributeError( - ObjcRuleClasses.PLATFORM_TYPE_ATTR_NAME, - String.format(UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT, attributeValue)); - } - } - - /** - * Returns the apple platform type for the given platform type string (corresponding directly with - * platform type attribute value). - * - * @throws UnsupportedPlatformTypeException if the given platform type string is not a valid type - */ - private static PlatformType getPlatformType(String platformTypeString) - throws ApplePlatform.UnsupportedPlatformTypeException { - PlatformType platformType = PlatformType.fromString(platformTypeString); - - if (!SUPPORTED_PLATFORM_TYPES.contains(platformType)) { - throw new ApplePlatform.UnsupportedPlatformTypeException( - String.format(UNSUPPORTED_PLATFORM_TYPE_ERROR_FORMAT, platformTypeString)); - } else { - return platformType; - } - } - - @Override - public SplitTransition create(AttributeTransitionData data) { - String platformTypeString = - data.attributes().get(ObjcRuleClasses.PLATFORM_TYPE_ATTR_NAME, STRING); - String minimumOsVersionString = - data.attributes().get(ObjcRuleClasses.MINIMUM_OS_VERSION, STRING); - PlatformType platformType; - Optional minimumOsVersion; - try { - platformType = getPlatformType(platformTypeString); - // TODO(b/37096178): This should be a mandatory attribute. - if (Strings.isNullOrEmpty(minimumOsVersionString)) { - minimumOsVersion = Optional.absent(); - } else { - minimumOsVersion = Optional.of(DottedVersion.fromString(minimumOsVersionString)); - } - } catch (ApplePlatform.UnsupportedPlatformTypeException - | DottedVersion.InvalidDottedVersionException exception) { - // There's no opportunity to propagate exception information up cleanly at the transition - // provider level. This should later be registered as a rule error during the initialization - // of the rule. - platformType = PlatformType.IOS; - minimumOsVersion = Optional.absent(); - } - - return new AppleBinaryTransition(platformType, minimumOsVersion); - } - - @Override - public TransitionType transitionType() { - return TransitionType.ATTRIBUTE; - } - - @Override - public boolean isSplit() { - return true; - } - - @Override - public boolean isImmutable() { - return true; - } - - @Override - public void repr(Printer printer) { - printer.append("apple_common.multi_arch_split"); - } - - /** - * Transition that results in one configured target per architecture specified in the - * platform-specific cpu flag for a particular platform type (for example, --watchos_cpus for - * watchos platform type). - */ - protected static final class AppleBinaryTransition implements SplitTransition { - - private final PlatformType platformType; - // TODO(b/37096178): This should be a mandatory attribute. - private final Optional minimumOsVersion; - - public AppleBinaryTransition(PlatformType platformType, - Optional minimumOsVersion) { - this.platformType = platformType; - this.minimumOsVersion = minimumOsVersion; - } - - @Override - public ImmutableSet> requiresOptionFragments() { - return ImmutableSet.of( - AppleCommandLineOptions.class, - CoreOptions.class, - CppOptions.class, - ObjcCommandLineOptions.class, - PlatformOptions.class); - } - - @Override - public final Map split( - BuildOptionsView buildOptions, EventHandler eventHandler) { - AppleCommandLineOptions appleOptions = buildOptions.get(AppleCommandLineOptions.class); - if (appleOptions.incompatibleUseToolchainResolution) { - List