From e96f91cb59ceba9b4e922ae8c8cd669bd4ca4e7d Mon Sep 17 00:00:00 2001 From: Alexey Loubyansky Date: Wed, 12 Feb 2020 17:43:32 +0100 Subject: [PATCH] A bit of a cleanup in the bootstrap code around maven resolver and workspace discovery: * a fallback to resolving an application model from the classpath was removed, if the execution flow reaches that point something is definitely wrong and most probably will end up failing later; * resolved workspace discovery in BootstrapAppModelFactory is cached now to avoid loading it multiple times for the same factory (helps optimize launching apps in dev mode). --- .../main/java/io/quarkus/maven/DevMojo.java | 41 ++--- .../bootstrap/BootstrapAppModelFactory.java | 162 ++++++------------ .../bootstrap/app/QuarkusBootstrap.java | 4 +- .../resolver/BootstrapAppModelResolver.java | 18 +- .../io/quarkus/test/QuarkusDevModeTest.java | 1 - .../test/junit/QuarkusTestExtension.java | 2 +- 6 files changed, 73 insertions(+), 155 deletions(-) diff --git a/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java b/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java index 2f9b54205..44ac9bfa6 100644 --- a/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java +++ b/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java @@ -48,16 +48,17 @@ import org.codehaus.plexus.util.xml.Xpp3Dom; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.collection.CollectRequest; import org.eclipse.aether.repository.RemoteRepository; import org.eclipse.aether.repository.WorkspaceReader; import org.eclipse.aether.resolution.ArtifactRequest; import org.eclipse.aether.resolution.ArtifactResolutionException; import org.eclipse.aether.resolution.ArtifactResult; +import org.eclipse.aether.resolution.DependencyRequest; import org.eclipse.aether.resolution.DependencyResult; +import org.eclipse.aether.util.artifact.JavaScopes; import org.twdata.maven.mojoexecutor.MojoExecutor; -import io.quarkus.bootstrap.resolver.maven.MavenArtifactResolver; -import io.quarkus.bootstrap.resolver.maven.MavenRepoInitializer; import io.quarkus.bootstrap.resolver.maven.workspace.LocalProject; import io.quarkus.dev.DevModeContext; import io.quarkus.dev.DevModeMain; @@ -561,33 +562,27 @@ public class DevMojo extends AbstractMojo { } else { localProject = LocalProject.loadWorkspace(outputDirectory.toPath()); for (LocalProject project : localProject.getSelfWithLocalDeps()) { - if (project.getClassesDir() != null) { - //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 - if (Files.exists(project.getClassesDir().resolve("META-INF/quarkus-extension.properties")) || - Files.exists(project.getClassesDir().resolve("META-INF/quarkus-build-steps.list"))) { - continue; - } + if (project.getClassesDir() != null && + //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 + (Files.exists(project.getClassesDir().resolve("META-INF/quarkus-extension.properties")) || + Files.exists(project.getClassesDir().resolve("META-INF/quarkus-build-steps.list")))) { + continue; } addProject(devModeContext, project); pomFiles.add(project.getDir().resolve("pom.xml")); } - repoSystem = MavenRepoInitializer.getRepositorySystem(repoSession.isOffline(), localProject.getWorkspace()); } - DefaultArtifact bootstrap = new DefaultArtifact("io.quarkus", "quarkus-development-mode", "jar", - pluginDef.getVersion()); - MavenArtifactResolver mavenArtifactResolver = MavenArtifactResolver.builder() - .setRepositorySystem(repoSystem) - .setRepositorySystemSession(repoSession) - .setRemoteRepositories(repos) - .build(); - DependencyResult cpRes = mavenArtifactResolver.resolveDependencies( - bootstrap, - Collections.emptyList(), Collections.emptyList()); - addToClassPaths(classPathManifest, devModeContext, - mavenArtifactResolver.resolve(bootstrap).getArtifact().getFile()); + final DefaultArtifact devModeJar = new DefaultArtifact("io.quarkus", "quarkus-development-mode", "jar", + pluginDef.getVersion()); + final DependencyResult cpRes = repoSystem.resolveDependencies(repoSession, + new DependencyRequest() + .setCollectRequest( + new CollectRequest() + .setRoot(new org.eclipse.aether.graph.Dependency(devModeJar, JavaScopes.RUNTIME)) + .setRepositories(repos))); for (ArtifactResult appDep : cpRes.getArtifactResults()) { addToClassPaths(classPathManifest, devModeContext, appDep.getArtifact().getFile()); diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/BootstrapAppModelFactory.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/BootstrapAppModelFactory.java index 94e556940..c855619c3 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/BootstrapAppModelFactory.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/BootstrapAppModelFactory.java @@ -2,29 +2,18 @@ package io.quarkus.bootstrap; import java.io.DataInputStream; import java.io.DataOutputStream; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; -import java.net.URL; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; -import java.util.Enumeration; -import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Properties; -import java.util.Set; -import java.util.jar.JarEntry; -import java.util.jar.JarInputStream; - import org.apache.maven.model.Dependency; import org.apache.maven.model.Model; import org.jboss.logging.Logger; @@ -76,9 +65,9 @@ public class BootstrapAppModelFactory { private Path appClasses; private List appCp = new ArrayList<>(0); - private boolean localProjectsDiscovery; + private Boolean localProjectsDiscovery; private Boolean offline; - private boolean enableClasspathCache = false; + private boolean enableClasspathCache; private boolean test; private boolean devMode; private AppModelResolver bootstrapAppModelResolver; @@ -89,6 +78,8 @@ public class BootstrapAppModelFactory { private AppArtifact appArtifact; private MavenArtifactResolver mavenArtifactResolver; + private LocalProject appClassesWorkspace; + private BootstrapAppModelFactory() { } @@ -112,7 +103,7 @@ public class BootstrapAppModelFactory { return this; } - public BootstrapAppModelFactory setLocalProjectsDiscovery(boolean localProjectsDiscovery) { + public BootstrapAppModelFactory setLocalProjectsDiscovery(Boolean localProjectsDiscovery) { this.localProjectsDiscovery = localProjectsDiscovery; return this; } @@ -153,13 +144,15 @@ public class BootstrapAppModelFactory { } public AppModelResolver getAppModelResolver() { + + if (bootstrapAppModelResolver != null) { + return bootstrapAppModelResolver; + } + if (appClasses == null) { + throw new IllegalArgumentException("Application classes path has not been set"); + } + try { - if (bootstrapAppModelResolver != null) { - return bootstrapAppModelResolver; - } - if (appClasses == null) { - throw new IllegalArgumentException("Application classes path has not been set"); - } if (!Files.isDirectory(appClasses)) { final MavenArtifactResolver mvn; if (mavenArtifactResolver == null) { @@ -167,13 +160,12 @@ public class BootstrapAppModelFactory { if (offline != null) { mvnBuilder.setOffline(offline); } - final LocalProject localProject = localProjectsDiscovery + final LocalProject localProject = isWorkspaceDiscoveryEnabled() ? LocalProject.loadWorkspace(Paths.get("").normalize().toAbsolutePath(), false) - : null; + : null; if (localProject != null) { mvnBuilder.setWorkspace(localProject.getWorkspace()); if (managingProject == null) { - //TODO: big hack, all this needs to be cleaned up managingProject = localProject.getAppArtifact(); } } @@ -187,11 +179,12 @@ public class BootstrapAppModelFactory { .setDevMode(devMode); } - final LocalProject localProject = loadProject(); - - final MavenArtifactResolver mvn; - if (mavenArtifactResolver == null) { + MavenArtifactResolver mvn = mavenArtifactResolver; + if (mvn == null) { final MavenArtifactResolver.Builder builder = MavenArtifactResolver.builder(); + final LocalProject localProject = isWorkspaceDiscoveryEnabled() + ? loadAppClassesWorkspace() + : null; if (localProject != null) { builder.setWorkspace(localProject.getWorkspace()); } @@ -199,8 +192,6 @@ public class BootstrapAppModelFactory { builder.setOffline(offline); } mvn = builder.build(); - } else { - mvn = mavenArtifactResolver; } return bootstrapAppModelResolver = new BootstrapAppModelResolver(mvn) .setTest(test) @@ -213,7 +204,6 @@ public class BootstrapAppModelFactory { public CurationResult resolveAppModel() throws BootstrapException { if (test || devMode) { //gradle tests and dev encode the result on the class path - final String serializedModel = System.getProperty(BootstrapConstants.SERIALIZED_APP_MODEL); if (serializedModel != null) { final Path p = Paths.get(serializedModel); @@ -238,13 +228,26 @@ public class BootstrapAppModelFactory { return createAppModelForJar(appClasses); } - final LocalProject localProject = loadProject(); + final LocalProject localProject = isWorkspaceDiscoveryEnabled() || enableClasspathCache + ? loadAppClassesWorkspace() + : LocalProject.load(appClasses, false); + LocalWorkspace workspace = null; + AppArtifact appArtifact = this.appArtifact; if (localProject == null) { log.warn("Unable to locate maven project, falling back to classpath discovery"); - return doClasspathDiscovery(); + if(appArtifact == null) { + throw new BootstrapException("Failed to determine the Maven artifact associated with the application"); + } + } else { + workspace = localProject.getWorkspace(); + if(appArtifact == null) { + appArtifact = localProject.getAppArtifact(); + } else if(!appArtifact.equals(localProject.getAppArtifact())) { + log.warn("Provided application artifact attributes " + appArtifact + + " do not match the actual project loaded from the disk " + localProject.getAppArtifact()); + } } - final LocalWorkspace workspace = localProject.getWorkspace(); try { Path cachedCpPath = null; if (workspace != null && enableClasspathCache) { @@ -256,21 +259,21 @@ public class BootstrapAppModelFactory { ObjectInputStream in = new ObjectInputStream(reader); return new CurationResult((AppModel) in.readObject()); } else { - debug("Cached deployment classpath has expired for %s", localProject.getAppArtifact()); + debug("Cached deployment classpath has expired for %s", appArtifact); } } else { debug("Unsupported classpath cache format in %s for %s", cachedCpPath, - localProject.getAppArtifact()); + appArtifact); } } catch (IOException e) { log.warn("Failed to read deployment classpath cache from " + cachedCpPath + " for " - + localProject.getAppArtifact(), e); + + appArtifact, e); } } } AppModelResolver appModelResolver = getAppModelResolver(); CurationResult curationResult = new CurationResult(appModelResolver - .resolveManagedModel(localProject.getAppArtifact(), Collections.emptyList(), managingProject)); + .resolveManagedModel(appArtifact, Collections.emptyList(), managingProject)); if (cachedCpPath != null) { Files.createDirectories(cachedCpPath.getParent()); try (DataOutputStream out = new DataOutputStream(Files.newOutputStream(cachedCpPath))) { @@ -284,68 +287,18 @@ public class BootstrapAppModelFactory { } return curationResult; } catch (Exception e) { - throw new BootstrapException("Failed to create the application model for " + localProject.getAppArtifact(), e); + throw new BootstrapException("Failed to create the application model for " + appArtifact, e); } } - private LocalProject loadProject() throws BootstrapException { - return localProjectsDiscovery || enableClasspathCache - ? LocalProject.loadWorkspace(appClasses, false) - : LocalProject.load(appClasses, false); + private boolean isWorkspaceDiscoveryEnabled() { + return localProjectsDiscovery == null ? test || devMode : localProjectsDiscovery; } - /** - * If no maven project is around do discovery based on the class path. - * - * This is used to run gradle tests, and allows them to run from both the IDE - * and the gradle test task - * - */ - private CurationResult doClasspathDiscovery() { - try { - AppModelResolver resolver = getAppModelResolver(); - - Set urls = new HashSet<>(); - //this is pretty yuck, but under JDK11 the URLClassLoader trick does not work - Enumeration manifests = Thread.currentThread().getContextClassLoader().getResources("META-INF/MANIFEST.MF"); - while (manifests.hasMoreElements()) { - URL url = manifests.nextElement(); - if (url.getProtocol().equals("jar")) { - String path = url.getPath(); - if (path.startsWith("file:")) { - path = path.substring(5, path.lastIndexOf('!')); - urls.add(new File(URLDecoder.decode(path, StandardCharsets.UTF_8.name())).toURI().toURL()); - } - } - } - List artifacts = new ArrayList<>(); - for (URL jarUrl : urls) { - try (JarInputStream file = new JarInputStream(jarUrl.openConnection().getInputStream())) { - JarEntry entry = file.getNextJarEntry(); - while (entry != null) { - if (entry.getName().endsWith("/pom.properties") && entry.getName().startsWith("META-INF/maven")) { - Properties p = new Properties(); - p.load(file); - AppArtifact artifact = new AppArtifact(p.getProperty("groupId"), - p.getProperty("artifactId"), - p.getProperty("classifier"), - "jar", - p.getProperty("version")); - artifact.setPath(Paths.get(jarUrl.toURI())); - artifacts.add( - new AppDependency(artifact, "compile")); - } - entry = file.getNextJarEntry(); - } - } - } - - //we now have our runtime time artifacts, lets resolve all their deps - AppModel model = resolver.resolveManagedModel(appArtifact, artifacts, managingProject); - return new CurationResult(model); - } catch (Exception e) { - throw new RuntimeException(e); - } + private LocalProject loadAppClassesWorkspace() throws BootstrapException { + return appClassesWorkspace == null + ? appClassesWorkspace = LocalProject.loadWorkspace(appClasses, false) + : appClassesWorkspace; } private CurationResult createAppModelForJar(Path appArtifactPath) { @@ -361,17 +314,7 @@ public class BootstrapAppModelFactory { if (appArtifact == null) { appArtifact = ModelUtils.resolveAppArtifact(appArtifactPath); } - Path appJar; - try { - appJar = modelResolver.resolve(appArtifact); - } catch (AppModelResolverException e) { - throw new RuntimeException("Failed to resolve artifact", e); - } - if (!Files.exists(appJar)) { - throw new RuntimeException("Application " + appJar + " does not exist on disk"); - } - - modelResolver.relink(appArtifact, appJar); + modelResolver.relink(appArtifact, appArtifactPath); if (dependenciesOrigin == DependenciesOrigin.LAST_UPDATE) { log.info("Looking for the state of the last update"); @@ -397,15 +340,6 @@ public class BootstrapAppModelFactory { } catch (IOException e) { throw new RuntimeException("Failed to read application state " + statePath, e); } - /* - * final Properties props = model.getProperties(); final String appGroupId = - * props.getProperty(CurateOutcome.CREATOR_APP_GROUP_ID); final String appArtifactId = - * props.getProperty(CurateOutcome.CREATOR_APP_ARTIFACT_ID); final String appClassifier = - * props.getProperty(CurateOutcome.CREATOR_APP_CLASSIFIER); final String appType = - * props.getProperty(CurateOutcome.CREATOR_APP_TYPE); final String appVersion = - * props.getProperty(CurateOutcome.CREATOR_APP_VERSION); final AppArtifact modelAppArtifact = new - * AppArtifact(appGroupId, appArtifactId, appClassifier, appType, appVersion); - */ final List modelStateDeps = model.getDependencies(); final List updatedDeps = new ArrayList<>(modelStateDeps.size()); final String groupIdProp = "${" + CREATOR_APP_GROUP_ID + "}"; diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/QuarkusBootstrap.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/QuarkusBootstrap.java index f22b93c83..ead9b2a55 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/QuarkusBootstrap.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/QuarkusBootstrap.java @@ -61,7 +61,7 @@ public class QuarkusBootstrap implements Serializable { private final Mode mode; private final boolean offline; private final boolean test; - private final boolean localProjectDiscovery; + private final Boolean localProjectDiscovery; private final ClassLoader baseClassLoader; private final AppModelResolver appModelResolver; @@ -204,7 +204,7 @@ public class QuarkusBootstrap implements Serializable { Mode mode = Mode.PROD; boolean offline; boolean test; - boolean localProjectDiscovery; + Boolean localProjectDiscovery; Path targetDirectory; AppModelResolver appModelResolver; VersionUpdateNumber versionUpdateNumber = VersionUpdateNumber.MICRO; diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/resolver/BootstrapAppModelResolver.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/resolver/BootstrapAppModelResolver.java index 9a300f2b8..0f69f5bbc 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/resolver/BootstrapAppModelResolver.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/resolver/BootstrapAppModelResolver.java @@ -4,15 +4,10 @@ import java.io.File; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.function.Consumer; -import java.util.stream.Collectors; - import org.eclipse.aether.RepositoryException; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.artifact.DefaultArtifact; @@ -174,9 +169,7 @@ public class BootstrapAppModelResolver implements AppModelResolver { resolvedDeps = mvn.resolveManagedDependencies(toAetherArtifact(appArtifact), directMvnDeps, managedDeps, managedRepos, excludedScopes.toArray(new String[0])).getRoot(); } else { - //if there is no main artifact we assume we already have all the deps we need - //we just turn them into a DependencyNode - resolvedDeps = mvn.toDependencyTree(directMvnDeps, managedRepos).getRoot(); + throw new IllegalArgumentException("Application artifact is null"); } final TreeDependencyVisitor visitor = new TreeDependencyVisitor(new DependencyVisitor() { @@ -199,13 +192,10 @@ public class BootstrapAppModelResolver implements AppModelResolver { for (DependencyNode child : resolvedDeps.getChildren()) { child.accept(visitor); } - List repos; - if (appArtifact != null) { - repos = mvn.aggregateRepositories(managedRepos, + + final List repos = mvn.aggregateRepositories(managedRepos, mvn.newResolutionRepositories(mvn.resolveDescriptor(toAetherArtifact(appArtifact)).getRepositories())); - } else { - repos = managedRepos; - } + final DeploymentInjectingDependencyVisitor deploymentInjector = new DeploymentInjectingDependencyVisitor(mvn, managedDeps, repos, appBuilder); try { diff --git a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java index 2d4790d04..b15e12417 100644 --- a/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java +++ b/test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java @@ -150,7 +150,6 @@ public class QuarkusDevModeTest DevModeContext context = exportArchive(deploymentDir, projectSourceRoot); context.setTest(true); context.setAbortOnFailedStart(true); - context.setLocalProjectDiscovery(true); devModeMain = new DevModeMain(context); devModeMain.start(); } catch (Exception e) { diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java index 0ecf99774..5bdb19d4e 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/QuarkusTestExtension.java @@ -80,7 +80,7 @@ public class QuarkusTestExtension CuratedApplication curatedApplication = runnerBuilder .setTest(true) .setProjectRoot(new File("").toPath()) - .setLocalProjectDiscovery(true).build() + .build() .bootstrap(); Timing.staticInitStarted(curatedApplication.getBaseRuntimeClassLoader()); AugmentAction augmentAction = curatedApplication.createAugmentor(TestBuildChainFunction.class.getName(),