Skip to content

Commit

Permalink
Disable JavaInfo.transitive_export field and propagation of Transitiv…
Browse files Browse the repository at this point in the history
…eExports provider.

RELNOTES[inc]: --incompatible_enable_exports_provider is flipped to false. See #13457 for details.

PiperOrigin-RevId: 373100553
  • Loading branch information
comius authored and copybara-github committed May 11, 2021
1 parent 7caa01f commit 7800ff9
Show file tree
Hide file tree
Showing 14 changed files with 80 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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";
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/java/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -666,7 +667,7 @@ public void addTransitiveInfoProviders(
NestedSet<Artifact> coverageSupportFiles) {

JavaCompilationInfoProvider compilationInfoProvider = createCompilationInfoProvider();
JavaExportsProvider exportsProvider = collectTransitiveExports();


builder
.addNativeDeclaredProvider(
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<JavaInfo> providers) {
public static JavaInfo merge(List<JavaInfo> providers, boolean withExportsProvider) {
List<JavaCompilationArgsProvider> javaCompilationArgsProviders =
JavaInfo.fetchProvidersFromList(providers, JavaCompilationArgsProvider.class);
List<JavaSourceJarsProvider> javaSourceJarsProviders =
Expand All @@ -135,8 +136,7 @@ public static JavaInfo merge(List<JavaInfo> providers) {
.map(JavaInfo::getJavaPluginInfo)
.filter(Objects::nonNull)
.collect(toImmutableList());
List<JavaExportsProvider> javaExportsProviders =
JavaInfo.fetchProvidersFromList(providers, JavaExportsProvider.class);

List<JavaRuleOutputJarsProvider> javaRuleOutputJarsProviders =
JavaInfo.fetchProvidersFromList(providers, JavaRuleOutputJarsProvider.class);
List<JavaCcInfoProvider> javaCcInfoProviders =
Expand All @@ -149,23 +149,31 @@ public static JavaInfo merge(List<JavaInfo> 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<JavaExportsProvider> javaExportsProviders =
JavaInfo.fetchProvidersFromList(providers, JavaExportsProvider.class);
javaInfoBuilder.addProvider(
JavaExportsProvider.class, JavaExportsProvider.merge(javaExportsProviders));
}

return javaInfoBuilder.build();
}

/**
Expand Down Expand Up @@ -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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ JavaInfo createJavaInfo(
Sequence<JavaInfo> runtimeDeps,
Sequence<JavaInfo> exports,
Sequence<CcInfo> nativeLibraries,
Location location) {
Location location,
boolean withExportsProvider) {
JavaInfo.Builder javaInfoBuilder = JavaInfo.Builder.create();
javaInfoBuilder.setLocation(location);

Expand Down Expand Up @@ -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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ public JavaPluginInfoApi<JavaPluginData> javaPluginInfo(
? NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER)
: NestedSetBuilder.create(Order.NAIVE_LINK_ORDER, (String) processorClass);
NestedSet<Artifact> 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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -200,9 +201,12 @@ public ImmutableList<String> getDefaultJavacOpts(JavaToolchainProvider javaToolc
}

@Override
public JavaInfo mergeJavaProviders(Sequence<?> providers /* <JavaInfo> expected. */)
public JavaInfo mergeJavaProviders(
Sequence<?> providers, /* <JavaInfo> 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 /* <JavaInfoT> expected. */)
},
useStarlarkThread = true)
JavaInfoT mergeJavaProviders(
Sequence<?> providers /* <JavaInfoT> expected. */, StarlarkThread thread)
throws EvalException;

@StarlarkMethod(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 /*<Label>*/ getTransitiveExports();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static java.util.stream.Collectors.toList;

import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.Artifact;
Expand All @@ -28,7 +27,6 @@
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.rules.android.AarImportTest.WithPlatforms;
import com.google.devtools.build.lib.rules.android.AarImportTest.WithoutPlatforms;
Expand Down Expand Up @@ -649,18 +647,6 @@ private String getAndroidManifest(String aarImport) throws Exception {
.getRootRelativePathString();
}

@Test
public void testTransitiveExports() throws Exception {
assertThat(
getConfiguredTarget("//a:bar")
.get(JavaInfo.PROVIDER)
.getTransitiveExports()
.toList(Label.class))
.containsExactly(
Label.parseAbsolute("//a:foo", ImmutableMap.of()),
Label.parseAbsolute("//java:baz", ImmutableMap.of()));
}

@Test
public void testRClassFromAarImportInCompileClasspath() throws Exception {
Collection<Artifact> compilationClasspath =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
import com.google.devtools.build.lib.rules.java.JavaCompilationInfoProvider;
import com.google.devtools.build.lib.rules.java.JavaCompileAction;
import com.google.devtools.build.lib.rules.java.JavaExportsProvider;
import com.google.devtools.build.lib.rules.java.JavaInfo;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider;
import com.google.devtools.build.lib.rules.java.JavaSemantics;
Expand Down Expand Up @@ -670,41 +669,6 @@ public void testExportsRunfiles() throws Exception {
assertNoEvents();
}

@Test
public void testTransitiveExports() throws Exception {
scratch.file(
"java/com/google/exports/BUILD",
"android_library(",
" name = 'dummy',",
" srcs = ['dummy.java'],",
" exports = [':dummy2'],",
")",
"android_library(",
" name = 'dummy2',",
" srcs = ['dummy2.java'],",
" exports = [':dummy3'],",
")",
"android_library(",
" name = 'dummy3',",
" srcs = ['dummy3.java'],",
" exports = [':dummy4'],",
")",
"android_library(",
" name = 'dummy4',",
" srcs = ['dummy4.java'],",
")");

ConfiguredTarget target = getConfiguredTarget("//java/com/google/exports:dummy");
List<Label> exports =
JavaInfo.getProvider(JavaExportsProvider.class, target).getTransitiveExports().toList();
assertThat(exports)
.containsExactly(
Label.parseAbsolute("//java/com/google/exports:dummy2", ImmutableMap.of()),
Label.parseAbsolute("//java/com/google/exports:dummy3", ImmutableMap.of()),
Label.parseAbsolute("//java/com/google/exports:dummy4", ImmutableMap.of()));
assertNoEvents();
}

@Test
public void testSimpleIdl() throws Exception {
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ public void buildHelperCreateJavaInfoJavaSourceJarsProviderAndTransitiveRuntimeD
/** Tests that JavaExportsProvider is empty by default. */
@Test
public void buildHelperCreateJavaInfoExportIsEmpty() throws Exception {
setBuildLanguageOptions("--incompatible_enable_exports_provider");
ruleBuilder().build();
scratch.file(
"foo/BUILD",
Expand All @@ -461,6 +462,7 @@ public void buildHelperCreateJavaInfoExportIsEmpty() throws Exception {
/** Test exports adds dependencies to JavaCompilationArgsProvider. */
@Test
public void buildHelperCreateJavaInfoExportProviderExportsDepsAdded() throws Exception {
setBuildLanguageOptions("--incompatible_enable_exports_provider");
ruleBuilder().build();
scratch.file(
"foo/BUILD",
Expand Down Expand Up @@ -502,6 +504,7 @@ public void buildHelperCreateJavaInfoExportProviderExportsDepsAdded() throws Exc
*/
@Test
public void buildHelperCreateJavaInfoExportProvider() throws Exception {
setBuildLanguageOptions("--incompatible_enable_exports_provider");
ruleBuilder().build();
scratch.file(
"foo/BUILD",
Expand Down Expand Up @@ -560,6 +563,7 @@ public void buildHelperCreateJavaInfoExportProvider() throws Exception {
*/
@Test
public void buildHelperCreateJavaInfoExportProvider001() throws Exception {
setBuildLanguageOptions("--incompatible_enable_exports_provider");
ruleBuilder().build();
scratch.file(
"foo/BUILD",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void testMergeJavaExportsProvider() throws Exception {
.addProvider(JavaExportsProvider.class, createJavaExportsProvider("bar", 3))
.build();

JavaInfo javaInfoMerged = JavaInfo.merge(ImmutableList.of(javaInfo1, javaInfo2));
JavaInfo javaInfoMerged = JavaInfo.merge(ImmutableList.of(javaInfo1, javaInfo2), true);

NestedSet<Label> labels = javaInfoMerged.getTransitiveExports().getSet(Label.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1897,6 +1897,7 @@ public void testJavaInfoGetTransitiveRuntimeDeps() throws Exception {

@Test
public void testJavaInfoGetTransitiveExports() throws Exception {
setBuildLanguageOptions("--incompatible_enable_exports_provider");
scratch.file(
"foo/extension.bzl",
"result = provider()",
Expand Down Expand Up @@ -2192,6 +2193,7 @@ private void writeJavaCustomLibraryWithLabels(String path) throws Exception {

@Test
public void javaCompile_transitiveExportsWithLabels() throws Exception {
setBuildLanguageOptions("--incompatible_enable_exports_provider");
JavaToolchainTestUtil.writeBuildFileForJavaToolchain(scratch);
writeJavaCustomLibraryWithLabels("tools/build_defs/java/java_library.bzl");
scratch.file(
Expand Down

0 comments on commit 7800ff9

Please sign in to comment.