Skip to content

Commit

Permalink
Merge pull request #24534 from aloubyansky/module-compile-order
Browse files Browse the repository at this point in the history
Make sure modules are compiled in the correct order in dev mode
  • Loading branch information
aloubyansky authored Mar 25, 2022
2 parents a0d6053 + 8e7c98a commit a728d8c
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import io.quarkus.deployment.pkg.builditem.ProcessInheritIODisabled;
import io.quarkus.deployment.pkg.builditem.ProcessInheritIODisabledBuildItem;
import io.quarkus.deployment.steps.LocaleProcessor;
import io.quarkus.maven.dependency.GACTV;
import io.quarkus.maven.dependency.ResolvedDependency;
import io.quarkus.runtime.LocalesBuildTimeConfig;

Expand Down Expand Up @@ -313,7 +312,7 @@ private void copyJarSourcesToLib(OutputTargetBuildItem outputTargetBuildItem,
}

for (ResolvedDependency depArtifact : curateOutcomeBuildItem.getApplicationModel().getRuntimeDependencies()) {
if (depArtifact.getType().equals(GACTV.TYPE_JAR)) {
if (depArtifact.isJar()) {
for (Path resolvedDep : depArtifact.getResolvedPaths()) {
if (!Files.isDirectory(resolvedDep)) {
// Do we need to handle transformed classes?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ public interface ArtifactCoords {

ArtifactKey getKey();

default boolean isJar() {
return TYPE_JAR.equals(getType());
}

default String toGACTVString() {
return getGroupId() + ":" + getArtifactId() + ":" + getClassifier() + ":" + getType() + ":" + getVersion();
}
Expand All @@ -28,7 +32,7 @@ default String toCompactCoords() {
if (!getClassifier().isEmpty()) {
b.append(getClassifier()).append(':');
}
if (!TYPE_JAR.equals(getType())) {
if (!isJar()) {
b.append(getType()).append(':');
}
b.append(getVersion());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import io.quarkus.bootstrap.model.AppModel;
import io.quarkus.bootstrap.model.ApplicationModel;
import io.quarkus.bootstrap.util.BootstrapUtils;
import io.quarkus.maven.dependency.ArtifactCoords;
import io.quarkus.maven.dependency.ArtifactKey;
import io.quarkus.maven.dependency.ResolvedDependency;
import io.quarkus.paths.OpenPathTree;
Expand Down Expand Up @@ -150,7 +149,7 @@ private Object runInCl(String consumerName, Map<String, Object> params, QuarkusC
}

private synchronized void processCpElement(ResolvedDependency artifact, Consumer<ClassPathElement> consumer) {
if (!artifact.getType().equals(ArtifactCoords.TYPE_JAR)) {
if (!artifact.isJar()) {
//avoid the need for this sort of check in multiple places
consumer.accept(ClassPathElement.EMPTY);
return;
Expand Down Expand Up @@ -337,8 +336,7 @@ public QuarkusClassLoader createDeploymentClassLoader() {
}
}
for (ResolvedDependency dependency : appModel.getDependencies()) {
if (dependency.isRuntimeCp() &&
dependency.getType().equals(ArtifactCoords.TYPE_JAR) &&
if (dependency.isRuntimeCp() && dependency.isJar() &&
(dependency.isReloadable() && appModel.getReloadableWorkspaceDependencies().contains(dependency.getKey()) ||
configuredClassLoading.reloadableArtifacts.contains(dependency.getKey()))) {
processCpElement(dependency, element -> addCpElement(builder, dependency, element));
Expand Down Expand Up @@ -375,8 +373,7 @@ public QuarkusClassLoader createRuntimeClassLoader(ClassLoader base, Map<String,
}
}
for (ResolvedDependency dependency : appModel.getDependencies()) {
if (dependency.isRuntimeCp() &&
dependency.getType().equals(ArtifactCoords.TYPE_JAR) &&
if (dependency.isRuntimeCp() && dependency.isJar() &&
(dependency.isReloadable() && appModel.getReloadableWorkspaceDependencies().contains(dependency.getKey()) ||
configuredClassLoading.reloadableArtifacts.contains(dependency.getKey()))) {
processCpElement(dependency, element -> addCpElement(builder, dependency, element));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,41 +1,100 @@
package io.quarkus.bootstrap.devmode;

import io.quarkus.bootstrap.model.ApplicationModel;
import io.quarkus.maven.dependency.GACTV;
import io.quarkus.maven.dependency.ArtifactKey;
import io.quarkus.maven.dependency.Dependency;
import io.quarkus.maven.dependency.ResolvedDependency;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.jboss.logging.Logger;

public class DependenciesFilter {

private static final Logger log = Logger.getLogger(DependenciesFilter.class);

public static List<ResolvedDependency> getReloadableModules(ApplicationModel appModel) {
final List<ResolvedDependency> reloadable = new ArrayList<>();
if (appModel.getApplicationModule() != null) {
reloadable.add(appModel.getAppArtifact());
}
final Map<ArtifactKey, WorkspaceDependencies> modules = new HashMap<>();
appModel.getDependencies().forEach(d -> {
if (d.isReloadable()) {
reloadable.add(d);
modules.put(d.getKey(), new WorkspaceDependencies(d));
} else if (d.isWorkspaceModule()) {
//if this project also contains Quarkus extensions we do no want to include these in the discovery
//a bit of an edge case, but if you try and include a sample project with your extension you will
//run into problems without this
final StringBuilder msg = new StringBuilder();
msg.append("Local Quarkus extension dependency ");
msg.append(d.getGroupId()).append(":").append(d.getArtifactId()).append(":");
if (!d.getClassifier().isEmpty()) {
msg.append(d.getClassifier()).append(":");
}
if (!GACTV.TYPE_JAR.equals(d.getType())) {
msg.append(d.getType()).append(":");
}
msg.append(d.getVersion()).append(" will not be hot-reloadable");
msg.append("Local Quarkus extension dependency ").append(d.toCompactCoords())
.append(" will not be hot-reloadable");
log.warn(msg.toString());
}
});
return reloadable;
if (modules.isEmpty()) {
return appModel.getApplicationModule() == null ? List.of() : List.of(appModel.getAppArtifact());
}

if (appModel.getApplicationModule() != null) {
modules.put(appModel.getAppArtifact().getKey(), new WorkspaceDependencies(appModel.getAppArtifact()));
}

// Here we are sorting the reloadable dependencies according to their interdependencies to make sure
// they are compiled in the correct order

for (WorkspaceDependencies ad : modules.values()) {
for (Dependency dd : ad.artifact.getWorkspaceModule().getDirectDependencies()) {
final WorkspaceDependencies dep = modules.get(dd.getKey());
if (dep != null) {
ad.addDependency(dep);
}
}
}

final List<ResolvedDependency> sorted = new ArrayList<>();
int toBeSorted;
do {
toBeSorted = 0;
for (WorkspaceDependencies ad : modules.values()) {
if (ad.sorted) {
continue;
}
if (ad.hasNotSortedDependencies()) {
++toBeSorted;
continue;
}
ad.sorted = true;
sorted.add(ad.artifact);
}
} while (toBeSorted > 0);

return sorted;
}

private static class WorkspaceDependencies {
final ResolvedDependency artifact;
List<WorkspaceDependencies> deps;
boolean sorted;

WorkspaceDependencies(ResolvedDependency artifact) {
this.artifact = artifact;
}

boolean hasNotSortedDependencies() {
if (deps == null) {
return false;
}
for (WorkspaceDependencies d : deps) {
if (!d.sorted) {
return true;
}
}
return false;
}

void addDependency(WorkspaceDependencies dep) {
if (deps == null) {
deps = new ArrayList<>();
}
deps.add(dep);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,8 @@ public WorkspaceModule toWorkspaceModule() {
moduleBuilder.setDependencyConstraints(getRawModel().getDependencyManagement() == null ? Collections.emptyList()
: toArtifactDependencies(getRawModel().getDependencyManagement().getDependencies()));

moduleBuilder.setDependencies(toArtifactDependencies(getRawModel().getDependencies()));
final Model model = modelBuildingResult == null ? rawModel : modelBuildingResult.getEffectiveModel();
moduleBuilder.setDependencies(toArtifactDependencies(model.getDependencies()));

return this.module = moduleBuilder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1194,6 +1194,39 @@ public void testThatDependencyInParentIsEvaluated() throws IOException, MavenInv
runAndCheck();
}

@Test
public void testModuleCompileOrder() throws IOException, MavenInvocationException {
testDir = initProject("projects/multimodule-parent-dep", "projects/multimodule-compile-order");
runAndCheck("-Dquarkus.bootstrap.effective-model-builder");

assertThat(DevModeTestUtils.getHttpResponse("/app/hello/")).isEqualTo("hello");

// modify classes in all the modules and make sure they are compiled in a correct order
File resource = new File(testDir, "level0/src/main/java/org/acme/level0/Level0Service.java");
filter(resource, Collections.singletonMap("getGreeting()", "getGreeting(String name)"));
filter(resource, Collections.singletonMap("return greeting;", "return greeting + \" \" + name;"));

resource = new File(testDir, "level1/src/main/java/org/acme/level1/Level1Service.java");
filter(resource, Collections.singletonMap("getGreetingFromLevel0()", "getGreetingFromLevel0(String name)"));
filter(resource, Collections.singletonMap("level0Service.getGreeting()", "level0Service.getGreeting(name)"));

resource = new File(testDir, "level2/submodule/src/main/java/org/acme/level2/submodule/Level2Service.java");
filter(resource, Collections.singletonMap("getGreetingFromLevel1()", "getGreetingFromLevel1(String name)"));
filter(resource,
Collections.singletonMap("level1Service.getGreetingFromLevel0()", "level1Service.getGreetingFromLevel0(name)"));

resource = new File(testDir, "runner/src/main/java/org/acme/rest/HelloResource.java");
filter(resource, Collections.singletonMap("level0Service.getGreeting()",
"level0Service.getGreeting(\"world\")"));
filter(resource, Collections.singletonMap("level2Service.getGreetingFromLevel1()",
"level2Service.getGreetingFromLevel1(\"world\")"));

await()
.pollDelay(300, TimeUnit.MILLISECONDS)
.atMost(1, TimeUnit.MINUTES)
.until(() -> DevModeTestUtils.getHttpResponse("/app/hello").contains("hello world"));
}

@Test
public void testThatGenerateCodeGoalIsNotTriggeredIfNotConfigured() throws IOException, MavenInvocationException {
testDir = initProject("projects/classic-no-generate");
Expand Down

0 comments on commit a728d8c

Please sign in to comment.