Skip to content

Commit

Permalink
fix: Fixes dependent source discovery
Browse files Browse the repository at this point in the history
Parent directories for sources are now added to the compiler's source
path so any types referenced in them that are available in the same
directories can be found even when they are not explicitely mentioned
in any `//SOURCES` lines. This also fixes the problem where adding a
`//DEPS` line would cause the compiler to be unable to find those same
types.

Fixes #1502
  • Loading branch information
quintesse committed Nov 21, 2022
1 parent 192972e commit 28bdc4b
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 3 deletions.
3 changes: 3 additions & 0 deletions itests/extending/Bar.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package extending;

public class Bar {}
10 changes: 10 additions & 0 deletions itests/extending/Foo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
///usr/bin/env jbang "$0" "$@" ; exit $?
///DEPS com.github.lalyos:jfiglet:0.0.9

package extending;

public class Foo extends Bar {
public static void main(String... args) {
System.out.println("hello JBang");
}
}
2 changes: 1 addition & 1 deletion itests/test_suite.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ assert_stderr(){
export NL=$'\n'

echo "Cleaning JBANG_CACHE"
rm -rf ~/.jbang/cache
rm -rf ~/.jbang/cache/{jars,urls,scripts,stdins}

echo Testing with `which jbang`

Expand Down
6 changes: 5 additions & 1 deletion src/main/java/dev/jbang/dependencies/ModularClassPath.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,16 @@ public List<String> getClassPaths() {

public String getClassPath() {
if (classPath == null) {
classPath = String.join(CP_SEPARATOR, getClassPaths());
classPath = toClassPath(getClassPaths());
}

return classPath;
}

public static String toClassPath(List<String> pathElements) {
return String.join(CP_SEPARATOR, pathElements);
}

public String getManifestPath() {
if (manifestPath == null) {
manifestPath = artifacts.stream()
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/dev/jbang/source/ResourceRef.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ public boolean isStdin() {
return originalResource != null && isStdin(originalResource);
}

public boolean isFile() {
return originalResource != null && Util.isValidPath(originalResource);
}

@Nullable
public Path getFile() {
return file;
Expand Down
30 changes: 30 additions & 0 deletions src/main/java/dev/jbang/source/Source.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package dev.jbang.source;

import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import dev.jbang.cli.BaseCommand;
import dev.jbang.cli.ExitException;
Expand Down Expand Up @@ -87,6 +90,29 @@ public ResourceRef getResourceRef() {
return resourceRef;
}

@Nullable
public ResourceRef getSourceDirRef() {
if (contents != null && resourceRef.isFile()) {
Path parent = getResourceRef().getFile().getParent();
Optional<String> pkg = getJavaPackage();
if (pkg.isPresent()) {
String[] elems = pkg.get().split("\\.");
Collections.reverse(Arrays.asList(elems));
for (String elem : elems) {
if (parent != null && !elem.equals(parent.getFileName().toString())) {
// if path doesn't match package we return null
return null;
}
parent = parent.getParent();
}
}
return ResourceRef.forFile(parent);
} else {
// If the resource isn't a local file we return null
return null;
}
}

public Optional<String> getJavaPackage() {
if (contents != null) {
return Util.getSourcePackage(contents);
Expand Down Expand Up @@ -145,6 +171,10 @@ public Project updateProject(Project prj, ResourceResolver resolver) {
if (!prj.getMainSourceSet().getSources().contains(getResourceRef())) {
SourceSet ss = prj.getMainSourceSet();
ss.addSource(this.getResourceRef());
ResourceRef srcDir = getSourceDirRef();
if (srcDir != null) {
ss.addSourceDir(srcDir);
}
ss.addResources(tagReader.collectFiles(resourceRef,
new SiblingResourceResolver(resourceRef, ResourceResolver.forResources())));
ss.addDependencies(collectDependencies());
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/dev/jbang/source/SourceSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;

import javax.annotation.Nonnull;
Expand All @@ -21,6 +23,7 @@
public class SourceSet {
private final List<ResourceRef> sources = new ArrayList<>();
private final List<RefTarget> resources = new ArrayList<>();
private final Set<ResourceRef> sourceDirs = new HashSet<>();
private final List<String> dependencies = new ArrayList<>();
private final List<String> classPaths = new ArrayList<>();
private final List<String> compileOptions = new ArrayList<>();
Expand Down Expand Up @@ -67,6 +70,23 @@ public SourceSet addResources(Collection<RefTarget> resources) {
return this;
}

@Nonnull
public Set<ResourceRef> getSourceDirs() {
return Collections.unmodifiableSet(sourceDirs);
}

@Nonnull
public SourceSet addSourceDir(ResourceRef sourceDir) {
sourceDirs.add(sourceDir);
return this;
}

@Nonnull
public SourceSet addSourceDirs(Collection<ResourceRef> sourceDirs) {
this.sourceDirs.addAll(sourceDirs);
return this;
}

@Nonnull
public List<String> getDependencies() {
return Collections.unmodifiableList(dependencies);
Expand Down
15 changes: 14 additions & 1 deletion src/main/java/dev/jbang/source/buildsteps/CompileBuildStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import dev.jbang.cli.ExitException;
import dev.jbang.dependencies.MavenCoordinate;
import dev.jbang.dependencies.ModularClassPath;
import dev.jbang.source.Builder;
import dev.jbang.source.Project;
import dev.jbang.util.CommandBuffer;
Expand Down Expand Up @@ -42,10 +43,22 @@ protected Project compile() throws IOException {
optionList.addAll(project.getMainSourceSet().getCompileOptions());
String path = project.resolveClassPath().getClassPath();
if (!Util.isBlankString(path)) {
optionList.addAll(Arrays.asList("-classpath", path));
optionList.add("-classpath");
optionList.add(path);
}
optionList.addAll(Arrays.asList("-d", compileDir.toAbsolutePath().toString()));

// add -sourcepath for all source folders
List<String> srcDirs = project .getMainSourceSet()
.getSourceDirs()
.stream()
.map(d -> d.getFile().toString())
.collect(Collectors.toList());
if (!srcDirs.isEmpty()) {
optionList.add("-sourcepath");
optionList.add(ModularClassPath.toClassPath(srcDirs));
}

// add source files to compile
optionList.addAll(project .getMainSourceSet()
.getSources()
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/dev/jbang/source/generators/JshCmdGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.apache.commons.text.StringEscapeUtils;

import dev.jbang.dependencies.ModularClassPath;
import dev.jbang.source.*;
import dev.jbang.util.JavaUtil;
import dev.jbang.util.Util;
Expand Down Expand Up @@ -98,6 +99,17 @@ protected List<String> generateCommandLineList() throws IOException {
fullArgs.addAll(optionalArgs);

if (project.isJShell()) {
// add -sourcepath for all source folders
List<String> srcDirs = project .getMainSourceSet()
.getSourceDirs()
.stream()
.map(d -> d.getFile().toString())
.collect(Collectors.toList());
if (!srcDirs.isEmpty()) {
fullArgs.add("-C-sourcepath");
fullArgs.add(ModularClassPath.toClassPath(srcDirs));
}

ArrayList<ResourceRef> revSources = new ArrayList<>(project.getMainSourceSet().getSources());
Collections.reverse(revSources);
for (ResourceRef s : revSources) {
Expand Down
22 changes: 22 additions & 0 deletions src/test/java/dev/jbang/source/TestBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ void testAdditionalSourcesFolder() throws IOException {
Util.setCwd(examplesTestFolder);
String mainFile = examplesTestFolder.resolve("foo.java").toString();
String incFile = examplesTestFolder.resolve("bar/Bar.java").toString();
String srcPath = examplesTestFolder.resolve("bar") + File.pathSeparator + examplesTestFolder;

ProjectBuilder pb = ProjectBuilder.create();
pb.additionalSources(Arrays.asList("bar"));
Expand All @@ -217,6 +218,7 @@ protected Builder<Project> getCompileBuildStep() {
protected void runCompiler(List<String> optionList) {
assertThat(optionList, hasItem(mainFile));
assertThat(optionList, hasItem(incFile));
assertThat(optionList, hasItems("-sourcepath", srcPath));
// Skip the compiler
}
};
Expand Down Expand Up @@ -490,4 +492,24 @@ protected void runNativeBuilder(List<String> optionList) throws IOException {
}
}.setFresh(true).build();
}

@Test
void testMultiSourceWithDeps() throws IOException {
Path foo = examplesTestFolder.resolve("extending").resolve("Foo.java");
ProjectBuilder pb = ProjectBuilder.create();
Project prj = pb.build(foo.toString());

new JavaSource.JavaAppBuilder(prj) {
@Override
protected Builder<Project> getCompileBuildStep() {
return new JavaCompileBuildStep() {
@Override
protected void runCompiler(List<String> optionList) {
assertThat(optionList, hasItems("-sourcepath", examplesTestFolder.toString(), foo.toString()));
// Skip the compiler
}
};
}
}.setFresh(true).build();
}
}

0 comments on commit 28bdc4b

Please sign in to comment.