Skip to content

Commit

Permalink
Add toggle for ijar in java_import
Browse files Browse the repository at this point in the history
Problem: java_import has been unusably broken for years for JARs with Kotlin interfaces, since ijar strips out important information. This has caused multiple dependent projects (includng official Bazel ones) to abandon java_import in favor of rolling their own versions, which themselves contain issues that are getting fixed in java_import. Fragmentation is bad, and fragmentation of bugs and fixes is worse.
For more, see
https://github.com/bazelbuild/rules_jvm_external/blob/master/private/rules/jvm_import.bzl
#4549
#14966
bazel-contrib/rules_jvm_external#672

Temporary solution: Until such time as ijar is fixed for Kotlin, this adds a toggle that enables/disables ijar on java_import. This should unblock use of java_import for libraries that might contain Kotlin, so implementations can reunify.

It also restores java_import to a state where it works correctly by default. Per the user manual, ijars are a build performance optimization to allow caching of actions that use JARs whose implementations change frequenly [1]. Imported (externally compiled) JARs shouldn't be changing very often, meaning that the build performance cost of disabling ijars for these prebuilt JARs should be relatively low. Therefore, the ijar toggle is off by default, so the build is correct by default. But ijar is still made available though the toggle, just in case someone is importing a Java-interface-only JAR that they change all the time.

[1] https://docs.bazel.build/versions/main/user-manual.html#flag--use_ijars
  • Loading branch information
cpsauer committed Mar 5, 2022
1 parent e33b54e commit 14d9dec
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider.JavaOutput;
import com.google.devtools.build.lib.packages.Type;
import java.util.LinkedHashSet;
import java.util.Set;

Expand Down Expand Up @@ -202,7 +203,8 @@ private ImmutableList<Artifact> processWithIjarIfNeeded(
RuleContext ruleContext,
ImmutableMap.Builder<Artifact, Artifact> compilationToRuntimeJarMap) {
ImmutableList.Builder<Artifact> interfaceJarsBuilder = ImmutableList.builder();
boolean useIjar = ruleContext.getFragment(JavaConfiguration.class).getUseIjars();
boolean useIjar = ruleContext.getFragment(JavaConfiguration.class).getUseIjars()
&& ruleContext.attributes().get("use_ijar", Type.BOOLEAN);
for (Artifact jar : jars) {
Artifact interfaceJar =
useIjar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
for IDE plug-ins or <code>tools.jar</code> for anything running on
a standard JDK.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("use_ijar", BOOLEAN).value(false))
/* <!-- #BLAZE_RULE($java_import_base).ATTRIBUTE(use_ijar) -->
If you import an oft-changing JAR that doesn't have a Kotlin interface,
you can enable this as a caching optimization.
Temporarily available until ijar is updated to properly handle Kotlin.
For more, see https://github.com/bazelbuild/bazel/issues/4549.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("neverlink", BOOLEAN).value(false))
/* <!-- #BLAZE_RULE($java_import_base).ATTRIBUTE(constraints) -->
Extra constraints imposed on this rule as a Java library.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,12 @@ public final void writeBuildFile() throws Exception {
scratch.file(
"java/jarlib/BUILD",
"java_import(name = 'libraryjar',",
" jars = ['library.jar'])",
" jars = ['library.jar'],",
" use_ijar = True)",
"java_import(name = 'libraryjar_with_srcjar',",
" jars = ['library.jar'],",
" srcjar = 'library.srcjar')");
" srcjar = 'library.srcjar',",
" use_ijar = True)");
}

@Test
Expand Down Expand Up @@ -130,11 +132,14 @@ public void testDeps() throws Exception {
" jars = ['import.jar'],",
" deps = ['//java/jarlib2:depjar'],",
" exports = ['//java/jarlib2:exportjar'],",
" use_ijar = True",
")",
"java_import(name = 'depjar',",
" jars = ['depjar.jar'])",
" jars = ['depjar.jar'],",
" use_ijar = True)",
"java_import(name = 'exportjar',",
" jars = ['exportjar.jar'])");
" jars = ['exportjar.jar'],",
" use_ijar = True)");

ConfiguredTarget importJar = getConfiguredTarget("//java/jarlib2:import-jar");

Expand Down Expand Up @@ -210,7 +215,8 @@ public void testFromGenrule() throws Exception {
"java_import(name = 'library-jar',",
" jars = [':generated_jar'],",
" srcjar = ':generated_src_jar',",
" exports = ['//java/jarlib:libraryjar'])");
" exports = ['//java/jarlib:libraryjar'],",
" use_ijar = True)");
ConfiguredTarget jarLib = getConfiguredTarget("//java/genrules:library-jar");

JavaCompilationArgsProvider compilationArgs =
Expand Down Expand Up @@ -441,7 +447,8 @@ public void testTransitiveDependencies() throws Exception {
" deps = ['//java/jarlib:libraryjar'])",
"java_import(name = 'library2-jar',",
" jars = ['library2.jar'],",
" exports = [':lib'])",
" exports = [':lib'],",
" use_ijar = True)",
"java_library(name = 'javalib2',",
" srcs = ['Other.java'],",
" deps = [':library2-jar'])");
Expand All @@ -464,6 +471,7 @@ public void testRuntimeDepsAreNotOnClasspath() throws Exception {
" name = 'import_dep',",
" jars = ['import_compile.jar'],",
" runtime_deps = ['import_runtime.jar'],",
" use_ijar = True",
")",
"java_library(",
" name = 'library_dep',",
Expand Down

0 comments on commit 14d9dec

Please sign in to comment.