From e679d02c7beea7864c1f5638507da21f178b429b Mon Sep 17 00:00:00 2001 From: elenairina Date: Mon, 7 Jan 2019 07:49:27 -0800 Subject: [PATCH] Add whitelist flag for --incompatible_disallow_legacy_javainfo. Progress on #5821 RELNOTES: There is a new flag available `--experimental_java_common_create_provider_enabled_packages` that acts as a whitelist for usages of `java_common.create_provider`. The constructor will be deprecated in Bazel 0.23. PiperOrigin-RevId: 228164706 --- .../lib/packages/SkylarkSemanticsOptions.java | 15 ++++++++ .../devtools/build/lib/rules/java/BUILD | 2 ++ .../lib/rules/java/JavaSkylarkCommon.java | 26 +++++++++++--- .../skylarkbuildapi/java/JavaCommonApi.java | 6 +++- .../build/lib/syntax/SkylarkSemantics.java | 5 +++ .../SkylarkSemanticsConsistencyTest.java | 6 ++++ .../lib/rules/java/JavaSkylarkApiTest.java | 35 +++++++++++++++++-- 7 files changed, 88 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java index 919064f20bfb0e..b389dc4dff829f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java @@ -100,6 +100,19 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable help = "If set to true, enables the use of the `repo_mapping` attribute in WORKSPACE files.") public boolean experimentalEnableRepoMapping; + // This flag is declared in SkylarkSemanticsOptions instead of JavaOptions because there is no + // way to retrieve the java configuration from the Java implementation of + // java_common.create_provider. + @Option( + name = "experimental_java_common_create_provider_enabled_packages", + converter = CommaSeparatedOptionListConverter.class, + defaultValue = "", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = {OptionMetadataTag.EXPERIMENTAL}, + help = "Passes list of packages that can use the java_common.create_provider Starlark API.") + public List experimentalJavaCommonCreateProviderEnabledPackages; + @Option( name = "experimental_remap_main_repo", defaultValue = "false", @@ -536,6 +549,8 @@ public SkylarkSemantics toSkylarkSemantics() { .experimentalEnableAndroidMigrationApis(experimentalEnableAndroidMigrationApis) .experimentalEnableRepoMapping(experimentalEnableRepoMapping) .experimentalRemapMainRepo(experimentalRemapMainRepo) + .experimentalJavaCommonCreateProviderEnabledPackages( + experimentalJavaCommonCreateProviderEnabledPackages) .experimentalPlatformsApi(experimentalPlatformsApi) .experimentalStarlarkConfigTransitions(experimentalStarlarkConfigTransitions) .incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement) 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 d01553af678a73..4d46d2211a2f74 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 @@ -54,6 +54,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:packages-internal", "//src/main/java/com/google/devtools/build/lib:proto-rules", "//src/main/java/com/google/devtools/build/lib:provider-collection", + "//src/main/java/com/google/devtools/build/lib:skylark_semantics", "//src/main/java/com/google/devtools/build/lib:skylarkinterface", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/actions", @@ -129,6 +130,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:events", "//src/main/java/com/google/devtools/build/lib:packages-internal", "//src/main/java/com/google/devtools/build/lib:provider-collection", + "//src/main/java/com/google/devtools/build/lib:skylark_semantics", "//src/main/java/com/google/devtools/build/lib:skylarkinterface", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib/actions", diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java index 7954c3b12cfa3a..5be43d7c6512da 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java @@ -31,7 +31,9 @@ import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; +import com.google.devtools.build.lib.syntax.SkylarkSemantics; import com.google.devtools.build.lib.syntax.Type; +import java.util.List; import javax.annotation.Nullable; /** A module that contains Skylark utilities for Java support. */ @@ -54,11 +56,10 @@ public JavaInfo create( Environment environment) throws EvalException { if (environment.getSemantics().incompatibleDisallowLegacyJavaInfo()) { - throw new EvalException( + checkCallPathInWhitelistedPackages( + environment.getSemantics(), location, - "create_provider is deprecated and cannot be used when " - + "--incompatible_disallow_legacy_javainfo is set. " - + "Please migrate to the JavaInfo constructor."); + environment.getCallerLabel().getPackageFragment().toString()); } return JavaInfoBuildHelper.getInstance() .create( @@ -221,4 +222,21 @@ private NestedSet asArtifactNestedSet(Object o) throws EvalException { .addAll(((SkylarkList) o).getContents(Artifact.class, /*description=*/ null)) .build(); } + + /** + * Throws an {@link EvalException} if the given {@code callPath} is not listed under the {@code + * --experimental_java_common_create_provider_enabled_packages} flag. + */ + private static void checkCallPathInWhitelistedPackages( + SkylarkSemantics semantics, Location location, String callPath) throws EvalException { + List whitelistedPackagesList = + semantics.experimentalJavaCommonCreateProviderEnabledPackages(); + if (whitelistedPackagesList.stream().noneMatch(path -> callPath.startsWith(path))) { + throw new EvalException( + location, + "java_common.create_provider is deprecated and cannot be used when " + + "--incompatible_disallow_legacy_javainfo is set. " + + "Please migrate to the JavaInfo constructor."); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/java/JavaCommonApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/java/JavaCommonApi.java index 3118c371a6128a..a66616f5ec2c19 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/java/JavaCommonApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/java/JavaCommonApi.java @@ -44,7 +44,11 @@ public interface JavaCommonApi< @SkylarkCallable( name = "create_provider", doc = - "Creates a JavaInfo from jars. compile_time/runtime_jars represent the outputs of the " + "This API is deprecated. It will be disabled by default in Bazel 0.23. Please use " + + "" + + "JavaInfo() instead." + + "Creates a JavaInfo from jars. compile_time/runtime_jars are the outputs of the " + "target providing a JavaInfo, while transitive_*_jars represent their dependencies." + "

Note: compile_time_jars and runtime_jars are not automatically merged into the " + "transitive jars (unless the given transitive_*_jars are empty) - if this is the " diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java index 413c81858a6a24..51961b9619f8ba 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java @@ -124,6 +124,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) { public abstract boolean experimentalEnableRepoMapping(); + public abstract ImmutableList experimentalJavaCommonCreateProviderEnabledPackages(); + public abstract boolean experimentalRemapMainRepo(); public abstract boolean experimentalPlatformsApi(); @@ -207,6 +209,7 @@ public static Builder builderWithDefaults() { .experimentalCcSkylarkApiEnabledPackages(ImmutableList.of()) .experimentalEnableAndroidMigrationApis(false) .experimentalEnableRepoMapping(false) + .experimentalJavaCommonCreateProviderEnabledPackages(ImmutableList.of()) .experimentalRemapMainRepo(false) .experimentalPlatformsApi(false) .experimentalStarlarkConfigTransitions(false) @@ -256,6 +259,8 @@ public abstract static class Builder { public abstract Builder experimentalRemapMainRepo(boolean value); + public abstract Builder experimentalJavaCommonCreateProviderEnabledPackages(List value); + public abstract Builder experimentalPlatformsApi(boolean value); public abstract Builder experimentalStarlarkConfigTransitions(boolean value); diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index c5ace037cfa13c..d74369be6cb257 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java @@ -125,6 +125,10 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex + rand.nextDouble(), "--experimental_enable_android_migration_apis=" + rand.nextBoolean(), "--experimental_enable_repo_mapping=" + rand.nextBoolean(), + "--experimental_java_common_create_provider_enabled_packages=" + + rand.nextDouble() + + "," + + rand.nextDouble(), "--experimental_platforms_api=" + rand.nextBoolean(), "--experimental_remap_main_repo=" + rand.nextBoolean(), "--experimental_starlark_config_transitions=" + rand.nextBoolean(), @@ -171,6 +175,8 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) { ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble()))) .experimentalEnableAndroidMigrationApis(rand.nextBoolean()) .experimentalEnableRepoMapping(rand.nextBoolean()) + .experimentalJavaCommonCreateProviderEnabledPackages( + ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble()))) .experimentalPlatformsApi(rand.nextBoolean()) .experimentalRemapMainRepo(rand.nextBoolean()) .experimentalStarlarkConfigTransitions(rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java index 2b34373ec5b5d3..4464f35be5d502 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/java/JavaSkylarkApiTest.java @@ -1839,14 +1839,45 @@ public void testIncompatibleDisallowLegacyJavaInfo() throws Exception { checkError( "java/test", "custom", - "create_provider is deprecated and cannot be used when " - + "--incompatible_disallow_legacy_javainfo is set. ", + "java_common.create_provider is deprecated and cannot be used when " + + "--incompatible_disallow_legacy_javainfo is set.", "load(':custom_rule.bzl', 'java_custom_library')", "java_custom_library(", " name = 'custom',", ")"); } + @Test + public void testIncompatibleDisallowLegacyJavaInfoWithFlag() throws Exception { + setSkylarkSemanticsOptions("--incompatible_disallow_legacy_javainfo"); + setSkylarkSemanticsOptions( + "--experimental_java_common_create_provider_enabled_packages=java/test"); + scratch.file( + "java/test/custom_rule.bzl", + "def _impl(ctx):", + " jar = ctx.file.jar", + " java_common.create_provider(", + " compile_time_jars = [jar],", + " transitive_compile_time_jars = [jar],", + " runtime_jars = [jar],", + " use_ijar = False,", + " )", + "java_custom_library = rule(", + " implementation = _impl,", + " attrs = {", + " 'jar': attr.label(allow_files = True, single_file = True),", + " }", + ")"); + scratch.file( + "java/test/BUILD", + "load(':custom_rule.bzl', 'java_custom_library')", + "java_custom_library(", + " name = 'custom',", + " jar = 'lib.jar'", + ")"); + assertThat(getConfiguredTarget("//java/test:custom")).isNotNull(); + } + private static boolean javaCompilationArgsHaveTheSameParent( JavaCompilationArgsProvider args, JavaCompilationArgsProvider otherArgs) { if (!nestedSetsOfArtifactHaveTheSameParent(