From c61a3e271328a25926337485b11aefaf023d4207 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Mon, 3 Mar 2025 10:10:25 +1100 Subject: [PATCH] Use source file mapping for all compilation providers All compilation providers should be setting the source file attribute for use in debugging, there is no need to limit this to Java. This should allow for deletions of Kotlin classes to be handled correctly. --- .../deployment/dev/CompilationProvider.java | 19 ++++++- .../dev/JavaCompilationProvider.java | 54 ------------------- .../dev/RuntimeUpdatesClassVisitor.java | 43 +++++++++++++++ .../deployment/KotlinCompilationProvider.java | 8 --- ...sicKotlinApplicationModuleDevModeTest.java | 5 ++ .../devmode/QuarkusDevGradleTestBase.java | 14 +++++ 6 files changed, 80 insertions(+), 63 deletions(-) create mode 100644 core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesClassVisitor.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/CompilationProvider.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/CompilationProvider.java index 552e52fce6579..cec76f90a84d2 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/dev/CompilationProvider.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/dev/CompilationProvider.java @@ -3,8 +3,10 @@ import java.io.Closeable; import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; import java.util.HashMap; @@ -12,6 +14,8 @@ import java.util.Map; import java.util.Set; +import org.objectweb.asm.ClassReader; + import io.quarkus.paths.PathCollection; public interface CompilationProvider extends Closeable { @@ -28,7 +32,18 @@ default Set handledSourcePaths() { void compile(Set files, Context context); - Path getSourcePath(Path classFilePath, PathCollection sourcePaths, String classesPath); + default Path getSourcePath(Path classFilePath, PathCollection sourcePaths, String classesPath) { + Path sourceFilePath; + final RuntimeUpdatesClassVisitor visitor = new RuntimeUpdatesClassVisitor(sourcePaths, classesPath); + try (final InputStream inputStream = Files.newInputStream(classFilePath)) { + final ClassReader reader = new ClassReader(inputStream); + reader.accept(visitor, 0); + sourceFilePath = visitor.getSourceFileForClass(classFilePath); + } catch (IOException e) { + throw new RuntimeException(e); + } + return sourceFilePath; + } @Override default void close() throws IOException { @@ -159,5 +174,7 @@ public boolean ignoreModuleInfo() { public File getGeneratedSourcesDirectory() { return generatedSourcesDirectory; } + } + } diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/JavaCompilationProvider.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/JavaCompilationProvider.java index bdd951b05767a..eccaca9322934 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/dev/JavaCompilationProvider.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/dev/JavaCompilationProvider.java @@ -2,11 +2,7 @@ import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.nio.charset.Charset; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.List; import java.util.Set; import java.util.function.BiConsumer; @@ -20,14 +16,10 @@ import javax.tools.ToolProvider; import org.jboss.logging.Logger; -import org.objectweb.asm.ClassReader; -import org.objectweb.asm.ClassVisitor; import io.quarkus.deployment.dev.filesystem.QuarkusFileManager; import io.quarkus.deployment.dev.filesystem.ReloadableFileManager; import io.quarkus.deployment.dev.filesystem.StaticFileManager; -import io.quarkus.gizmo.Gizmo; -import io.quarkus.paths.PathCollection; public class JavaCompilationProvider implements CompilationProvider { @@ -115,20 +107,6 @@ public void compile(Set filesToCompile, CompilationProvider.Context contex } } - @Override - public Path getSourcePath(Path classFilePath, PathCollection sourcePaths, String classesPath) { - Path sourceFilePath; - final RuntimeUpdatesClassVisitor visitor = new RuntimeUpdatesClassVisitor(sourcePaths, classesPath); - try (final InputStream inputStream = Files.newInputStream(classFilePath)) { - final ClassReader reader = new ClassReader(inputStream); - reader.accept(visitor, 0); - sourceFilePath = visitor.getSourceFileForClass(classFilePath); - } catch (IOException e) { - throw new RuntimeException(e); - } - return sourceFilePath; - } - @Override public void close() throws IOException { if (this.fileManager != null) { @@ -154,36 +132,4 @@ private String extractCompilationErrorMessage(final DiagnosticCollector builder.append("\n").append(diagnostic)); return String.format("\u001B[91mCompilation Failed:%s\u001b[0m", builder); } - - private static class RuntimeUpdatesClassVisitor extends ClassVisitor { - private final PathCollection sourcePaths; - private final String classesPath; - private String sourceFile; - - public RuntimeUpdatesClassVisitor(PathCollection sourcePaths, String classesPath) { - super(Gizmo.ASM_API_VERSION); - this.sourcePaths = sourcePaths; - this.classesPath = classesPath; - } - - @Override - public void visitSource(String source, String debug) { - this.sourceFile = source; - } - - public Path getSourceFileForClass(final Path classFilePath) { - for (Path sourcesDir : sourcePaths) { - final Path classesDir = Paths.get(classesPath); - final StringBuilder sourceRelativeDir = new StringBuilder(); - sourceRelativeDir.append(classesDir.relativize(classFilePath.getParent())); - sourceRelativeDir.append(File.separator); - sourceRelativeDir.append(sourceFile); - final Path sourceFilePath = sourcesDir.resolve(Path.of(sourceRelativeDir.toString())); - if (Files.exists(sourceFilePath)) { - return sourceFilePath; - } - } - return null; - } - } } diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesClassVisitor.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesClassVisitor.java new file mode 100644 index 0000000000000..1c8fcd8a11bce --- /dev/null +++ b/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesClassVisitor.java @@ -0,0 +1,43 @@ +package io.quarkus.deployment.dev; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import org.objectweb.asm.ClassVisitor; + +import io.quarkus.gizmo.Gizmo; +import io.quarkus.paths.PathCollection; + +public class RuntimeUpdatesClassVisitor extends ClassVisitor { + private final PathCollection sourcePaths; + private final String classesPath; + private String sourceFile; + + public RuntimeUpdatesClassVisitor(PathCollection sourcePaths, String classesPath) { + super(Gizmo.ASM_API_VERSION); + this.sourcePaths = sourcePaths; + this.classesPath = classesPath; + } + + @Override + public void visitSource(String source, String debug) { + this.sourceFile = source; + } + + public Path getSourceFileForClass(final Path classFilePath) { + for (Path sourcesDir : sourcePaths) { + final Path classesDir = Paths.get(classesPath); + final StringBuilder sourceRelativeDir = new StringBuilder(); + sourceRelativeDir.append(classesDir.relativize(classFilePath.getParent())); + sourceRelativeDir.append(File.separator); + sourceRelativeDir.append(sourceFile); + final Path sourceFilePath = sourcesDir.resolve(Path.of(sourceRelativeDir.toString())); + if (Files.exists(sourceFilePath)) { + return sourceFilePath; + } + } + return null; + } +} diff --git a/extensions/kotlin/deployment/src/main/java/io/quarkus/kotlin/deployment/KotlinCompilationProvider.java b/extensions/kotlin/deployment/src/main/java/io/quarkus/kotlin/deployment/KotlinCompilationProvider.java index b397845ab08c1..60a37f4ef562d 100644 --- a/extensions/kotlin/deployment/src/main/java/io/quarkus/kotlin/deployment/KotlinCompilationProvider.java +++ b/extensions/kotlin/deployment/src/main/java/io/quarkus/kotlin/deployment/KotlinCompilationProvider.java @@ -1,7 +1,6 @@ package io.quarkus.kotlin.deployment; import java.io.File; -import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -22,7 +21,6 @@ import org.jetbrains.kotlin.config.Services; import io.quarkus.deployment.dev.CompilationProvider; -import io.quarkus.paths.PathCollection; public class KotlinCompilationProvider implements CompilationProvider { @@ -103,12 +101,6 @@ public void compile(Set filesToCompile, Context context) { } } - @Override - public Path getSourcePath(Path classFilePath, PathCollection sourcePaths, String classesPath) { - // return same class so it is not removed - return classFilePath; - } - private static class SimpleKotlinCompilerMessageCollector implements MessageCollector { private final List errors = new ArrayList<>(); diff --git a/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode/BasicKotlinApplicationModuleDevModeTest.java b/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode/BasicKotlinApplicationModuleDevModeTest.java index 80d3ca4602e14..f7f3da3d7f9ca 100644 --- a/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode/BasicKotlinApplicationModuleDevModeTest.java +++ b/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode/BasicKotlinApplicationModuleDevModeTest.java @@ -23,5 +23,10 @@ protected void testDevMode() throws Exception { ImmutableMap.of("return \"hello\"", "return \"" + uuid + "\"")); assertUpdatedResponseContains("/hello", uuid); + + delete("src/main/kotlin/org/acme/GreetingResource.kt"); + + assertStatusCode("/hello", 404); + } } diff --git a/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode/QuarkusDevGradleTestBase.java b/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode/QuarkusDevGradleTestBase.java index cad11217f4043..2e41d42ceb16e 100644 --- a/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode/QuarkusDevGradleTestBase.java +++ b/integration-tests/gradle/src/test/java/io/quarkus/gradle/devmode/QuarkusDevGradleTestBase.java @@ -22,6 +22,7 @@ import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.Test; +import org.wildfly.common.Assert; import io.quarkus.gradle.BuildResult; import io.quarkus.gradle.QuarkusGradleWrapperTestBase; @@ -180,6 +181,12 @@ protected void replace(String srcFile, Map tokens) { } } + protected void delete(String srcFile) { + final File source = new File(getProjectDir(), srcFile); + assertThat(source).exists(); + Assert.assertTrue(source.delete()); + } + protected void assertUpdatedResponseContains(String path, String value) { assertUpdatedResponseContains(path, value, devModeTimeoutSeconds(), TimeUnit.SECONDS); } @@ -198,4 +205,11 @@ protected void assertUpdatedResponseContains(String path, String value, long wai .pollDelay(100, TimeUnit.MILLISECONDS) .atMost(waitAtMost, timeUnit).until(() -> getHttpResponse(path, waitAtMost, timeUnit).contains(value)); } + + protected void assertStatusCode(String path, int code) { + await() + .pollDelay(100, TimeUnit.MILLISECONDS) + .atMost(devModeTimeoutSeconds(), TimeUnit.SECONDS) + .until(() -> Assert.assertTrue(devModeClient.getStrictHttpResponse(path, code))); + } }