Skip to content

Commit

Permalink
Enable not building unnecessary jars by default (#6685)
Browse files Browse the repository at this point in the history
* Enable optimizing built jars by default

* restore ijwb.bazelproject
  • Loading branch information
mai93 authored Aug 30, 2024
1 parent 41a3a10 commit 5884f07
Show file tree
Hide file tree
Showing 15 changed files with 116 additions and 331 deletions.
78 changes: 78 additions & 0 deletions MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 7 additions & 27 deletions aspect/intellij_info_impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -204,20 +204,14 @@ def annotation_processing_jars(generated_class_jar, generated_source_jar):
source_jars = [artifact_location(src_jar)] if src_jar else None,
)

def jars_from_output(output, should_optimize_building_jars):
def jars_from_output(output):
"""Collect jars for intellij-resolve-files from Java output."""
if output == None:
return []
source_jars = get_source_jars(output)
if should_optimize_building_jars:
return [
jar
for jar in ([output.ijar if len(source_jars) > 0 and output.ijar else output.class_jar] + source_jars)
if jar != None and not jar.is_source
]
return [
jar
for jar in ([output.class_jar, output.ijar] + source_jars)
for jar in ([output.ijar if len(source_jars) > 0 and output.ijar else output.class_jar] + source_jars)
if jar != None and not jar.is_source
]

Expand Down Expand Up @@ -617,11 +611,6 @@ def _collect_generated_files(java):
return [(java.annotation_processing.class_jar, java.annotation_processing.source_jar)]
return []

def _should_optimize_building_jars(ctx):
if ctx.attr.optimize_building_jars == "enabled":
return True
return False

def collect_java_info(target, ctx, semantics, ide_info, ide_info_file, output_groups):
"""Updates Java-specific output groups, returns false if not a Java target."""
java = get_java_provider(target)
Expand All @@ -642,7 +631,7 @@ def collect_java_info(target, ctx, semantics, ide_info, ide_info_file, output_gr
sources = sources_from_target(ctx)
jars = [library_artifact(output, ctx.rule.kind) for output in java_outputs]
class_jars = [output.class_jar for output in java_outputs if output and output.class_jar]
output_jars = [jar for output in java_outputs for jar in jars_from_output(output, _should_optimize_building_jars(ctx))]
output_jars = [jar for output in java_outputs for jar in jars_from_output(output)]
resolve_files = output_jars
compile_files = class_jars

Expand Down Expand Up @@ -785,20 +774,16 @@ def _build_filtered_gen_jar(ctx, target, java_outputs, gen_java_sources, srcjars
"""Filters the passed jar to contain only classes from the given manifest."""
jar_artifacts = []
source_jar_artifacts = []
should_optimize_building_jars = _should_optimize_building_jars(ctx)
for jar in java_outputs:
if jar.ijar:
jar_artifacts.append(jar.ijar)
elif jar.class_jar and not should_optimize_building_jars:
jar_artifacts.append(jar.class_jar)
if hasattr(jar, "source_jars") and jar.source_jars:
source_jar_artifacts.extend(_list_or_depset_to_list(jar.source_jars))
elif hasattr(jar, "source_jar") and jar.source_jar:
source_jar_artifacts.append(jar.source_jar)

if should_optimize_building_jars:
if len(source_jar_artifacts) == 0 or len(jar_artifacts) == 0:
jar_artifacts.extend([jar.class_jar for jar in java_outputs if jar.class_jar])
if len(source_jar_artifacts) == 0 or len(jar_artifacts) == 0:
jar_artifacts.extend([jar.class_jar for jar in java_outputs if jar.class_jar])

filtered_jar = ctx.actions.declare_file(target.label.name + "-filtered-gen.jar")
filtered_source_jar = ctx.actions.declare_file(target.label.name + "-filtered-gen-src.jar")
Expand Down Expand Up @@ -905,7 +890,7 @@ def _collect_android_ide_info(target, ctx, semantics, ide_info, ide_info_file, o

resources = []
res_folders = []
resolve_files = jars_from_output(output_jar, _should_optimize_building_jars(ctx))
resolve_files = jars_from_output(output_jar)
if hasattr(ctx.rule.attr, "resource_files"):
for artifact_path_fragments, res_files in get_res_artifacts(ctx.rule.attr.resource_files).items():
# Generate unique ArtifactLocation for resource directories.
Expand Down Expand Up @@ -977,7 +962,7 @@ def _collect_android_ide_info(target, ctx, semantics, ide_info, ide_info_file, o
# b/176209293: expose resource jar to make sure empty library
# knows they are remote output artifact
if android.resource_jar:
resolve_files += [jar for jar in jars_from_output(android.resource_jar, _should_optimize_building_jars(ctx))]
resolve_files += [jar for jar in jars_from_output(android.resource_jar)]

ide_info["android_ide_info"] = android_info
update_sync_output_groups(output_groups, "intellij-resolve-android", depset(resolve_files))
Expand Down Expand Up @@ -1289,11 +1274,6 @@ def make_intellij_info_aspect(aspect_impl, semantics):
executable = True,
allow_files = True,
),
"optimize_building_jars": attr.string(
doc = "Reduce the number of jars built by the aspect and tracked by the ide",
default = "disabled",
values = ["enabled", "disabled"],
),
}

# add attrs required by semantics
Expand Down
8 changes: 1 addition & 7 deletions aspect/testing/rules/intellij_aspect_test_fixture.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,16 @@ _intellij_aspect_test_fixture = rule(
executable = True,
allow_files = True,
),
"optimize_building_jars": attr.string(
doc = "Reduce the number of jars built by the aspect and tracked by the ide",
default = "disabled",
values = ["enabled", "disabled"],
),
},
)

def intellij_aspect_test_fixture(name, deps, transitive_configs = [], optimize_building_jars = "disabled"):
def intellij_aspect_test_fixture(name, deps, transitive_configs = []):
_intellij_aspect_test_fixture(
name = name,
output = name + ".intellij-aspect-test-fixture",
deps = deps,
testonly = 1,
transitive_configs = transitive_configs,
optimize_building_jars = optimize_building_jars,
)

def test_sources(outs):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,6 @@ java_library(

intellij_aspect_test_fixture(
name = "noide_fixture",
optimize_building_jars = "disabled",
deps = [
":baz",
":foo",
],
)

intellij_aspect_test_fixture(
name = "noide_fixture_optimize_building_jars",
optimize_building_jars = "enabled",
deps = [
":baz",
":foo",
Expand All @@ -46,7 +36,6 @@ java_test(
srcs = ["NoIdeTest.java"],
data = [
":noide_fixture",
":noide_fixture_optimize_building_jars",
],
deps = [
"//aspect/testing:BazelIntellijAspectTest",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,6 @@ public void testNoIde() throws Exception {
assertThat(findTarget(testFixture, ":bar")).isNull();
assertThat(findTarget(testFixture, ":baz")).isNull();

assertThat(getOutputGroupFiles(testFixture, "intellij-info-java"))
.containsAtLeast(
testRelative("foo.java-manifest"), testRelative(intellijInfoFileName("foo")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsExactly(
testRelative("libfoo.jar"),
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libfoo.jdeps"));
assertThat(getOutputGroupFiles(testFixture, "intellij-compile-java"))
.containsExactly(testRelative("libfoo.jar"));
}

@Test
public void testNoIdeOptimizedJars() throws Exception {
IntellijAspectTestFixture testFixture = loadTestFixture(":noide_fixture_optimize_building_jars");
assertThat(findTarget(testFixture, ":foo")).isNotNull();
assertThat(findTarget(testFixture, ":bar")).isNull();
assertThat(findTarget(testFixture, ":baz")).isNull();

assertThat(getOutputGroupFiles(testFixture, "intellij-info-java"))
.containsAtLeast(
testRelative("foo.java-manifest"), testRelative(intellijInfoFileName("foo")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,23 @@ public void testJavaLibraryWithTransitiveDependencies() throws Exception {
testRelative(intellijInfoFileName("single_dep")),
testRelative("transitive_dep.java-manifest"),
testRelative(intellijInfoFileName("transitive_dep")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))

var intellijResolveJava = getOutputGroupFiles(testFixture, "intellij-resolve-java");
assertThat(intellijResolveJava)
.containsAtLeast(
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libsingle_dep-hjar.jar"),
testRelative("libsingle_dep-src.jar"),
testRelative("libtransitive_dep-hjar.jar"),
testRelative("libtransitive_dep-src.jar"));
assertThat(intellijResolveJava)
.doesNotContain(testRelative("libfoo.jar"));
assertThat(intellijResolveJava)
.doesNotContain(testRelative("libsingle_dep.jar"));
assertThat(intellijResolveJava)
.doesNotContain(testRelative("libtransitive_dep.jar"));

assertThat(getOutputGroupFiles(testFixture, "intellij-compile-java"))
.containsAtLeast(
testRelative("libfoo.jar"),
Expand Down Expand Up @@ -98,14 +107,23 @@ public void testJavaLibraryWithExports() throws Exception {
testRelative(intellijInfoFileName("foo_exporter")),
testRelative("export_consumer.java-manifest"),
testRelative(intellijInfoFileName("export_consumer")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))

var intellijResolveJava = getOutputGroupFiles(testFixture, "intellij-resolve-java");
assertThat(intellijResolveJava)
.containsAtLeast(
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libfoo_exporter-hjar.jar"),
testRelative("libfoo_exporter-src.jar"),
testRelative("libexport_consumer-hjar.jar"),
testRelative("libexport_consumer-src.jar"));
assertThat(intellijResolveJava)
.doesNotContain(testRelative("libfoo.jar"));
assertThat(intellijResolveJava)
.doesNotContain(testRelative("libfoo_exporter.jar"));
assertThat(intellijResolveJava)
.doesNotContain(testRelative("libexport_consumer.jar"));

assertThat(getOutputGroupFiles(testFixture, "intellij-compile-java"))
.containsAtLeast(
testRelative("libfoo.jar"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,12 @@ public void testJavaBinary() throws Exception {
testRelative(intellijInfoFileName("foo")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsAtLeast(
testRelative("libfoolib.jar"),
testRelative("libfoolib-hjar.jar"),
testRelative("libfoolib-src.jar"),
testRelative("foo.jar"),
testRelative("foo-src.jar"));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.doesNotContain(testRelative("libfoolib.jar"));
assertThat(getOutputGroupFiles(testFixture, "intellij-compile-java"))
.containsExactly(testRelative("libfoolib.jar"), testRelative("foo.jar"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ java_library(

intellij_aspect_test_fixture(
name = "foo_fixture",
optimize_building_jars = "disabled",
deps = [":foo"],
)

intellij_aspect_test_fixture(
name = "foo_fixture_optimize_building_jars",
optimize_building_jars = "enabled",
deps = [":foo"],
)

Expand All @@ -63,7 +56,6 @@ java_library(

intellij_aspect_test_fixture(
name = "foo_exports_fixture",
optimize_building_jars = "disabled",
deps = [":foo_exports"],
)

Expand All @@ -73,7 +65,6 @@ java_test(
data = [
":foo_exports_fixture",
":foo_fixture",
":foo_fixture_optimize_building_jars",
],
deps = [
"//aspect/testing:BazelIntellijAspectTest",
Expand Down
Loading

0 comments on commit 5884f07

Please sign in to comment.