From f929b1a174b535ad6e42bf908879996b77060c31 Mon Sep 17 00:00:00 2001 From: lberki Date: Tue, 25 Jul 2017 08:47:23 +0200 Subject: Add a command line option to make the minimum_os_version attribute mandatory on Apple rules. RELNOTES: None. PiperOrigin-RevId: 163035922 --- .../lib/rules/apple/AppleCommandLineOptions.java | 11 +++++++++++ .../build/lib/rules/apple/AppleConfiguration.java | 7 +++++++ .../objc/MultiArchSplitTransitionProvider.java | 13 +++++++++---- .../build/lib/rules/objc/AppleBinaryTest.java | 21 +++++++++++++++++++++ .../lib/rules/objc/AppleStaticLibraryTest.java | 20 ++++++++++++++++++++ .../build/lib/rules/objc/AppleStubBinaryTest.java | 18 ++++++++++++++++++ 6 files changed, 86 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java index 3b13623b37..36c9449462 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java @@ -41,6 +41,17 @@ import java.util.List; */ public class AppleCommandLineOptions extends FragmentOptions { + @Option( + name = "experimental_apple_mandatory_minimum_version", + defaultValue = "false", + category = "experimental", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = { OptionEffectTag.LOSES_INCREMENTAL_STATE, OptionEffectTag.BUILD_FILE_SEMANTICS }, + help = "Whether Apple rules must have a mandatory minimum_os_version attribute." + ) + // TODO(b/37096178): This flag should be default-on and then be removed. + public boolean mandatoryMinimumVersion; + @Option( name = "xcode_version", defaultValue = "null", diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java index 01c5879a22..97027f3d99 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java @@ -95,6 +95,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment { private final boolean enableAppleCrosstool; @Nullable private final String xcodeToolchain; @Nullable private final Label defaultProvisioningProfileLabel; + private final boolean mandatoryMinimumVersion; AppleConfiguration( AppleCommandLineOptions appleOptions, @@ -146,6 +147,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment { this.enableAppleCrosstool = appleOptions.enableAppleCrosstoolTransition; this.defaultProvisioningProfileLabel = appleOptions.defaultProvisioningProfile; this.xcodeToolchain = appleOptions.xcodeToolchain; + this.mandatoryMinimumVersion = appleOptions.mandatoryMinimumVersion; } /** Determines cpu value from apple-specific toolchain identifier. */ @@ -616,6 +618,11 @@ public class AppleConfiguration extends BuildConfiguration.Fragment { return xcodeToolchain; } + /** Returns true if the minimum_os_version attribute should be mandatory on rules with linking. */ + public boolean isMandatoryMinimumVersion() { + return mandatoryMinimumVersion; + } + /** Returns true if {@link AppleCrosstoolTransition} should be applied to every apple rule. */ public boolean isAppleCrosstoolEnabled() { return enableAppleCrosstool; 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 index 5e36fdef53..6a6d19b3d4 100644 --- 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 @@ -83,17 +83,22 @@ public class MultiArchSplitTransitionProvider implements SplitTransitionProvider */ public static void validateMinimumOs(RuleContext ruleContext) throws RuleErrorException { String attributeValue = ruleContext.attributes().get(PlatformRule.MINIMUM_OS_VERSION, STRING); - // TODO(b/37096178): This should be a mandatory attribute. - if (!Strings.isNullOrEmpty(attributeValue)) { + // TODO(b/37096178): This attribute should always be a version. + if (Strings.isNullOrEmpty(attributeValue)) { + if (ruleContext.getFragment(AppleConfiguration.class).isMandatoryMinimumVersion()) { + ruleContext.throwWithAttributeError(PlatformRule.MINIMUM_OS_VERSION, + "This attribute must be explicitly specified"); + } + } else { try { DottedVersion minimumOsVersion = DottedVersion.fromString(attributeValue); if (minimumOsVersion.hasAlphabeticCharacters() || minimumOsVersion.numComponents() > 2) { - throw ruleContext.throwWithAttributeError( + ruleContext.throwWithAttributeError( PlatformRule.MINIMUM_OS_VERSION, String.format(INVALID_VERSION_STRING_ERROR_FORMAT, attributeValue)); } } catch (IllegalArgumentException exception) { - throw ruleContext.throwWithAttributeError( + ruleContext.throwWithAttributeError( PlatformRule.MINIMUM_OS_VERSION, String.format(INVALID_VERSION_STRING_ERROR_FORMAT, attributeValue)); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java index 89d3517442..a359149170 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java @@ -74,6 +74,27 @@ public class AppleBinaryTest extends ObjcRuleTestCase { private static final ImmutableSet COCOA_FEATURE_FLAGS = ImmutableSet.of(COCOA_FRAMEWORK_FLAG); + @Test + public void testMandatoryMinimumOsVersionUnset() throws Exception { + RULE_TYPE.scratchTarget(scratch, + "srcs", "['a.m']", + "platform_type", "'watchos'"); + useConfiguration("--experimental_apple_mandatory_minimum_version"); + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//x:x"); + assertContainsEvent("must be explicitly specified"); + } + + @Test + public void testMandatoryMinimumOsVersionSet() throws Exception { + RULE_TYPE.scratchTarget(scratch, + "minimum_os_version", "'8.0'", + "srcs", "['a.m']", + "platform_type", "'watchos'"); + useConfiguration("--experimental_apple_mandatory_minimum_version"); + getConfiguredTarget("//x:x"); + } + @Test public void testLipoActionEnv() throws Exception { RULE_TYPE.scratchTarget(scratch, diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java index 19e2013ede..6d8a4125ab 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibraryTest.java @@ -56,6 +56,26 @@ public class AppleStaticLibraryTest extends ObjcRuleTestCase { } }; + @Test + public void testMandatoryMinimumOsVersionUnset() throws Exception { + RULE_TYPE.scratchTarget(scratch, + "srcs", "['a.m']", + "platform_type", "'watchos'"); + useConfiguration("--experimental_apple_mandatory_minimum_version"); + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//x:x"); + assertContainsEvent("must be explicitly specified"); + } + + @Test + public void testMandatoryMinimumOsVersionSet() throws Exception { + RULE_TYPE.scratchTarget(scratch, + "minimum_os_version", "'8.0'", + "srcs", "['a.m']", + "platform_type", "'watchos'"); + useConfiguration("--experimental_apple_mandatory_minimum_version"); + getConfiguredTarget("//x:x"); + } @Test public void testUnknownPlatformType() throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStubBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStubBinaryTest.java index 695a218a7b..c6f58ef67e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStubBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleStubBinaryTest.java @@ -45,6 +45,24 @@ public class AppleStubBinaryTest extends ObjcRuleTestCase { } }; + @Test + public void testMandatoryMinimumOsVersionUnset() throws Exception { + RULE_TYPE.scratchTarget(scratch); + useConfiguration("--experimental_apple_mandatory_minimum_version"); + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//x:x"); + assertContainsEvent("must be explicitly specified"); + } + + @Test + public void testMandatoryMinimumOsVersionSet() throws Exception { + RULE_TYPE.scratchTarget(scratch, + "xcenv_based_path", "'$(SDKROOT)/Foo'", + "minimum_os_version", "'8.0'"); + useConfiguration("--experimental_apple_mandatory_minimum_version"); + getConfiguredTarget("//x:x"); + } + @Test public void testCopyActionEnv() throws Exception { RULE_TYPE.scratchTarget( -- cgit v1.2.3