From 7800ff9eb2dc0b4b3b7a2e6b8a908078f3e73d3b Mon Sep 17 00:00:00 2001 From: ilist Date: Tue, 11 May 2021 01:20:11 -0700 Subject: [PATCH] Disable JavaInfo.transitive_export field and propagation of TransitiveExports provider. RELNOTES[inc]: --incompatible_enable_exports_provider is flipped to false. See #13457 for details. PiperOrigin-RevId: 373100553 --- .../semantics/BuildLanguageOptions.java | 15 ++++++ .../devtools/build/lib/rules/java/BUILD | 2 + .../build/lib/rules/java/JavaCommon.java | 8 ++- .../build/lib/rules/java/JavaInfo.java | 51 +++++++++++-------- .../lib/rules/java/JavaInfoBuildHelper.java | 5 +- .../build/lib/rules/java/JavaPluginInfo.java | 4 +- .../lib/rules/java/JavaStarlarkCommon.java | 8 ++- .../starlarkbuildapi/java/JavaCommonApi.java | 6 ++- .../starlarkbuildapi/java/JavaInfoApi.java | 3 ++ .../lib/rules/android/AarImportTest.java | 14 ----- .../lib/rules/android/AndroidLibraryTest.java | 36 ------------- .../rules/java/JavaInfoStarlarkApiTest.java | 4 ++ .../build/lib/rules/java/JavaInfoTest.java | 2 +- .../lib/rules/java/JavaStarlarkApiTest.java | 2 + 14 files changed, 80 insertions(+), 80 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index fd7f6969631f85..d42e7fff2117a4 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -146,6 +146,18 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable { help = "If set to true, enables the APIs required to support the Android Starlark migration.") public boolean experimentalEnableAndroidMigrationApis; + @Option( + name = "incompatible_enable_exports_provider", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS, OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = "This flag enables exports provider and JavaInfo.transitive_exports call.") + public boolean incompatibleEnableExportsProvider; + @Option( name = "experimental_google_legacy_api", defaultValue = "false", @@ -635,6 +647,7 @@ public StarlarkSemantics toStarlarkSemantics() { .set(EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE, experimentalBuiltinsInjectionOverride) .setBool( EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, experimentalEnableAndroidMigrationApis) + .setBool(INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER, incompatibleEnableExportsProvider) .setBool(EXPERIMENTAL_GOOGLE_LEGACY_API, experimentalGoogleLegacyApi) .setBool(EXPERIMENTAL_NINJA_ACTIONS, experimentalNinjaActions) .setBool(EXPERIMENTAL_PLATFORMS_API, experimentalPlatformsApi) @@ -702,6 +715,8 @@ public StarlarkSemantics toStarlarkSemantics() { "-experimental_disable_external_package"; public static final String EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS = "-experimental_enable_android_migration_apis"; + public static final String INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER = + "-incompatible_enable_exports_provider"; public static final String EXPERIMENTAL_EXEC_GROUPS = "-experimental_exec_groups"; public static final String EXPERIMENTAL_GOOGLE_LEGACY_API = "-experimental_google_legacy_api"; public static final String EXPERIMENTAL_NINJA_ACTIONS = "-experimental_ninja_actions"; diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/BUILD b/src/main/java/com/google/devtools/build/lib/rules/java/BUILD index 3399a06ba55415..48c69cbad37326 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/java/BUILD @@ -74,6 +74,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/rules:alias", "//src/main/java/com/google/devtools/build/lib/rules/cpp", "//src/main/java/com/google/devtools/build/lib/rules/proto", @@ -199,6 +200,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_resolver", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/rules/cpp", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index 4b20c81fc2a547..861d6eac0b0211 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.java; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -666,7 +667,7 @@ public void addTransitiveInfoProviders( NestedSet coverageSupportFiles) { JavaCompilationInfoProvider compilationInfoProvider = createCompilationInfoProvider(); - JavaExportsProvider exportsProvider = collectTransitiveExports(); + builder .addNativeDeclaredProvider( @@ -678,7 +679,10 @@ public void addTransitiveInfoProviders( coverageSupportFiles)) .addOutputGroup(OutputGroupInfo.FILES_TO_COMPILE, getFilesToCompile(classJar)); - javaInfoBuilder.addProvider(JavaExportsProvider.class, exportsProvider); + if (ruleContext.getStarlarkSemantics().getBool(INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER)) { + JavaExportsProvider exportsProvider = collectTransitiveExports(); + javaInfoBuilder.addProvider(JavaExportsProvider.class, exportsProvider); + } javaInfoBuilder.addProvider(JavaCompilationInfoProvider.class, compilationInfoProvider); addCcRelatedProviders(javaInfoBuilder); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfo.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfo.java index fa0e1231a302e0..ac3f2a79e89aac 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfo.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.java; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -125,7 +126,7 @@ public JavaPluginInfo getJavaPluginInfo() { * Merges the given providers into one {@link JavaInfo}. All the providers with the same type in * the given list are merged into one provider that is added to the resulting {@link JavaInfo}. */ - public static JavaInfo merge(List providers) { + public static JavaInfo merge(List providers, boolean withExportsProvider) { List javaCompilationArgsProviders = JavaInfo.fetchProvidersFromList(providers, JavaCompilationArgsProvider.class); List javaSourceJarsProviders = @@ -135,8 +136,7 @@ public static JavaInfo merge(List providers) { .map(JavaInfo::getJavaPluginInfo) .filter(Objects::nonNull) .collect(toImmutableList()); - List javaExportsProviders = - JavaInfo.fetchProvidersFromList(providers, JavaExportsProvider.class); + List javaRuleOutputJarsProviders = JavaInfo.fetchProvidersFromList(providers, JavaRuleOutputJarsProvider.class); List javaCcInfoProviders = @@ -149,23 +149,31 @@ public static JavaInfo merge(List providers) { javaConstraints.addAll(javaInfo.getJavaConstraints()); } - return JavaInfo.Builder.create() - .addProvider( - JavaCompilationArgsProvider.class, - JavaCompilationArgsProvider.merge(javaCompilationArgsProviders)) - .addProvider( - JavaSourceJarsProvider.class, JavaSourceJarsProvider.merge(javaSourceJarsProviders)) - .addProvider( - JavaRuleOutputJarsProvider.class, - JavaRuleOutputJarsProvider.merge(javaRuleOutputJarsProviders)) - .javaPluginInfo(JavaPluginInfo.merge(javaPluginInfos)) - .addProvider(JavaExportsProvider.class, JavaExportsProvider.merge(javaExportsProviders)) - .addProvider(JavaCcInfoProvider.class, JavaCcInfoProvider.merge(javaCcInfoProviders)) - // TODO(b/65618333): add merge function to JavaGenJarsProvider. See #3769 - // TODO(iirina): merge or remove JavaCompilationInfoProvider - .setRuntimeJars(runtimeJars.build()) - .setJavaConstraints(javaConstraints.build()) - .build(); + JavaInfo.Builder javaInfoBuilder = + JavaInfo.Builder.create() + .addProvider( + JavaCompilationArgsProvider.class, + JavaCompilationArgsProvider.merge(javaCompilationArgsProviders)) + .addProvider( + JavaSourceJarsProvider.class, JavaSourceJarsProvider.merge(javaSourceJarsProviders)) + .addProvider( + JavaRuleOutputJarsProvider.class, + JavaRuleOutputJarsProvider.merge(javaRuleOutputJarsProviders)) + .javaPluginInfo(JavaPluginInfo.merge(javaPluginInfos)) + .addProvider(JavaCcInfoProvider.class, JavaCcInfoProvider.merge(javaCcInfoProviders)) + // TODO(b/65618333): add merge function to JavaGenJarsProvider. See #3769 + // TODO(iirina): merge or remove JavaCompilationInfoProvider + .setRuntimeJars(runtimeJars.build()) + .setJavaConstraints(javaConstraints.build()); + + if (withExportsProvider) { + List javaExportsProviders = + JavaInfo.fetchProvidersFromList(providers, JavaExportsProvider.class); + javaInfoBuilder.addProvider( + JavaExportsProvider.class, JavaExportsProvider.merge(javaExportsProviders)); + } + + return javaInfoBuilder.build(); } /** @@ -510,7 +518,8 @@ public JavaInfo javaInfo( Sequence.cast(runtimeDeps, JavaInfo.class, "runtime_deps"), Sequence.cast(exports, JavaInfo.class, "exports"), Sequence.cast(nativeLibraries, CcInfo.class, "native_libraries"), - thread.getCallerLocation()); + thread.getCallerLocation(), + thread.getSemantics().getBool(INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER)); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java index cf51744367a66c..ca9eeddec603f3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaInfoBuildHelper.java @@ -85,7 +85,8 @@ JavaInfo createJavaInfo( Sequence runtimeDeps, Sequence exports, Sequence nativeLibraries, - Location location) { + Location location, + boolean withExportsProvider) { JavaInfo.Builder javaInfoBuilder = JavaInfo.Builder.create(); javaInfoBuilder.setLocation(location); @@ -121,9 +122,11 @@ JavaInfo createJavaInfo( javaInfoBuilder.addProvider( JavaCompilationArgsProvider.class, javaCompilationArgsBuilder.build()); + if (withExportsProvider) { javaInfoBuilder.addProvider( JavaExportsProvider.class, createJavaExportsProvider(exports, /* labels = */ ImmutableList.of())); + } javaInfoBuilder.javaPluginInfo(mergeExportedJavaPluginInfo(exports)); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java index f36df8b7f5098a..f898f35fbeb256 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java @@ -63,7 +63,9 @@ public JavaPluginInfoApi javaPluginInfo( ? NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER) : NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, (String) processorClass); NestedSet processorClasspath = - JavaInfo.merge(Sequence.cast(runtimeDeps, JavaInfo.class, "runtime_deps")) + JavaInfo.merge( + Sequence.cast(runtimeDeps, JavaInfo.class, "runtime_deps"), + /*withExportsProvider=*/ false) .getProvider(JavaCompilationArgsProvider.class) .getRuntimeJars(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java index df737a23e520e0..05335fd820871e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.java; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -200,9 +201,12 @@ public ImmutableList getDefaultJavacOpts(JavaToolchainProvider javaToolc } @Override - public JavaInfo mergeJavaProviders(Sequence providers /* expected. */) + public JavaInfo mergeJavaProviders( + Sequence providers, /* expected. */ StarlarkThread thread) throws EvalException { - return JavaInfo.merge(Sequence.cast(providers, JavaInfo.class, "providers")); + return JavaInfo.merge( + Sequence.cast(providers, JavaInfo.class, "providers"), + thread.getSemantics().getBool(INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER)); } // TODO(b/65113771): Remove this method because it's incorrect. diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java index 9816d81b9c87d3..682829b259251a 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java @@ -416,8 +416,10 @@ FileApi packSources( named = false, allowedTypes = {@ParamType(type = Sequence.class, generic1 = JavaInfoApi.class)}, doc = "The list of providers to merge."), - }) - JavaInfoT mergeJavaProviders(Sequence providers /* expected. */) + }, + useStarlarkThread = true) + JavaInfoT mergeJavaProviders( + Sequence providers /* expected. */, StarlarkThread thread) throws EvalException; @StarlarkMethod( diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaInfoApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaInfoApi.java index 25b9cd96d4a949..9099630a27e6d1 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaInfoApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaInfoApi.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.starlarkbuildapi.java; +import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER; + import com.google.common.collect.ImmutableList; import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.docgen.annot.StarlarkConstructor; @@ -172,6 +174,7 @@ public interface JavaInfoApi< @StarlarkMethod( name = "transitive_exports", structField = true, + enableOnlyWithFlag = INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER, doc = "Returns a set of labels that are being exported from this rule transitively.") Depset /*