Skip to content

Commit

Permalink
Add whitelist flag for --incompatible_disallow_legacy_javainfo.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
iirina authored and Copybara-Service committed Jan 7, 2019
1 parent c72aa8c commit e679d02
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> experimentalJavaCommonCreateProviderEnabledPackages;

@Option(
name = "experimental_remap_main_repo",
defaultValue = "false",
Expand Down Expand Up @@ -536,6 +549,8 @@ public SkylarkSemantics toSkylarkSemantics() {
.experimentalEnableAndroidMigrationApis(experimentalEnableAndroidMigrationApis)
.experimentalEnableRepoMapping(experimentalEnableRepoMapping)
.experimentalRemapMainRepo(experimentalRemapMainRepo)
.experimentalJavaCommonCreateProviderEnabledPackages(
experimentalJavaCommonCreateProviderEnabledPackages)
.experimentalPlatformsApi(experimentalPlatformsApi)
.experimentalStarlarkConfigTransitions(experimentalStarlarkConfigTransitions)
.incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
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 @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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(
Expand Down Expand Up @@ -221,4 +222,21 @@ private NestedSet<Artifact> 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<String> 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.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
+ "<a href ="
+ "'https://docs.bazel.build/versions/master/skylark/lib/JavaInfo.html#JavaInfo'>"
+ "JavaInfo()</a> 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."
+ "<p>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 "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {

public abstract boolean experimentalEnableRepoMapping();

public abstract ImmutableList<String> experimentalJavaCommonCreateProviderEnabledPackages();

public abstract boolean experimentalRemapMainRepo();

public abstract boolean experimentalPlatformsApi();
Expand Down Expand Up @@ -207,6 +209,7 @@ public static Builder builderWithDefaults() {
.experimentalCcSkylarkApiEnabledPackages(ImmutableList.of())
.experimentalEnableAndroidMigrationApis(false)
.experimentalEnableRepoMapping(false)
.experimentalJavaCommonCreateProviderEnabledPackages(ImmutableList.of())
.experimentalRemapMainRepo(false)
.experimentalPlatformsApi(false)
.experimentalStarlarkConfigTransitions(false)
Expand Down Expand Up @@ -256,6 +259,8 @@ public abstract static class Builder {

public abstract Builder experimentalRemapMainRepo(boolean value);

public abstract Builder experimentalJavaCommonCreateProviderEnabledPackages(List<String> value);

public abstract Builder experimentalPlatformsApi(boolean value);

public abstract Builder experimentalStarlarkConfigTransitions(boolean value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit e679d02

Please sign in to comment.