diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java index 1324ddd5b2e095..828fc47b045db2 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java @@ -621,6 +621,18 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl + "instead of linkopts to cc_toolchain_config") public boolean incompatibleLinkoptsToLinkLibs; + @Option( + name = "incompatible_objc_provider_remove_compile_info", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = "If set to true, the ObjcProvider's APIs for compile info/merge_zip will be removed.") + public boolean incompatibleObjcProviderRemoveCompileInfo; + @Option( name = "max_computation_steps", defaultValue = "0", @@ -697,6 +709,7 @@ public StarlarkSemantics toStarlarkSemantics() { .incompatibleRequireLinkerInputCcApi(incompatibleRequireLinkerInputCcApi) .incompatibleRestrictStringEscapes(incompatibleRestrictStringEscapes) .incompatibleLinkoptsToLinkLibs(incompatibleLinkoptsToLinkLibs) + .incompatibleObjcProviderRemoveCompileInfo(incompatibleObjcProviderRemoveCompileInfo) .maxComputationSteps(maxComputationSteps) .recordRuleInstantiationCallstack(recordRuleInstantiationCallstack) .build(); 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 ff7e964140f4ff..ec5fe74abb2d69 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 @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.syntax.Location; import com.google.devtools.build.lib.syntax.Sequence; import com.google.devtools.build.lib.syntax.Starlark; +import com.google.devtools.build.lib.syntax.StarlarkSemantics; import com.google.devtools.build.lib.syntax.StarlarkThread; import com.google.devtools.build.lib.syntax.StarlarkValue; import java.util.Map; @@ -62,6 +63,15 @@ public class AppleStarlarkCommon XcodeConfigInfo, ApplePlatform> { + @VisibleForTesting + public static final String DEPRECATED_KEY_ERROR = + "Key '%s' no longer supported in ObjcProvider (use CcInfo instead)."; + + @VisibleForTesting + public static final String BAD_DIRECT_DEP_PROVIDERS_ERROR = + "Argument 'direct_dep_providers' no longer supported. Please use 'strict_include' field " + + "in ObjcProvider."; + @VisibleForTesting public static final String BAD_KEY_ERROR = "Argument %s not a recognized key," @@ -188,8 +198,8 @@ public SplitTransitionProviderApi getMultiArchSplitProvider() { // This method is registered statically for Starlark, and never called directly. public ObjcProvider newObjcProvider(Boolean usesSwift, Dict kwargs, StarlarkThread thread) throws EvalException { - ObjcProvider.StarlarkBuilder resultBuilder = - new ObjcProvider.StarlarkBuilder(thread.getSemantics()); + StarlarkSemantics semantics = thread.getSemantics(); + ObjcProvider.StarlarkBuilder resultBuilder = new ObjcProvider.StarlarkBuilder(semantics); if (usesSwift) { resultBuilder.add(ObjcProvider.FLAG, ObjcProvider.Flag.USES_SWIFT); } @@ -202,6 +212,9 @@ public ObjcProvider newObjcProvider(Boolean usesSwift, Dict kwargs, Starla } else if (entry.getKey().equals("providers")) { resultBuilder.addProvidersFromStarlark(entry.getValue()); } else if (entry.getKey().equals("direct_dep_providers")) { + if (semantics.incompatibleObjcProviderRemoveCompileInfo()) { + throw new EvalException(null, BAD_DIRECT_DEP_PROVIDERS_ERROR); + } resultBuilder.addDirectDepProvidersFromStarlark(entry.getValue()); } else { throw new EvalException(null, String.format(BAD_KEY_ERROR, entry.getKey())); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java index 84d2a4afe84f80..2106b32f4091f2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java @@ -318,10 +318,13 @@ public enum Flag { private final CcCompilationContext ccCompilationContext; - /** Keys corresponding to compile information that has been migrated to CcCompilationContext. */ - static final ImmutableSet> KEYS_FOR_COMPILE_INFO = + /** + * Keys that are deprecated and will be removed. These include compile information that has been + * migrated to CcCompilationContext, plus MERGE_ZIP. + */ + static final ImmutableSet> DEPRECATED_KEYS = ImmutableSet.>of( - DEFINE, FRAMEWORK_SEARCH_PATHS, HEADER, INCLUDE, INCLUDE_SYSTEM, IQUOTE); + DEFINE, FRAMEWORK_SEARCH_PATHS, HEADER, INCLUDE, INCLUDE_SYSTEM, IQUOTE, MERGE_ZIP); /** All keys in ObjcProvider that will be passed in the corresponding Starlark provider. */ static final ImmutableList> KEYS_FOR_STARLARK = @@ -990,6 +993,10 @@ public Builder(StarlarkSemantics semantics) { this.starlarkSemantics = semantics; } + public StarlarkSemantics getStarlarkSemantics() { + return starlarkSemantics; + } + private static void maybeAddEmptyBuilder(Map, NestedSetBuilder> set, Key key) { set.computeIfAbsent(key, k -> new NestedSetBuilder<>(k.order)); } @@ -1185,53 +1192,17 @@ public StarlarkBuilder(StarlarkSemantics semantics) { */ void addElementsFromStarlark(Key key, Object starlarkToAdd) throws EvalException { NestedSet toAdd = ObjcProviderStarlarkConverters.convertToJava(key, starlarkToAdd); - if (KEYS_FOR_COMPILE_INFO.contains(key)) { - String keyName = key.getStarlarkKeyName(); - - if (key == DEFINE) { - ccCompilationContextBuilder.addDefines( - Depset.noneableCast(starlarkToAdd, String.class, keyName)); - } else if (key == FRAMEWORK_SEARCH_PATHS) { - // Due to legacy reasons, There is a mismatch between the starlark interface for the - // framework search path, and the internal representation. The interface specifies that - // framework_search_paths include the framework directories, but internally we only store - // their parents. We will eventually clean up the interface, but for now we need to do - // this ugly conversion. - - ImmutableList frameworks = - Depset.noneableCast(starlarkToAdd, String.class, keyName).toList().stream() - .map(x -> PathFragment.create(x)) - .collect(ImmutableList.toImmutableList()); - - ImmutableList.Builder frameworkSearchPaths = ImmutableList.builder(); - for (PathFragment framework : frameworks) { - if (!framework.getSafePathString().endsWith(FRAMEWORK_SUFFIX)) { - throw new EvalException( - null, String.format(AppleStarlarkCommon.BAD_FRAMEWORK_PATH_ERROR, framework)); - } - frameworkSearchPaths.add(framework.getParentDirectory()); + if (DEPRECATED_KEYS.contains(key)) { + if (getStarlarkSemantics().incompatibleObjcProviderRemoveCompileInfo()) { + if (!KEYS_FOR_DIRECT.contains(key)) { + throw new EvalException( + null, + String.format(AppleStarlarkCommon.DEPRECATED_KEY_ERROR, key.getStarlarkKeyName())); + } + } else { + if (!addCompileElementsFromStarlark(key, starlarkToAdd)) { + uncheckedAddTransitive(key, toAdd); } - ccCompilationContextBuilder.addFrameworkIncludeDirs(frameworkSearchPaths.build()); - } else if (key == HEADER) { - ImmutableList hdrs = - Depset.noneableCast(starlarkToAdd, Artifact.class, keyName).toList(); - ccCompilationContextBuilder.addDeclaredIncludeSrcs(hdrs); - ccCompilationContextBuilder.addTextualHdrs(hdrs); - } else if (key == INCLUDE) { - ccCompilationContextBuilder.addIncludeDirs( - Depset.noneableCast(starlarkToAdd, String.class, keyName).toList().stream() - .map(x -> PathFragment.create(x)) - .collect(ImmutableList.toImmutableList())); - } else if (key == INCLUDE_SYSTEM) { - ccCompilationContextBuilder.addSystemIncludeDirs( - Depset.noneableCast(starlarkToAdd, String.class, keyName).toList().stream() - .map(x -> PathFragment.create(x)) - .collect(ImmutableList.toImmutableList())); - } else if (key == IQUOTE) { - ccCompilationContextBuilder.addQuoteIncludeDirs( - Depset.noneableCast(starlarkToAdd, String.class, keyName).toList().stream() - .map(x -> PathFragment.create(x)) - .collect(ImmutableList.toImmutableList())); } } else { uncheckedAddTransitive(key, toAdd); @@ -1242,6 +1213,64 @@ void addElementsFromStarlark(Key key, Object starlarkToAdd) throws EvalExcept } } + private boolean addCompileElementsFromStarlark(Key key, Object starlarkToAdd) + throws EvalException { + String keyName = key.getStarlarkKeyName(); + + if (key == DEFINE) { + ccCompilationContextBuilder.addDefines( + Depset.noneableCast(starlarkToAdd, String.class, keyName)); + return true; + } else if (key == FRAMEWORK_SEARCH_PATHS) { + // Due to legacy reasons, There is a mismatch between the starlark interface for the + // framework search path, and the internal representation. The interface specifies that + // framework_search_paths include the framework directories, but internally we only store + // their parents. We will eventually clean up the interface, but for now we need to do + // this ugly conversion. + + ImmutableList frameworks = + Depset.noneableCast(starlarkToAdd, String.class, keyName).toList().stream() + .map(PathFragment::create) + .collect(ImmutableList.toImmutableList()); + + ImmutableList.Builder frameworkSearchPaths = ImmutableList.builder(); + for (PathFragment framework : frameworks) { + if (!framework.getSafePathString().endsWith(FRAMEWORK_SUFFIX)) { + throw new EvalException( + null, String.format(AppleStarlarkCommon.BAD_FRAMEWORK_PATH_ERROR, framework)); + } + frameworkSearchPaths.add(framework.getParentDirectory()); + } + ccCompilationContextBuilder.addFrameworkIncludeDirs(frameworkSearchPaths.build()); + return true; + } else if (key == HEADER) { + ImmutableList hdrs = + Depset.noneableCast(starlarkToAdd, Artifact.class, keyName).toList(); + ccCompilationContextBuilder.addDeclaredIncludeSrcs(hdrs); + ccCompilationContextBuilder.addTextualHdrs(hdrs); + return true; + } else if (key == INCLUDE) { + ccCompilationContextBuilder.addIncludeDirs( + Depset.noneableCast(starlarkToAdd, String.class, keyName).toList().stream() + .map(PathFragment::create) + .collect(ImmutableList.toImmutableList())); + return true; + } else if (key == INCLUDE_SYSTEM) { + ccCompilationContextBuilder.addSystemIncludeDirs( + Depset.noneableCast(starlarkToAdd, String.class, keyName).toList().stream() + .map(PathFragment::create) + .collect(ImmutableList.toImmutableList())); + return true; + } else if (key == IQUOTE) { + ccCompilationContextBuilder.addQuoteIncludeDirs( + Depset.noneableCast(starlarkToAdd, String.class, keyName).toList().stream() + .map(PathFragment::create) + .collect(ImmutableList.toImmutableList())); + return true; + } + return false; + } + /** * Adds the given providers from Starlark. An error is thrown if toAdd is not an iterable of * ObjcProvider instances. diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/ObjcProviderApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/ObjcProviderApi.java index 64f35ec52350b2..07a6c7e6e7e70f 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/ObjcProviderApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple/ObjcProviderApi.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.skylarkbuildapi.FileApi; import com.google.devtools.build.lib.skylarkbuildapi.cpp.CcCompilationContextApi; import com.google.devtools.build.lib.syntax.Sequence; +import com.google.devtools.build.lib.syntax.StarlarkSemantics.FlagIdentifier; import com.google.devtools.build.lib.syntax.StarlarkValue; import net.starlark.java.annot.StarlarkBuiltin; import net.starlark.java.annot.StarlarkDocumentationCategory; @@ -38,7 +39,8 @@ public interface ObjcProviderApi extends StarlarkValue structField = true, doc = "A set of strings from 'defines' attributes. These are to be passed as '-D' flags to " - + "all invocations of the compiler for this target and all depending targets.") + + "all invocations of the compiler for this target and all depending targets.", + disableWithFlag = FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO) Depset /**/ defineForStarlark(); @StarlarkMethod( @@ -60,7 +62,8 @@ public interface ObjcProviderApi extends StarlarkValue structField = true, doc = "Exec paths of .framework directories corresponding to frameworks to include " - + "in search paths, but not to link.") + + "in search paths, but not to link.", + disableWithFlag = FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO) Depset /**/ frameworkIncludeForStarlark(); @StarlarkMethod( @@ -72,7 +75,8 @@ public interface ObjcProviderApi extends StarlarkValue @StarlarkMethod( name = "header", structField = true, - doc = "All header files. These may be either public or private headers.") + doc = "All header files. These may be either public or private headers.", + disableWithFlag = FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO) Depset /**/ headerForStarlark(); @StarlarkMethod( @@ -94,7 +98,8 @@ public interface ObjcProviderApi extends StarlarkValue structField = true, doc = "Include search paths specified with '-I' on the command line. Also known as " - + "header search paths (and distinct from user header search paths).") + + "header search paths (and distinct from user header search paths).", + disableWithFlag = FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO) Depset includeForStarlark(); @StarlarkMethod( @@ -108,13 +113,15 @@ public interface ObjcProviderApi extends StarlarkValue @StarlarkMethod( name = "include_system", structField = true, - doc = "System include search paths (typically specified with -isystem).") + doc = "System include search paths (typically specified with -isystem).", + disableWithFlag = FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO) Depset systemIncludeForStarlark(); @StarlarkMethod( name = "iquote", structField = true, - doc = "User header search paths (typically specified with -iquote).") + doc = "User header search paths (typically specified with -iquote).", + disableWithFlag = FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO) Depset quoteIncludeForStarlark(); @StarlarkMethod( @@ -168,7 +175,8 @@ public interface ObjcProviderApi extends StarlarkValue doc = "Merge zips to include in the bundle. The entries of these zip files are included " + "in the final bundle with the same path. The entries in the merge zips should not " - + "include the bundle root path (e.g. 'Foo.app').") + + "include the bundle root path (e.g. 'Foo.app').", + disableWithFlag = FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO) Depset /**/ mergeZip(); @StarlarkMethod( @@ -276,6 +284,7 @@ public interface ObjcProviderApi extends StarlarkValue doc = "Returns the embedded CcCompilationContext that contains the" + "provider's compilation information.", - structField = true) + structField = true, + disableWithFlag = FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO) CcCompilationContextApi getCcCompilationContext(); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java index 70c6f43c8df889..cfafb1302e012f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java @@ -99,6 +99,8 @@ private FlagIdentifier() {} // uninstantiable public static final String INCOMPATIBLE_NO_ATTR_LICENSE = "incompatible_no_attr_license"; public static final String INCOMPATIBLE_ALLOW_TAGS_PROPAGATION = "incompatible_allow_tags_propagation"; + public static final String INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO = + "incompatible_objc_provider_remove_compile_info"; public static final String INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API = "incompatible_require_linker_input_cc_api"; public static final String INCOMPATIBLE_LINKOPTS_TO_LINKLIBS = @@ -147,6 +149,8 @@ public boolean flagValue(String flag) { return incompatibleNoAttrLicense(); case FlagIdentifier.INCOMPATIBLE_ALLOW_TAGS_PROPAGATION: return experimentalAllowTagsPropagation(); + case FlagIdentifier.INCOMPATIBLE_OBJC_PROVIDER_REMOVE_COMPILE_INFO: + return incompatibleObjcProviderRemoveCompileInfo(); case FlagIdentifier.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API: return incompatibleRequireLinkerInputCcApi(); case FlagIdentifier.INCOMPATIBLE_LINKOPTS_TO_LINKLIBS: @@ -264,6 +268,8 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli public abstract boolean incompatibleDepsetForLibrariesToLinkGetter(); + public abstract boolean incompatibleObjcProviderRemoveCompileInfo(); + public abstract boolean incompatibleRequireLinkerInputCcApi(); public abstract boolean incompatibleRestrictStringEscapes(); @@ -347,6 +353,7 @@ public static Builder builderWithDefaults() { .internalStarlarkFlagTestCanary(false) .incompatibleDoNotSplitLinkingCmdline(true) .incompatibleDepsetForLibrariesToLinkGetter(true) + .incompatibleObjcProviderRemoveCompileInfo(false) .incompatibleRequireLinkerInputCcApi(false) .incompatibleRestrictStringEscapes(false) .incompatibleUseCcConfigureFromRulesCc(false) @@ -432,6 +439,8 @@ public abstract static class Builder { public abstract Builder incompatibleDepsetForLibrariesToLinkGetter(boolean value); + public abstract Builder incompatibleObjcProviderRemoveCompileInfo(boolean value); + public abstract Builder incompatibleRequireLinkerInputCcApi(boolean value); public abstract Builder incompatibleRestrictStringEscapes(boolean value); diff --git a/src/test/java/com/google/devtools/build/lib/packages/StarlarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/StarlarkSemanticsConsistencyTest.java index 212a2f6bdabb6a..63f2d10ed1eb09 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/StarlarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/StarlarkSemanticsConsistencyTest.java @@ -155,6 +155,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E "--incompatible_no_implicit_file_export=" + rand.nextBoolean(), "--incompatible_no_rule_outputs_param=" + rand.nextBoolean(), "--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(), + "--incompatible_objc_provider_remove_compile_info=" + rand.nextBoolean(), "--incompatible_run_shell_command_string=" + rand.nextBoolean(), "--incompatible_string_replace_count=" + rand.nextBoolean(), "--incompatible_visibility_private_attributes_at_definition=" + rand.nextBoolean(), @@ -207,6 +208,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .incompatibleNoImplicitFileExport(rand.nextBoolean()) .incompatibleNoRuleOutputsParam(rand.nextBoolean()) .incompatibleNoSupportToolsInActionInputs(rand.nextBoolean()) + .incompatibleObjcProviderRemoveCompileInfo(rand.nextBoolean()) .incompatibleRunShellCommandString(rand.nextBoolean()) .incompatibleStringReplaceCount(rand.nextBoolean()) .incompatibleVisibilityPrivateAttributesAtDefinition(rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkTest.java index a9fbf073a68194..2b127bc7e67e7b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcStarlarkTest.java @@ -140,13 +140,13 @@ public void testObjcProviderLegacyName() throws Exception { "test/my_rule.bzl", "load('//myinfo:myinfo.bzl', 'MyInfo')", "def _dep_rule_impl(ctx):", - " objc_provider = apple_common.new_objc_provider(define=depset(['mock_define']))", + " objc_provider = apple_common.new_objc_provider(linkopt=depset(['mock_linkopt']))", " return struct(foo = objc_provider)", "", "def _root_rule_impl(ctx):", " dep = ctx.attr.deps[0]", " return MyInfo(", - " define = dep.objc.define,", + " linkopt = dep.objc.linkopt,", " )", "", "root_rule = rule(implementation = _root_rule_impl,", @@ -166,9 +166,9 @@ public void testObjcProviderLegacyName() throws Exception { ConfiguredTarget starlarkTarget = getConfiguredTarget("//test:test"); StructImpl myInfo = getMyInfoFromTarget(starlarkTarget); - Depset defineSet = (Depset) myInfo.getValue("define"); + Depset linkoptSet = (Depset) myInfo.getValue("linkopt"); - assertThat(defineSet.getSet(String.class).toList()).containsExactly("mock_define"); + assertThat(linkoptSet.getSet(String.class).toList()).containsExactly("mock_linkopt"); } @Test @@ -996,21 +996,17 @@ private ConfiguredTarget createObjcProviderStarlarkTarget(String... implLines) t public void testStarlarkCanCreateObjcProviderFromScratch() throws Exception { ConfiguredTarget starlarkTarget = createObjcProviderStarlarkTarget( - " defines = depset(['define1', 'define2'])", " linkopts = depset(['somelinkopt'])", " created_provider = apple_common.new_objc_provider\\", - "(define=defines, linkopt=linkopts)", + "(linkopt=linkopts)", " return [created_provider]"); Iterable foundLinkopts = starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).get(ObjcProvider.LINKOPT).toList(); - Iterable foundDefines = - starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).define().toList(); boolean usesSwift = starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).is(ObjcProvider.Flag.USES_SWIFT); assertThat(foundLinkopts).containsExactly("somelinkopt"); - assertThat(foundDefines).containsExactly("define1", "define2"); assertThat(usesSwift).isFalse(); } @@ -1044,22 +1040,21 @@ public void testStarlarkCanPassUsesSwiftFlag() throws Exception { } @Test - public void testStarlarkCanCreateObjcProviderWithDefines() throws Exception { + public void testStarlarkCanCreateObjcProviderWithLinkopts() throws Exception { ConfiguredTarget starlarkTarget = createObjcProviderStarlarkTarget( - " define = depset(['def1', 'def2', 'def3'])", + " linkopt = depset(['opt1', 'opt2', 'opt3'])", " created_provider = apple_common.new_objc_provider\\", - "(define=define)", + "(linkopt=linkopt)", " return [created_provider]"); - Iterable foundDefines = - starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).define().toList(); + Iterable foundLinkopts = + starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).get(ObjcProvider.LINKOPT).toList(); - assertThat(foundDefines).containsExactly("def1", "def2", "def3"); + assertThat(foundLinkopts).containsExactly("opt1", "opt2", "opt3"); } - @Test - public void testStarlarkCanCreateObjcProviderWithHeaders() throws Exception { + private void testStarlarkCanCreateObjcProviderWithHeaders() throws Exception { ConfiguredTarget starlarkTarget = createObjcProviderStarlarkTarget( " hdr1 = ctx.actions.declare_file('hdr1')", @@ -1071,13 +1066,30 @@ public void testStarlarkCanCreateObjcProviderWithHeaders() throws Exception { " return [created_provider]"); Iterable foundHeaders = - starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).header().toList(); + starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).get(ObjcProvider.HEADER).toList(); + assertThat(foundHeaders).isEmpty(); - assertThat(ActionsTestUtil.baseArtifactNames(foundHeaders)).containsExactly("hdr1", "hdr2"); + Iterable directHeaders = + starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).getDirect(ObjcProvider.HEADER); + assertThat(ActionsTestUtil.baseArtifactNames(directHeaders)).containsExactly("hdr1", "hdr2"); + } + + @Test + public void testStarlarkCanCreateObjcProviderWithHeadersPreAPIRemoval() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); + testStarlarkCanCreateObjcProviderWithHeaders(); + } + + @Test + public void testStarlarkCanCreateObjcProviderWithHeadersPostAPIRemoval() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=true"); + testStarlarkCanCreateObjcProviderWithHeaders(); } @Test - public void testStarlarkCanCreateObjcProviderWithIncludePathFragments() throws Exception { + public void testStarlarkCanCreateObjcProviderWithIncludePathFragmentsPreAPIRemoval() + throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); ConfiguredTarget starlarkTarget = createObjcProviderStarlarkTarget( " includes = depset(['path1', 'path_dir/path2', 'path_dir1/path_dir2/path3'])", @@ -1096,7 +1108,9 @@ public void testStarlarkCanCreateObjcProviderWithIncludePathFragments() throws E } @Test - public void testStarlarkCanCreateObjcProviderWithFrameworkIncludes() throws Exception { + public void testStarlarkCanCreateObjcProviderWithFrameworkIncludesPreAPIRemoval() + throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); ConfiguredTarget starlarkTarget = createObjcProviderStarlarkTarget( " includes = depset(['path1/foo.framework', 'path_dir/path2/bar.framework'])", @@ -1112,7 +1126,8 @@ public void testStarlarkCanCreateObjcProviderWithFrameworkIncludes() throws Exce } @Test - public void testStarlarkCanCreateObjcProviderWithSystemIncludes() throws Exception { + public void testStarlarkCanCreateObjcProviderWithSystemIncludesPreAPIRemoval() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); ConfiguredTarget starlarkTarget = createObjcProviderStarlarkTarget( " includes = depset(['path1', 'path_dir/path2', 'path_dir1/path_dir2/path3'])", @@ -1131,7 +1146,8 @@ public void testStarlarkCanCreateObjcProviderWithSystemIncludes() throws Excepti } @Test - public void testStarlarkCanCreateObjcProviderWithQuoteIncludes() throws Exception { + public void testStarlarkCanCreateObjcProviderWithQuoteIncludesPreAPIRemoval() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); ConfiguredTarget starlarkTarget = createObjcProviderStarlarkTarget( " includes = depset(['path1', 'path_dir/path2', 'path_dir1/path_dir2/path3'])", @@ -1149,6 +1165,131 @@ public void testStarlarkCanCreateObjcProviderWithQuoteIncludes() throws Exceptio PathFragment.create("path_dir1/path_dir2/path3")); } + @Test + public void testStarlarkCannotCreateObjcProviderWithIncludePathFragmentsPostAPIRemoval() + throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=true"); + AssertionError e = + assertThrows( + AssertionError.class, + () -> + createObjcProviderStarlarkTarget( + " includes = depset(['path'])", + " created_provider = apple_common.new_objc_provider\\", + "(include=includes)", + " return [created_provider]")); + assertThat(e) + .hasMessageThat() + .contains(String.format(AppleStarlarkCommon.DEPRECATED_KEY_ERROR, "include")); + } + + @Test + public void testStarlarkCannotCreateObjcProviderWithDefinePostAPIRemoval() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=true"); + AssertionError e = + assertThrows( + AssertionError.class, + () -> + createObjcProviderStarlarkTarget( + " define = depset(['def'])", + " created_provider = apple_common.new_objc_provider\\", + "(define=define)", + " return [created_provider]")); + assertThat(e) + .hasMessageThat() + .contains(String.format(AppleStarlarkCommon.DEPRECATED_KEY_ERROR, "define")); + } + + @Test + public void testStarlarkCannotCreateObjcProviderWithFrameworkIncludesPostAPIRemoval() + throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=true"); + AssertionError e = + assertThrows( + AssertionError.class, + () -> + createObjcProviderStarlarkTarget( + " includes = depset(['path1/foo.framework'])", + " created_provider = apple_common.new_objc_provider\\", + "(framework_search_paths=includes)", + " return [created_provider]")); + assertThat(e) + .hasMessageThat() + .contains( + String.format(AppleStarlarkCommon.DEPRECATED_KEY_ERROR, "framework_search_paths")); + } + + @Test + public void testStarlarkCannotCreateObjcProviderWithSystemIncludesPostAPIRemoval() + throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=true"); + AssertionError e = + assertThrows( + AssertionError.class, + () -> + createObjcProviderStarlarkTarget( + " includes = depset(['path1'])", + " created_provider = apple_common.new_objc_provider\\", + "(include_system=includes)", + " return [created_provider]")); + assertThat(e) + .hasMessageThat() + .contains(String.format(AppleStarlarkCommon.DEPRECATED_KEY_ERROR, "include_system")); + } + + @Test + public void testStarlarkCannotCreateObjcProviderWithQuoteIncludesPostAPIRemoval() + throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=true"); + AssertionError e = + assertThrows( + AssertionError.class, + () -> + createObjcProviderStarlarkTarget( + " includes = depset(['path1'])", + " created_provider = apple_common.new_objc_provider\\", + "(iquote=includes)", + " return [created_provider]")); + assertThat(e) + .hasMessageThat() + .contains(String.format(AppleStarlarkCommon.DEPRECATED_KEY_ERROR, "iquote")); + } + + @Test + public void testStarlarkCanCreateObjcProviderWithMergeZipsPreAPIRemoval() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); + ConfiguredTarget starlarkTarget = + createObjcProviderStarlarkTarget( + " file = ctx.actions.declare_file('file')", + " ctx.actions.run_shell(outputs=[file], command='echo')", + " created_provider = apple_common.new_objc_provider\\", + "(merge_zip = depset([file]))", + " return [created_provider]"); + + Iterable foundMergeZips = + starlarkTarget.get(ObjcProvider.STARLARK_CONSTRUCTOR).get(ObjcProvider.MERGE_ZIP).toList(); + + assertThat(ActionsTestUtil.baseArtifactNames(foundMergeZips)).containsExactly("file"); + } + + @Test + public void testStarlarkCannotCreateObjcProviderWithMergeZipsPostAPIRemoval() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=true"); + AssertionError e = + assertThrows( + AssertionError.class, + () -> + createObjcProviderStarlarkTarget( + " file = ctx.actions.declare_file('file')", + " ctx.actions.run_shell(outputs=[file], command='echo')", + " created_provider = apple_common.new_objc_provider\\", + "(merge_zip = depset([file]))", + " return [created_provider]")); + assertThat(e) + .hasMessageThat() + .contains(String.format(AppleStarlarkCommon.DEPRECATED_KEY_ERROR, "merge_zip")); + } + @Test public void testStarlarkCanCreateObjcProviderWithStrictDepsPreMigration() throws Exception { useConfiguration("--incompatible_objc_compile_info_migration=false"); @@ -1195,6 +1336,7 @@ public void testStarlarkCanCreateObjcProviderWithStrictDepsPreMigration() throws @Test public void testStarlarkCanCreateObjcProviderWithStrictDepsPostMigration() throws Exception { useConfiguration("--incompatible_objc_compile_info_migration=true"); + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); ConfiguredTarget starlarkTarget = createObjcProviderStarlarkTarget( " strict_includes = depset(['path1'])", @@ -1252,6 +1394,7 @@ public void testStarlarkCanCreateObjcProviderWithStrictDepsDirectly() throws Exc @Test public void testStarlarkStrictDepsDoesNotSupportDefine() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); AssertionError e = assertThrows( AssertionError.class, @@ -1270,6 +1413,7 @@ public void testStarlarkStrictDepsDoesNotSupportDefine() throws Exception { @Test public void testStarlarkStrictDepsDoesNotSupportLinkopt() throws Exception { + setStarlarkSemanticsOptions("--incompatible_objc_provider_remove_compile_info=false"); AssertionError e = assertThrows( AssertionError.class, @@ -1379,7 +1523,7 @@ public void testEmptyObjcProviderKeysArePresent() throws Exception { "def swift_binary_impl(ctx):", " objc_provider = ctx.attr.deps[0].objc", " return MyInfo(", - " empty_value=objc_provider.include,", + " empty_value=objc_provider.linkopt,", " )", "swift_binary = rule(", "implementation = swift_binary_impl,", diff --git a/src/test/java/com/google/devtools/build/lib/skylark/StarlarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/StarlarkIntegrationTest.java index 0ecbc0ed06fce9..388449084a9491 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/StarlarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/StarlarkIntegrationTest.java @@ -3319,7 +3319,7 @@ public void testLegacyProvider_AddCanonicalLegacyKeyAndModernKey() throws Except scratch.file( "test/starlark/extension.bzl", "def custom_rule_impl(ctx):", - " return struct(foo = apple_common.new_objc_provider(define=depset(['foo'])))", + " return struct(foo = apple_common.new_objc_provider(linkopt=depset(['foo'])))", "", "custom_rule = rule(implementation = custom_rule_impl)"); @@ -3336,9 +3336,9 @@ public void testLegacyProvider_AddCanonicalLegacyKeyAndModernKey() throws Except ObjcProvider providerFromFoo = (ObjcProvider) target.get("foo"); // The modern key and the canonical legacy key "objc" are set to the one available ObjcProvider. - assertThat(providerFromModernKey.define().toList()).containsExactly("foo"); - assertThat(providerFromObjc.define().toList()).containsExactly("foo"); - assertThat(providerFromFoo.define().toList()).containsExactly("foo"); + assertThat(providerFromModernKey.get(ObjcProvider.LINKOPT).toList()).containsExactly("foo"); + assertThat(providerFromObjc.get(ObjcProvider.LINKOPT).toList()).containsExactly("foo"); + assertThat(providerFromFoo.get(ObjcProvider.LINKOPT).toList()).containsExactly("foo"); } @Test @@ -3347,9 +3347,9 @@ public void testLegacyProvider_DontAutomaticallyAddKeysAlreadyPresent() throws E scratch.file( "test/starlark/extension.bzl", "def custom_rule_impl(ctx):", - " return struct(providers = [apple_common.new_objc_provider(define=depset(['prov']))],", - " bah = apple_common.new_objc_provider(define=depset(['bah'])),", - " objc = apple_common.new_objc_provider(define=depset(['objc'])))", + " return struct(providers = [apple_common.new_objc_provider(linkopt=depset(['prov']))],", + " bah = apple_common.new_objc_provider(linkopt=depset(['bah'])),", + " objc = apple_common.new_objc_provider(linkopt=depset(['objc'])))", "", "custom_rule = rule(implementation = custom_rule_impl)"); @@ -3365,9 +3365,9 @@ public void testLegacyProvider_DontAutomaticallyAddKeysAlreadyPresent() throws E ObjcProvider providerFromObjc = (ObjcProvider) target.get("objc"); ObjcProvider providerFromBah = (ObjcProvider) target.get("bah"); - assertThat(providerFromModernKey.define().toList()).containsExactly("prov"); - assertThat(providerFromObjc.define().toList()).containsExactly("objc"); - assertThat(providerFromBah.define().toList()).containsExactly("bah"); + assertThat(providerFromModernKey.get(ObjcProvider.LINKOPT).toList()).containsExactly("prov"); + assertThat(providerFromObjc.get(ObjcProvider.LINKOPT).toList()).containsExactly("objc"); + assertThat(providerFromBah.get(ObjcProvider.LINKOPT).toList()).containsExactly("bah"); } @Test @@ -3376,9 +3376,9 @@ public void testLegacyProvider_FirstNoncanonicalKeyBecomesCanonical() throws Exc scratch.file( "test/starlark/extension.bzl", "def custom_rule_impl(ctx):", - " return struct(providers = [apple_common.new_objc_provider(define=depset(['prov']))],", - " foo = apple_common.new_objc_provider(define=depset(['foo'])),", - " bar = apple_common.new_objc_provider(define=depset(['bar'])))", + " return struct(providers = [apple_common.new_objc_provider(linkopt=depset(['prov']))],", + " foo = apple_common.new_objc_provider(linkopt=depset(['foo'])),", + " bar = apple_common.new_objc_provider(linkopt=depset(['bar'])))", "", "custom_rule = rule(implementation = custom_rule_impl)"); @@ -3395,11 +3395,11 @@ public void testLegacyProvider_FirstNoncanonicalKeyBecomesCanonical() throws Exc ObjcProvider providerFromFoo = (ObjcProvider) target.get("foo"); ObjcProvider providerFromBar = (ObjcProvider) target.get("bar"); - assertThat(providerFromModernKey.define().toList()).containsExactly("prov"); + assertThat(providerFromModernKey.get(ObjcProvider.LINKOPT).toList()).containsExactly("prov"); // The first defined provider is set to the legacy "objc" key. - assertThat(providerFromObjc.define().toList()).containsExactly("foo"); - assertThat(providerFromFoo.define().toList()).containsExactly("foo"); - assertThat(providerFromBar.define().toList()).containsExactly("bar"); + assertThat(providerFromObjc.get(ObjcProvider.LINKOPT).toList()).containsExactly("foo"); + assertThat(providerFromFoo.get(ObjcProvider.LINKOPT).toList()).containsExactly("foo"); + assertThat(providerFromBar.get(ObjcProvider.LINKOPT).toList()).containsExactly("bar"); } @Test