fix: jbang doesn't store additional deps in JAR Class-Path anymore (#708

) Fixes #707
jbangdev · Feb 5, 2021 · d41e4ae · d41e4ae
1 parent b276d7f
commit d41e4ae
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 30 deletions.
diff --git a/src/main/java/dev/jbang/dependencies/DependencyUtil.java b/src/main/java/dev/jbang/dependencies/DependencyUtil.java
@@ -15,6 +15,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.jboss.shrinkwrap.resolver.api.maven.ConfigurableMavenResolverSystem;
 import org.jboss.shrinkwrap.resolver.api.maven.Maven;
@@ -293,4 +294,12 @@ public void apply(ConfigurableMavenResolverSystem resolver) {
 			return new MavenRepo(repoid, reporef);
 		}
 	}
+
+	@SafeVarargs
+	public static List<String> joinClasspaths(List<String>... classpaths) {
+		return Stream	.of(classpaths)
+						.flatMap(x -> x.stream())
+						.distinct()
+						.collect(Collectors.toList());
+	}
 }
diff --git a/src/main/java/dev/jbang/dependencies/ModularClassPath.java b/src/main/java/dev/jbang/dependencies/ModularClassPath.java
@@ -24,25 +24,34 @@
 import dev.jbang.util.Util;
 
 public class ModularClassPath {
-
 	static final String JAVAFX_PREFIX = "javafx";
 
+	private final List<ArtifactInfo> artifacts;
+
+	private List<String> classPaths;
 	private String classPath;
 	private String manifestPath;
-	private final List<ArtifactInfo> artifacts;
 	private Optional<Boolean> javafx = Optional.empty();
 
 	public ModularClassPath(List<ArtifactInfo> artifacts) {
 		this.artifacts = artifacts;
 	}
 
-	public String getClassPath() {
-		if (classPath == null) {
-			classPath = artifacts	.stream()
+	public List<String> getClassPaths() {
+		if (classPaths == null) {
+			classPaths = artifacts	.stream()
 									.map(it -> it.asFile().getAbsolutePath())
 									.map(it -> it.contains(" ") ? '"' + it + '"' : it)
 									.distinct()
-									.collect(Collectors.joining(CP_SEPARATOR));
+									.collect(Collectors.toList());
+		}
+
+		return classPaths;
+	}
+
+	public String getClassPath() {
+		if (classPath == null) {
+			classPath = String.join(CP_SEPARATOR, getClassPaths());
 		}
 
 		return classPath;

diff --git a/src/main/java/dev/jbang/source/RunContext.java b/src/main/java/dev/jbang/source/RunContext.java
@@ -1,16 +1,18 @@
 package dev.jbang.source;
 
+import static dev.jbang.dependencies.DependencyUtil.joinClasspaths;
+
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import dev.jbang.Settings;
 import dev.jbang.catalog.Alias;
+import dev.jbang.cli.BaseCommand;
+import dev.jbang.cli.ExitException;
 import dev.jbang.dependencies.ModularClassPath;
 
 /**
@@ -43,7 +45,8 @@ public class RunContext {
 
 	private Alias alias;
 
-	private ModularClassPath classpath;
+	private ModularClassPath mcp;
+	private ModularClassPath additionalMcp;
 
 	public static RunContext empty() {
 		return new RunContext();
@@ -234,41 +237,36 @@ public List<String> collectAllDependenciesFor(Source src) {
 		if (getProperties() != null) {
 			p.putAll(getProperties());
 		}
-		return getAllDependencies(src, p);
-	}
-
-	private List<String> getAllDependencies(Source src, Properties props) {
-		return Stream	.concat(getAdditionalDependencies().stream(), src.getAllDependencies(props).stream())
-						.collect(Collectors.toList());
+		return src.getAllDependencies(p);
 	}
 
 	/**
 	 * Return resolved classpath lazily. resolution will only happen once, any
 	 * consecutive calls return the same classpath.
 	 **/
 	public String resolveClassPath(Source src, boolean offline) {
-		if (classpath == null) {
-			classpath = src.resolveClassPath(collectAllDependenciesFor(src), offline);
+		if (additionalMcp == null) {
+			additionalMcp = src.resolveClassPath(getAdditionalDependencies(), offline);
 		}
-		StringBuilder cp = new StringBuilder(classpath.getClassPath());
-		for (String addcp : getAdditionalClasspaths()) {
-			if (cp.length() > 0) {
-				cp.append(Settings.CP_SEPARATOR);
-			}
-			cp.append(addcp);
+		if (mcp == null) {
+			mcp = src.resolveClassPath(collectAllDependenciesFor(src), offline);
 		}
-		return cp.toString();
+		List<String> cp = joinClasspaths(additionalMcp.getClassPaths(), mcp.getClassPaths(), getAdditionalClasspaths());
+		return String.join(Settings.CP_SEPARATOR, cp);
 	}
 
 	public List<String> getAutoDetectedModuleArguments(Source src, String requestedVersion, boolean offline) {
-		if (classpath == null) {
+		if (mcp == null) {
 			resolveClassPath(src, offline);
 		}
-		return classpath.getAutoDectectedModuleArguments(requestedVersion);
+		return mcp.getAutoDectectedModuleArguments(requestedVersion);
 	}
 
 	public ModularClassPath getClassPath() {
-		return classpath;
+		if (mcp == null) {
+			throw new ExitException(BaseCommand.EXIT_INTERNAL_ERROR, "Classpath must be resolved first");
+		}
+		return mcp;
 	}
 
 	/**

diff --git a/src/test/java/dev/jbang/cli/TestRun.java b/src/test/java/dev/jbang/cli/TestRun.java
@@ -517,6 +517,7 @@ void testCreateJar(@TempDir Path rootdir) throws IOException {
 		RunContext ctx = RunContext.empty();
 		ctx.setMainClass("wonkabear");
 
+		ctx.resolveClassPath(src, true);
 		BaseBuildCommand.createJarFile(src, ctx, dir, out);
 
 		try (JarFile jf = new JarFile(out)) {

diff --git a/src/test/java/dev/jbang/dependencies/DependencyResolverTest.java b/src/test/java/dev/jbang/dependencies/DependencyResolverTest.java
@@ -148,7 +148,7 @@ void testResolveDependencies() {
 		ModularClassPath classpath = dr.resolveDependencies(deps, Collections.emptyList(), false, true);
 
 		// if returns 5 its because optional deps are included which they shouldn't
-		assertEquals(2, classpath.getClassPath().split(Settings.CP_SEPARATOR).length);
+		assertEquals(2, classpath.getClassPaths().size());
 
 	}
 
@@ -166,7 +166,7 @@ void testResolveDependenciesNoDuplicates() {
 		// if returns with duplicates its because some dependencies are multiple times
 		// in the
 		// classpath (commons-text-1.8, commons-lang3-3.9)
-		List<String> cps = Arrays.asList(classpath.getClassPath().split(Settings.CP_SEPARATOR));
+		List<String> cps = classpath.getClassPaths();
 
 		HashSet<String> othercps = new HashSet<>();
 		othercps.addAll(cps);
@@ -188,7 +188,7 @@ void testResolveNativeDependencies() {
 
 		ModularClassPath classpath = dr.resolveDependencies(deps, Collections.emptyList(), false, true);
 
-		assertEquals(46, classpath.getClassPath().split(Settings.CP_SEPARATOR).length);
+		assertEquals(46, classpath.getClassPaths().size());
 
 	}