Skip to content

Commit

Permalink
Add --incompatible_use_toolchain_providers_in_java_common
Browse files Browse the repository at this point in the history
Which requires passing JavaToolchainInfo and JavaRuntimeInfo providers to
java_common APIs instead of configured targets.

See: #7186

RELNOTES: incompatible_use_toolchain_providers_in_java_common: pass JavaToolchainInfo and JavaRuntimeInfo providers to java_common APIs instead of configured targets (#7186)
PiperOrigin-RevId: 231169120
  • Loading branch information
cushon authored and Copybara-Service committed Jan 28, 2019
1 parent f04e676 commit 4a6f708
Show file tree
Hide file tree
Showing 10 changed files with 215 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,20 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
)
public boolean internalSkylarkFlagTestCanary;

@Option(
name = "incompatible_use_toolchain_providers_in_java_common",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If set to true, java_common APIs that take a java_toolchain or host_javabase parameter "
+ " require a JavaTootoolchainInfo or JavaRuntimeInfo instead of a configured"
+ " target.")
public boolean incompatibleUseToolchainProvidersInJavaCommon;

/** Constructs a {@link SkylarkSemantics} object corresponding to this set of option values. */
public SkylarkSemantics toSkylarkSemantics() {
Expand Down Expand Up @@ -544,6 +558,8 @@ public SkylarkSemantics toSkylarkSemantics() {
.incompatibleRequireFeatureConfigurationForPic(requireFeatureConfigurationForPic)
.incompatibleStricArgumentOrdering(incompatibleStricArgumentOrdering)
.incompatibleStringIsNotIterable(incompatibleStringIsNotIterable)
.incompatibleUseToolchainProvidersInJavaCommon(
incompatibleUseToolchainProvidersInJavaCommon)
.internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ public JavaInfo javaInfo(
javaToolchain,
hostJavabase,
jdeps,
env.getSemantics(),
loc);
}
if (compileJar == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import java.util.stream.Stream;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -75,6 +76,7 @@ public static JavaInfoBuildHelper getInstance() {
* @param actions used to create the ijar and single jar actions
* @param javaToolchain the toolchain to be used for retrieving the ijar tool
* @param jdeps optional jdeps information for outputJar
* @param semantics the skylark semantics
* @return new created JavaInfo instance
* @throws EvalException if some mandatory parameter are missing
*/
Expand All @@ -92,6 +94,7 @@ JavaInfo createJavaInfoLegacy(
Object javaToolchain,
Object hostJavabase,
@Nullable Artifact jdeps,
SkylarkSemantics semantics,
Location location)
throws EvalException {
final Artifact sourceJar;
Expand All @@ -103,10 +106,10 @@ JavaInfo createJavaInfoLegacy(
if (!(actions instanceof SkylarkActionFactory)) {
throw new EvalException(location, "Must pass ctx.actions when packing sources.");
}
if (!isValidJavaToolchain(javaToolchain)) {
if (!isValidJavaToolchain(semantics, javaToolchain)) {
throw new EvalException(location, "Must pass java_toolchain when packing sources.");
}
if (!isValidJavaRuntime(hostJavabase)) {
if (!isValidJavaRuntime(semantics, hostJavabase)) {
throw new EvalException(location, "Must pass host_javabase when packing sources.");
}
sourceJar =
Expand All @@ -117,6 +120,7 @@ JavaInfo createJavaInfoLegacy(
sourceJars,
javaToolchain,
hostJavabase,
semantics,
location);
}
final Artifact iJar;
Expand All @@ -126,7 +130,7 @@ JavaInfo createJavaInfoLegacy(
location,
"The value of use_ijar is True. Make sure the ctx.actions argument is valid.");
}
if (!isValidJavaToolchain(javaToolchain)) {
if (!isValidJavaToolchain(semantics, javaToolchain)) {
throw new EvalException(
location,
"The value of use_ijar is True. Make sure the java_toolchain argument is valid.");
Expand All @@ -136,7 +140,8 @@ JavaInfo createJavaInfoLegacy(
(SkylarkActionFactory) actions,
outputJar,
null,
getJavaToolchainProvider(location, javaToolchain),
getJavaToolchainProvider(semantics, location, javaToolchain),
semantics,
location);
} else {
iJar = outputJar;
Expand Down Expand Up @@ -245,6 +250,7 @@ Artifact packSourceFiles(
SkylarkList<Artifact> sourceJars,
Object javaToolchain,
Object hostJavabase,
SkylarkSemantics semantics,
Location location)
throws EvalException {
// No sources to pack, return None
Expand All @@ -257,8 +263,10 @@ Artifact packSourceFiles(
}
ActionRegistry actionRegistry = actions.asActionRegistry(location, actions);
Artifact outputSrcJar = getSourceJar(actions.getActionConstructionContext(), outputJar);
JavaRuntimeInfo javaRuntimeInfo = getJavaRuntimeProvider(location, hostJavabase, null);
JavaToolchainProvider javaToolchainProvider = getJavaToolchainProvider(location, javaToolchain);
JavaRuntimeInfo javaRuntimeInfo =
getJavaRuntimeProvider(semantics, location, hostJavabase, null);
JavaToolchainProvider javaToolchainProvider =
getJavaToolchainProvider(semantics, location, javaToolchain);
JavaSemantics javaSemantics = javaToolchainProvider.getJavaSemantics();
SingleJarActionBuilder.createSourceJarAction(
actionRegistry,
Expand Down Expand Up @@ -318,6 +326,7 @@ public JavaInfo create(
NestedSet<Artifact> transitiveCompileTimeJars,
NestedSet<Artifact> transitiveRuntimeJars,
NestedSet<Artifact> sourceJars,
SkylarkSemantics semantics,
Location location)
throws EvalException {

Expand All @@ -329,7 +338,7 @@ public JavaInfo create(
location,
"The value of use_ijar is True. Make sure the ctx.actions argument is valid.");
}
if (!isValidJavaToolchain(javaToolchain)) {
if (!isValidJavaToolchain(semantics, javaToolchain)) {
throw new EvalException(
location,
"The value of use_ijar is True. Make sure the java_toolchain argument is valid.");
Expand All @@ -341,7 +350,8 @@ public JavaInfo create(
(SkylarkActionFactory) actions,
compileJar,
null,
getJavaToolchainProvider(location, javaToolchain),
getJavaToolchainProvider(semantics, location, javaToolchain),
semantics,
location));
}
javaCompilationArgsBuilder.addDirectCompileTimeJars(
Expand Down Expand Up @@ -397,12 +407,17 @@ public JavaInfo createJavaCompileAction(
}

JavaRuntimeInfo javaRuntimeInfo =
getJavaRuntimeProvider(location, hostJavabase, skylarkRuleContext.getRuleContext());
getJavaRuntimeProvider(
skylarkRuleContext.getSkylarkSemantics(),
location,
hostJavabase,
skylarkRuleContext.getRuleContext());
if (javaRuntimeInfo == null) {
throw new EvalException(location, "'host_javabase' must point to a Java runtime");
}

JavaToolchainProvider toolchainProvider = getJavaToolchainProvider(location, javaToolchain);
JavaToolchainProvider toolchainProvider =
getJavaToolchainProvider(environment.getSemantics(), location, javaToolchain);

JavaLibraryHelper helper =
new JavaLibraryHelper(skylarkRuleContext.getRuleContext())
Expand Down Expand Up @@ -500,11 +515,13 @@ public Artifact buildIjar(
Artifact inputJar,
@Nullable Label targetLabel,
Object javaToolchain,
SkylarkSemantics semantics,
Location location)
throws EvalException {
String ijarBasename = FileSystemUtils.removeExtension(inputJar.getFilename()) + "-ijar.jar";
Artifact interfaceJar = actions.declareFile(ijarBasename, inputJar);
FilesToRunProvider ijarTarget = getJavaToolchainProvider(location, javaToolchain).getIjar();
FilesToRunProvider ijarTarget =
getJavaToolchainProvider(semantics, location, javaToolchain).getIjar();
CustomCommandLine.Builder commandLine =
CustomCommandLine.builder().addExecPath(inputJar).addExecPath(interfaceJar);
if (targetLabel != null) {
Expand All @@ -528,12 +545,14 @@ public Artifact stampJar(
Artifact inputJar,
Label targetLabel,
Object javaToolchain,
SkylarkSemantics semantics,
Location location)
throws EvalException {
String basename = FileSystemUtils.removeExtension(inputJar.getFilename()) + "-stamped.jar";
Artifact outputJar = actions.declareFile(basename, inputJar);
// ijar doubles as a stamping tool
FilesToRunProvider ijarTarget = getJavaToolchainProvider(location, javaToolchain).getIjar();
FilesToRunProvider ijarTarget =
getJavaToolchainProvider(semantics, location, javaToolchain).getIjar();
CustomCommandLine.Builder commandLine =
CustomCommandLine.builder()
.addExecPath(inputJar)
Expand All @@ -553,15 +572,24 @@ public Artifact stampJar(
return outputJar;
}

private static boolean isValidJavaToolchain(Object javaToolchain) {
return javaToolchain instanceof ConfiguredTarget
private static boolean isValidJavaToolchain(
SkylarkSemantics skylarkSemantics, Object javaToolchain) {
return (!skylarkSemantics.incompatibleUseToolchainProvidersInJavaCommon()
&& javaToolchain instanceof ConfiguredTarget)
|| javaToolchain instanceof JavaToolchainProvider;
}

JavaToolchainProvider getJavaToolchainProvider(Location location, Object javaToolchain)
throws EvalException {
JavaToolchainProvider getJavaToolchainProvider(
SkylarkSemantics semantics, Location location, Object javaToolchain) throws EvalException {
if (javaToolchain instanceof ConfiguredTarget) {
// TODO(b/122738702): remove support for passing toolchains as configured targets
if (semantics.incompatibleUseToolchainProvidersInJavaCommon()) {
// TODO(b/122738702): remove support for passing toolchains as configured targets
throw new EvalException(
location,
javaToolchain
+ " pass a java_common.JavaToolchainInfo instead of a configured target;"
+ " see https://github.com/bazelbuild/bazel/issues/7186.");
}
ConfiguredTarget target = (ConfiguredTarget) javaToolchain;
JavaToolchainProvider javaToolchainProvider = JavaToolchainProvider.from(target);
if (javaToolchainProvider == null) {
Expand All @@ -576,14 +604,27 @@ JavaToolchainProvider getJavaToolchainProvider(Location location, Object javaToo
throw new EvalException(null, javaToolchain + " is not a JavaToolchainProvider.");
}

private static boolean isValidJavaRuntime(Object javaRuntime) {
return javaRuntime instanceof ConfiguredTarget || javaRuntime instanceof JavaRuntimeInfo;
private static boolean isValidJavaRuntime(SkylarkSemantics skylarkSemantics, Object javaRuntime) {
return (!skylarkSemantics.incompatibleUseToolchainProvidersInJavaCommon()
&& javaRuntime instanceof ConfiguredTarget)
|| javaRuntime instanceof JavaRuntimeInfo;
}

private JavaRuntimeInfo getJavaRuntimeProvider(
Location location, Object javabase, RuleContext ruleContext) throws EvalException {
SkylarkSemantics skylarkSemantics,
Location location,
Object javabase,
RuleContext ruleContext)
throws EvalException {
if (javabase instanceof TransitiveInfoCollection) {
// TODO(b/122738702): remove support for passing toolchains as configured targets
if (skylarkSemantics.incompatibleUseToolchainProvidersInJavaCommon()) {
// TODO(b/122738702): remove support for passing toolchains as configured targets
throw new EvalException(
location,
javabase
+ " pass a java_common.JavaRuntimeInfo instead of a configured target;"
+ " see https://github.com/bazelbuild/bazel/issues/7186.");
}
return JavaRuntimeInfo.from((TransitiveInfoCollection) javabase, ruleContext);
}
if (javabase instanceof JavaRuntimeInfo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public JavaInfo create(
asArtifactNestedSet(transitiveCompileTimeJars),
asArtifactNestedSet(transitiveRuntimeJars),
asArtifactNestedSet(sourceJars),
environment.getSemantics(),
location);
}

Expand Down Expand Up @@ -131,14 +132,16 @@ public Artifact runIjar(
Artifact jar,
Object targetLabel,
Object javaToolchain,
Location location)
Location location,
SkylarkSemantics semantics)
throws EvalException {
return JavaInfoBuildHelper.getInstance()
.buildIjar(
actions,
jar,
targetLabel != Runtime.NONE ? (Label) targetLabel : null,
javaToolchain,
semantics,
location);
}

Expand All @@ -148,10 +151,11 @@ public Artifact stampJar(
Artifact jar,
Label targetLabel,
Object javaToolchain,
Location location)
Location location,
SkylarkSemantics semantics)
throws EvalException {
return JavaInfoBuildHelper.getInstance()
.stampJar(actions, jar, targetLabel, javaToolchain, location);
.stampJar(actions, jar, targetLabel, javaToolchain, semantics, location);
}

@Override
Expand All @@ -162,25 +166,36 @@ public Artifact packSources(
SkylarkList<Artifact> sourceJars,
Object javaToolchain,
Object hostJavabase,
Location location)
Location location,
SkylarkSemantics semantics)
throws EvalException {
return JavaInfoBuildHelper.getInstance()
.packSourceFiles(
actions, outputJar, sourceFiles, sourceJars, javaToolchain, hostJavabase, location);
actions,
outputJar,
sourceFiles,
sourceJars,
javaToolchain,
hostJavabase,
semantics,
location);
}

@Override
// TODO(b/78512644): migrate callers to passing explicit javacopts or using custom toolchains, and
// delete
public ImmutableList<String> getDefaultJavacOpts(
SkylarkRuleContext skylarkRuleContext, String javaToolchainAttr, Location location)
SkylarkRuleContext skylarkRuleContext,
String javaToolchainAttr,
Location location,
SkylarkSemantics skylarkSemantics)
throws EvalException {
RuleContext ruleContext = skylarkRuleContext.getRuleContext();
ConfiguredTarget javaToolchainConfigTarget =
(ConfiguredTarget) skylarkRuleContext.getAttr().getValue(javaToolchainAttr);
JavaToolchainProvider toolchain =
JavaInfoBuildHelper.getInstance()
.getJavaToolchainProvider(location, javaToolchainConfigTarget);
.getJavaToolchainProvider(skylarkSemantics, location, javaToolchainConfigTarget);
ImmutableList<String> javacOptsFromAttr;
if (ruleContext.getRule().isAttrDefined("javacopts", Type.STRING_LIST)) {
javacOptsFromAttr = ruleContext.getExpander().withDataLocations().tokenized("javacopts");
Expand Down
Loading

0 comments on commit 4a6f708

Please sign in to comment.