From 71da5a1955dcae10377861e83bdf5de7005f9f80 Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Wed, 21 May 2025 18:37:11 +0200 Subject: [PATCH] better deps order for kotlin compiler; fix IC- data collection for kotlin GitOrigin-RevId: 8481762f19582dab91129a53adca5445a1f148ca --- build/jvm-rules/jvm-inc-builder/BUILD.bazel | 6 +-- .../bazel/jvmIncBuilder/BazelIncBuilder.java | 39 ++++++++++--------- .../jvmIncBuilder/impl/BuildContextImpl.java | 3 ++ .../impl/JavaCompilerRunner.java | 13 +++---- .../impl/KotlinCompilerRunner.java | 14 +++++-- .../jvmIncBuilder/runner/CompilerRunner.java | 2 +- 6 files changed, 42 insertions(+), 35 deletions(-) diff --git a/build/jvm-rules/jvm-inc-builder/BUILD.bazel b/build/jvm-rules/jvm-inc-builder/BUILD.bazel index d50db6b0f6cc..c845cfd3ded3 100644 --- a/build/jvm-rules/jvm-inc-builder/BUILD.bazel +++ b/build/jvm-rules/jvm-inc-builder/BUILD.bazel @@ -20,8 +20,6 @@ kt_jvm_library( "//dependency-graph", "//jps-builders-6:build-javac-rt", "//:jps-javac-extension", - "//src/worker-framework", - "//src/worker-util", "//:annotations", "//:h2-mvstore", "//:hash4j", @@ -30,10 +28,12 @@ kt_jvm_library( "//:caffeine", "//:intellij-deps-fastutil", "//:qdox", + "//:jps", # temporary dep for instrumentation-util and instrumenters, must be the last dep in classpath + "//src/worker-framework", + "//src/worker-util", "patched-kotlin-compiler-for-bazel", "//:kotlin-compose-compiler-plugin", "//:kotlin-serialization-compiler-plugin", - "//:jps", # temporary dep for instrumentation-util and instrumenters, must be the last dep in classpath ] ) diff --git a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/BazelIncBuilder.java b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/BazelIncBuilder.java index 727addbbbafb..7193aa927f15 100644 --- a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/BazelIncBuilder.java +++ b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/BazelIncBuilder.java @@ -161,7 +161,9 @@ public class BazelIncBuilder { if (!diagnostic.hasErrors()) { if (!srcSnapshotDelta.isRecompileAll()) { // delete outputs corresponding to deleted or recompiled sources - cleanOutputsForCompiledFiles(context, srcSnapshotDelta, storageManager.getGraph(), roundCompilers, outSink); + for (CompilerRunner compiler : roundCompilers) { + cleanOutputsForCompiledFiles(context, srcSnapshotDelta, storageManager.getGraph(), compiler, outSink); + } } for (CompilerRunner runner : roundCompilers) { @@ -256,38 +258,37 @@ public class BazelIncBuilder { } } - private static void cleanOutputsForCompiledFiles(BuildContext context, NodeSourceSnapshotDelta snapshotDelta, DependencyGraph depGraph, List roundCompilers, OutputSink outSink) { + private static void cleanOutputsForCompiledFiles(BuildContext context, NodeSourceSnapshotDelta snapshotDelta, DependencyGraph depGraph, CompilerRunner compiler, OutputSink outSink) { // separately logging deleted outputs for 'deleted' and 'modified' sources to adjust for existing test data - Iterable<@NotNull NodeSource> deleted = snapshotDelta.getDeleted(); - if (!isEmpty(deleted)) { - logDeletedPaths( - context, - deleteCompilerOutputs(depGraph, deleted, outSink, new ArrayList<>()) - ); - } + Collection cleanedOutputsOfDeletedSources = deleteCompilerOutputs( + depGraph, filter(snapshotDelta.getDeleted(), compiler::canCompile), outSink, new ArrayList<>() + ); + logDeletedPaths(context, cleanedOutputsOfDeletedSources); - Iterable<@NotNull NodeSource> modified = snapshotDelta.getModified(); - if (!isEmpty(modified)) { - Collection cleaned = deleteCompilerOutputs(depGraph, modified, outSink, new ArrayList<>()); - for (String toDelete : flat(map(roundCompilers, CompilerRunner::getPathsToDelete))) { + Collection cleanedOutputsOfModifiedSources = deleteCompilerOutputs( + depGraph, filter(snapshotDelta.getModified(), compiler::canCompile), outSink, new ArrayList<>() + ); + if (!cleanedOutputsOfDeletedSources.isEmpty() || !cleanedOutputsOfModifiedSources.isEmpty()) { + // delete additional paths only if there are any changes in the output caused by changes in sources + for (String toDelete : compiler.getOutputPathsToDelete()) { if (outSink.deletePath(toDelete)) { - cleaned.add(toDelete); + cleanedOutputsOfModifiedSources.add(toDelete); } } - logDeletedPaths(context, cleaned); } + logDeletedPaths(context, cleanedOutputsOfModifiedSources); } private static Collection deleteCompilerOutputs( - DependencyGraph depGraph, Iterable<@NotNull NodeSource> sourceGroup, OutputSink outSink, Collection deletedPaths + DependencyGraph depGraph, Iterable<@NotNull NodeSource> sourcesToCompile, OutputSink outSink, Collection deletedPathsAcc ) { - for (Node node : filter(flat(map(sourceGroup, depGraph::getNodes)), n -> n instanceof JVMClassNode)) { + for (Node node : filter(flat(map(sourcesToCompile, depGraph::getNodes)), n -> n instanceof JVMClassNode)) { String outputPath = ((JVMClassNode) node).getOutFilePath(); if (outSink.deletePath(outputPath)) { - deletedPaths.add(outputPath); + deletedPathsAcc.add(outputPath); } } - return deletedPaths; + return deletedPathsAcc; } private static void logDeletedPaths(BuildContext context, Iterable deletedPaths) { diff --git a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/BuildContextImpl.java b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/BuildContextImpl.java index 0180fcd85197..bb3e639d4bd2 100644 --- a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/BuildContextImpl.java +++ b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/BuildContextImpl.java @@ -142,6 +142,9 @@ public class BuildContextImpl implements BuildContext { private static @NotNull List buildJavaOptions(Map> flags) { // for now, only options available in the flags map can be specified in the build configuration List options = new ArrayList<>(); + options.add("-encoding"); // todo: for now hardcoded + options.add("UTF-8"); + String jvmTarget = CLFlags.JVM_TARGET.getOptionalScalarValue(flags); if (jvmTarget != null) { options.add("-source"); diff --git a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/JavaCompilerRunner.java b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/JavaCompilerRunner.java index 7edf09abf9f5..9b29cd5b3c24 100644 --- a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/JavaCompilerRunner.java +++ b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/JavaCompilerRunner.java @@ -18,11 +18,8 @@ import org.jetbrains.jps.javac.ast.api.JavacFileData; import org.jetbrains.jps.javac.ast.api.JavacRef; import javax.lang.model.element.Modifier; -import javax.tools.Diagnostic; -import javax.tools.JavaFileManager; -import javax.tools.JavaFileObject; +import javax.tools.*; import java.io.File; -import java.io.IOException; import java.util.*; import static org.jetbrains.jps.util.Iterators.*; @@ -249,16 +246,16 @@ public class JavaCompilerRunner implements CompilerRunner { if (diagnostic.getPosition() != Diagnostic.NOPOS) { msgBuilder.append(" (").append(diagnostic.getLineNumber()).append(":").append(diagnostic.getColumnNumber()).append(")"); try { - int start = (int) diagnostic.getStartPosition(); - int end = (int) diagnostic.getEndPosition(); - if (end > start) { + int start = (int)(diagnostic.getStartPosition()); + int end = (int)(diagnostic.getEndPosition()); + if (start >= 0 && end > start) { CharSequence charContent = source.getCharContent(true); if (end < charContent.length()) { msgBuilder.append("\ncode: \"").append(charContent.subSequence(start, end)).append("\""); } } } - catch (IOException ignored) { + catch (Throwable ignored) { } } } diff --git a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/KotlinCompilerRunner.java b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/KotlinCompilerRunner.java index 652e3d070393..8915097e3005 100644 --- a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/KotlinCompilerRunner.java +++ b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/impl/KotlinCompilerRunner.java @@ -107,7 +107,7 @@ public class KotlinCompilerRunner implements CompilerRunner { } @Override - public Iterable getPathsToDelete() { + public Iterable getOutputPathsToDelete() { return myModuleEntryPath != null? List.of(myModuleEntryPath) : List.of(); } @@ -121,9 +121,15 @@ public class KotlinCompilerRunner implements CompilerRunner { // todo: make sure if we really need to process generated outputs after the compilation and not "in place" List generatedClasses = new ArrayList<>(); AbstractCliPipeline pipeline = createPipeline(out, generatedFile -> { - if (generatedFile instanceof GeneratedJvmClass jvmClass) { - String jvmClassName = jvmClass.getOutputClass().getClassName().getInternalName(); - for (File sourceFile : jvmClass.getSourceFiles()) { + String jvmClassName = null; + if (generatedFile instanceof KotlinJvmGeneratedFile jvmClass) { + jvmClassName = jvmClass.getOutputClass().getClassName().getInternalName(); + } + else if (generatedFile instanceof GeneratedJvmClass jvmClass) { + jvmClassName = jvmClass.getOutputClass().getClassName().getInternalName(); + } + if (jvmClassName != null) { + for (File sourceFile : generatedFile.getSourceFiles()) { generatedClasses.add(new GeneratedClass(jvmClassName, sourceFile)); } } diff --git a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/runner/CompilerRunner.java b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/runner/CompilerRunner.java index 57fb38f4aaf2..c4039ade3194 100644 --- a/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/runner/CompilerRunner.java +++ b/build/jvm-rules/jvm-inc-builder/src/com/intellij/tools/build/bazel/jvmIncBuilder/runner/CompilerRunner.java @@ -19,7 +19,7 @@ public interface CompilerRunner extends Runner{ ExitCode compile(Iterable sources, Iterable deletedSources, DiagnosticSink diagnostic, OutputSink out); - default Iterable getPathsToDelete() { + default Iterable getOutputPathsToDelete() { return List.of(); }