Skip to content

Commit

Permalink
Fix dev mode compile issues
Browse files Browse the repository at this point in the history
If a file has a compile problem, and then another file is modified
that compiles correctly dev mode will restart and ignore the original
compile error.

This changes the compilation so that the timestamps are only updated
on a sucessful compile, so files that failed to compile will always
be recompiled.

(cherry picked from commit 0fcde15)
  • Loading branch information
stuartwdouglas authored and gsmet committed Jun 23, 2021
1 parent 8cdda6b commit 1bbbe60
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,9 @@ private ClassScanResult compileTestClasses() {
testRunner.testCompileFailed(compileProblem);
compileProblem = null; //we don't want to block the app over a test problem
} else {
testRunner.testCompileSucceeded();
if (changedTestClassResult.isChanged()) {
testRunner.testCompileSucceeded();
}
}
} catch (Throwable e) {
testRunner.testCompileFailed(e);
Expand Down Expand Up @@ -558,7 +560,7 @@ ClassScanResult checkForChangedTestClasses(boolean firstScan) {
* This is useful in two ways.
* - To make sure that source time stamps have been recorded at least once
* - To avoid re-compiling on first run by ignoring all first time changes detected by
* {@link RuntimeUpdatesProcessor#checkIfFileModified(Path, Map, boolean)} during the first scan.
* {@link RuntimeUpdatesProcessor#checkIfFileModified(Path, Map, boolean, boolean)} during the first scan.
*/
ClassScanResult checkForChangedClasses(QuarkusCompiler compiler,
Function<DevModeContext.ModuleInfo, DevModeContext.CompilationUnit> cuf, boolean firstScan,
Expand All @@ -579,7 +581,7 @@ ClassScanResult checkForChangedClasses(QuarkusCompiler compiler,
changedSourceFiles = sourcesStream
.parallel()
.filter(p -> matchingHandledExtension(p).isPresent()
&& sourceFileWasRecentModified(p, ignoreFirstScanChanges))
&& sourceFileWasRecentModified(p, ignoreFirstScanChanges, firstScan))
.map(Path::toFile)
//Needing a concurrent Set, not many standard options:
.collect(Collectors.toCollection(ConcurrentSkipListSet::new));
Expand Down Expand Up @@ -643,6 +645,7 @@ && sourceFileWasRecentModified(p, ignoreFirstScanChanges))
//now we re-update the underlying timestamps, to the values we just compiled
//if the file has changed in the meantime it will be picked up in the next
//scan
//note that if compile failed these are not updated, so failing files will always be re-compiled
for (Map.Entry<File, Long> entry : compileTimestamps.entrySet()) {
sourceFileTimestamps.put(entry.getKey().toPath(), entry.getValue());
}
Expand Down Expand Up @@ -881,13 +884,13 @@ Set<String> checkForFileChange(Function<DevModeContext.ModuleInfo, DevModeContex
return ret;
}

private boolean sourceFileWasRecentModified(final Path sourcePath, boolean ignoreFirstScanChanges) {
return checkIfFileModified(sourcePath, sourceFileTimestamps, ignoreFirstScanChanges);
private boolean sourceFileWasRecentModified(final Path sourcePath, boolean ignoreFirstScanChanges, boolean firstScan) {
return checkIfFileModified(sourcePath, sourceFileTimestamps, ignoreFirstScanChanges, firstScan);
}

private boolean classFileWasRecentModified(final Path classFilePath, boolean ignoreFirstScanChanges,
TimestampSet timestampSet) {
return checkIfFileModified(classFilePath, timestampSet.classFileChangeTimeStamps, ignoreFirstScanChanges);
return checkIfFileModified(classFilePath, timestampSet.classFileChangeTimeStamps, ignoreFirstScanChanges, true);
}

private boolean classFileWasAdded(final Path classFilePath, boolean ignoreFirstScanChanges, TimestampSet timestampSet) {
Expand All @@ -902,18 +905,23 @@ private boolean classFileWasAdded(final Path classFilePath, boolean ignoreFirstS
return lastRecordedChange == null && !ignoreFirstScanChanges;
}

private boolean checkIfFileModified(Path path, Map<Path, Long> pathModificationTimes, boolean ignoreFirstScanChanges) {
private boolean checkIfFileModified(Path path, Map<Path, Long> pathModificationTimes, boolean ignoreFirstScanChanges,
boolean updateTimestamp) {
try {
final long lastModificationTime = Files.getLastModifiedTime(path).toMillis();
final Long lastRecordedChange = pathModificationTimes.get(path);

if (lastRecordedChange == null) {
pathModificationTimes.put(path, lastModificationTime);
if (updateTimestamp) {
pathModificationTimes.put(path, lastModificationTime);
}
return !ignoreFirstScanChanges;
}

if (lastRecordedChange != lastModificationTime) {
pathModificationTimes.put(path, lastModificationTime);
if (updateTimestamp) {
pathModificationTimes.put(path, lastModificationTime);
}
return true;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.quarkus.vertx.http.devmode;

import javax.enterprise.event.Observes;

import io.vertx.ext.web.Router;

public class CompileCorrectlyEndpoint {

void addConfigRoute(@Observes Router router) {
router.route("/correct")
.produces("text/plain")
.handler(rc -> rc.response().end("correct"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package io.quarkus.vertx.http.devmode;

import javax.enterprise.event.Observes;

import io.vertx.ext.web.Router;

public class CompileErrorEndpoint {

void addConfigRoute(@Observes Router router) {
router.route("/error")
.produces("text/plain")
.handler(rc -> rc.response().end("error"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package io.quarkus.vertx.http.devmode;

import static org.hamcrest.Matchers.equalTo;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusDevModeTest;
import io.restassured.RestAssured;

/**
* Tests that once a file has a compile error restart will not happen until it is fixed, even if
* other files are subsequently modified that do compile.
*/
public class CompileProblemTest {

@RegisterExtension
static final QuarkusDevModeTest test = new QuarkusDevModeTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(CompileErrorEndpoint.class, CompileCorrectlyEndpoint.class));

@Test
public void test() {
RestAssured.get("/error").then().body(equalTo("error"));
RestAssured.get("/correct").then().body(equalTo("correct"));
test.modifySourceFile(CompileErrorEndpoint.class, s -> s.replace("\"error\"", "\"compile error"));
RestAssured.get("/error").then().statusCode(500);
RestAssured.get("/correct").then().statusCode(500);
test.modifySourceFile(CompileCorrectlyEndpoint.class, s -> s.replace("\"correct\"", "\"compiled correctly\""));
//make sure that we are still in an error state, as CompileErrorEndpoint is broken
RestAssured.get("/error").then().statusCode(500);
RestAssured.get("/correct").then().statusCode(500);
test.modifySourceFile(CompileErrorEndpoint.class, s -> s.replace("compile error", "compile error fixed\""));
RestAssured.get("/error").then().body(equalTo("compile error fixed"));
RestAssured.get("/correct").then().body(equalTo("compiled correctly"));
}
}

0 comments on commit 1bbbe60

Please sign in to comment.