From d1a2f171e7d9f1a402d0b38cc48b4231a2ec728f Mon Sep 17 00:00:00 2001 From: Tako Schotanus Date: Fri, 17 Mar 2023 10:59:38 +0100 Subject: [PATCH] fix: Module fixes (#1594) * chore: removed unneeded `info modulepath` And any code creating module paths because a list with only modules is either the same as the list of dependencies (class path) or it would contain only "real" modules which is a useless distinction. * refactor: minor rollback of changes to `resolveInJavaHome()` Earlier changes were moving in the direction of passing a `Jdk` object instead of a version string. But that is actually not future save, where there might be a point where we would want to be able to say: "okay, I've used 'javac' from Jdk X to compile this and now I need 'native-image', but it's not in this Jdk X. Is there perhaps a newer Jdk I could use?". If we pass a `Jdk` object we would be stuck at the current version. * chore: minor change in running `jar` Now using the generic `Util.runCommand()` * fix: fixed issue with module name handling Setting the module name to an empty string on the command line would not result in the expected behavior (turn the script into a module with a name generated from the script name) * fix: fixed info classpath implementation * chore: make `ModuleUtil.isModule()` slightly smarter The method will now also look for an alternative location for the `module-info.class` file. This should still be improved though. * chore: minor text change * fix: `CmdGeneratorBuilder` now has its own `moduleName` This is similar to `mainClass` which also exists both in the `Project` and in the `CmdGenerator`. And for the same reason: we want to be able to use a different value at runtime than that at buid time. But unlike `mainClass` the value for `moduleName` is not always copied as the default value from `Project` to `CmdGenerator`; we don't always want to run JARs as modules by default, for example, but instead we'd like the user to state their intentions explicitely by passing `--module` to the `run` command. * feat: added `info jar` and some other options The command `info classpath` now takes an optional `--deps-only` and `info tools` taken an optional `--select` where you tell it to only return information for that specific field. * chore: allow `//MODULE` tag without name This will cause JBang to generate a proper module name. * chore: rewording of error message * feat: `//MODULE` is now more usable JBang will now automatically add all system modules as required and will open the script's package to everyone. This is not ideal because it breaks all encapsulation, but it results in code that will work in most cases. If you want better control you'll have to fall back to using a separate `module-info.java` file. * test: fixed tests not always working right First of all having a `@BeforeEach` with the same method name in a base class and a sub class can sometimes cause problems, especially when you try to run single tests. And the second problem would normally occur only on Windows where the TEMP folder is actually in the same filesystem tree as the user files, meaning that when we created a temp folder strcture for the tests the code that looks for aliases and catalogs etc would still return actual user files, ruining the tests. --- src/main/java/dev/jbang/Settings.java | 21 ++++ src/main/java/dev/jbang/cli/Info.java | 79 ++++++++------ src/main/java/dev/jbang/cli/Run.java | 1 + .../dev/jbang/dependencies/ArtifactInfo.java | 11 +- .../jbang/dependencies/ModularClassPath.java | 12 --- .../java/dev/jbang/source/AppBuilder.java | 4 +- .../dev/jbang/source/CmdGeneratorBuilder.java | 10 ++ src/main/java/dev/jbang/source/Project.java | 3 +- src/main/java/dev/jbang/source/TagReader.java | 13 ++- .../source/buildsteps/CompileBuildStep.java | 54 ++-------- .../source/generators/JarCmdGenerator.java | 12 ++- .../source/generators/JshCmdGenerator.java | 8 +- src/main/java/dev/jbang/util/JarUtil.java | 16 +-- src/main/java/dev/jbang/util/JavaUtil.java | 6 +- src/main/java/dev/jbang/util/ModuleUtil.java | 102 +++++++++++++++++- src/main/java/dev/jbang/util/Util.java | 11 +- .../java9/dev/jbang/util/ModuleUtil9.java | 12 +++ src/main/resources/module-info.qute.java | 3 +- src/test/java/dev/jbang/BaseTest.java | 2 + .../java/dev/jbang/cli/TestAliasNearest.java | 2 +- .../cli/TestAliasNearestWithBaseRef.java | 2 +- .../dev/jbang/cli/TestAliasWithBaseRef.java | 2 +- src/test/java/dev/jbang/cli/TestCatalog.java | 2 +- .../dev/jbang/cli/TestCatalogNearest.java | 4 +- src/test/java/dev/jbang/cli/TestRun.java | 24 +++++ src/test/java/dev/jbang/cli/TestTemplate.java | 2 +- .../java/dev/jbang/source/TestModule.java | 60 ++++++++++- 27 files changed, 331 insertions(+), 147 deletions(-) diff --git a/src/main/java/dev/jbang/Settings.java b/src/main/java/dev/jbang/Settings.java index f547b4a66..59e323065 100644 --- a/src/main/java/dev/jbang/Settings.java +++ b/src/main/java/dev/jbang/Settings.java @@ -13,6 +13,7 @@ public class Settings { public static final String JBANG_REPO = "JBANG_REPO"; public static final String JBANG_DIR = "JBANG_DIR"; public static final String JBANG_CACHE_DIR = "JBANG_CACHE_DIR"; + public static final String JBANG_LOCAL_ROOT = "JBANG_LOCAL_ROOT"; public static final String TRUSTED_SOURCES_JSON = "trusted-sources.json"; public static final String DEPENDENCY_CACHE_JSON = "dependency_cache.json"; @@ -81,6 +82,26 @@ public static Path getConfigEditorDir() { return getConfigDir(true).resolve(EDITOR_DIR); } + /** + * This returns the directory where the lookup of "local" files should stop. + * Local files are files like `jbang-catalog.json` that are read from the + * current directory and then recursively upwards until the root of the file + * system has been reached or until the folder returned by this method was + * encountered. + * + * @return The directory where local lookup should be stopped + */ + public static Path getLocalRootDir() { + Path dir; + String jlr = System.getenv(JBANG_LOCAL_ROOT); + if (jlr != null) { + dir = Paths.get(jlr); + } else { + dir = Paths.get(System.getProperty("user.home")); + } + return dir; + } + public static void setupJBangDir(Path dir) { // create JBang configuration dir if it does not yet exist dir.toFile().mkdirs(); diff --git a/src/main/java/dev/jbang/cli/Info.java b/src/main/java/dev/jbang/cli/Info.java index ebe8636dc..9f7a588f1 100644 --- a/src/main/java/dev/jbang/cli/Info.java +++ b/src/main/java/dev/jbang/cli/Info.java @@ -3,11 +3,10 @@ import static dev.jbang.Settings.CP_SEPARATOR; import java.io.IOException; +import java.lang.reflect.Field; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -24,11 +23,12 @@ import dev.jbang.net.JdkProvider; import dev.jbang.source.*; import dev.jbang.util.JavaUtil; +import dev.jbang.util.ModuleUtil; import picocli.CommandLine; @CommandLine.Command(name = "info", description = "Provides info about the script for tools (and humans who are tools).", subcommands = { - Tools.class, ClassPath.class, ModulePath.class }) + Tools.class, ClassPath.class, Jar.class }) public class Info { } @@ -84,7 +84,6 @@ static class ScriptInfo { List dependencies; List repositories; List resolvedDependencies; - List resolvedModules; String javaVersion; String requestedJavaVersion; String availableJdkPath; @@ -113,6 +112,7 @@ public ScriptInfo(Project prj, BuildContext ctx, boolean assureJdkInstalled) { ? ctx.getNativeImageFile().toAbsolutePath().toString() : null; mainClass = prj.getMainClass(); + module = ModuleUtil.getModuleName(prj); requestedJavaVersion = prj.getJavaVersion(); try { @@ -125,18 +125,14 @@ public ScriptInfo(Project prj, BuildContext ctx, boolean assureJdkInstalled) { // Ignore } - String cp = prj.resolveClassPath().getClassPath(); - if (cp.isEmpty()) { + List artifacts = prj.resolveClassPath().getArtifacts(); + if (artifacts.isEmpty()) { resolvedDependencies = Collections.emptyList(); } else { - resolvedDependencies = Arrays.asList(cp.split(CP_SEPARATOR)); - } - - String mp = prj.resolveClassPath().getModulePath(); - if (mp.isEmpty()) { - resolvedModules = null; - } else { - resolvedModules = Arrays.asList(cp.split(CP_SEPARATOR)); + resolvedDependencies = artifacts + .stream() + .map(a -> a.getFile().toString()) + .collect(Collectors.toList()); } if (prj.getJavaVersion() != null) { @@ -152,7 +148,7 @@ public ScriptInfo(Project prj, BuildContext ctx, boolean assureJdkInstalled) { Project jarProject = Project.builder().build(ctx.getJarFile()); mainClass = jarProject.getMainClass(); gav = jarProject.getGav().orElse(gav); - module = jarProject.getModuleName().orElse(module); + module = ModuleUtil.getModuleName(jarProject); } } } @@ -239,11 +235,38 @@ ProjectBuilder createProjectBuilder() { @CommandLine.Command(name = "tools", description = "Prints a json description usable for tools/IDE's to get classpath and more info for a jbang script/application. Exact format is still quite experimental.") class Tools extends BaseInfoCommand { + @CommandLine.Option(names = { + "--select" }, description = "Indicate the name of the field to select and return from the full info result") + String select; + @Override public Integer doCall() throws IOException { Gson parser = new GsonBuilder().disableHtmlEscaping().setPrettyPrinting().create(); - parser.toJson(getInfo(true), System.out); + ScriptInfo info = getInfo(true); + if (select != null) { + try { + Field f = info.getClass().getDeclaredField(select); + Object v = f.get(info); + if (v != null) { + if (v instanceof String || v instanceof Number) { + System.out.println(v); + } else { + parser.toJson(v, System.out); + } + } else { + // We'll return an error code for `null` so + // any calling scripts can easily detect that + // situation instead of having to ambiguously + // compare against the string "null" + return EXIT_GENERIC_ERROR; + } + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new ExitException(EXIT_INVALID_INPUT, "Cannot return value of unknown field: " + select, e); + } + } else { + parser.toJson(info, System.out); + } return EXIT_OK; } @@ -252,12 +275,17 @@ public Integer doCall() throws IOException { @CommandLine.Command(name = "classpath", description = "Prints class-path used for this application using operating system specific path separation.") class ClassPath extends BaseInfoCommand { + @CommandLine.Option(names = { + "--deps-only" }, description = "Only include the dependencies in the output, not the application jar itself") + boolean dependenciesOnly; + @Override public Integer doCall() throws IOException { ScriptInfo info = getInfo(false); List cp = new ArrayList<>(info.resolvedDependencies.size() + 1); - if (info.applicationJar != null && !info.resolvedDependencies.contains(info.applicationJar)) { + if (!dependenciesOnly && info.applicationJar != null + && !info.resolvedDependencies.contains(info.applicationJar)) { cp.add(info.applicationJar); } cp.addAll(info.resolvedDependencies); @@ -267,24 +295,13 @@ public Integer doCall() throws IOException { } } -@CommandLine.Command(name = "modulepath", description = "Prints module-path used for this application using operating system specific path separation.") -class ModulePath extends BaseInfoCommand { +@CommandLine.Command(name = "jar", description = "Prints the path to this application's JAR file.") +class Jar extends BaseInfoCommand { @Override public Integer doCall() throws IOException { - ScriptInfo info = getInfo(false); - if (info.resolvedModules != null) { - List cp = new ArrayList<>(info.resolvedModules.size() + 1); - if (info.applicationJar != null - && !info.resolvedModules.contains(info.applicationJar) - && ArtifactInfo.isModule(Paths.get(info.applicationJar))) { - cp.add(info.applicationJar); - } - cp.addAll(info.resolvedModules); - System.out.println(String.join(CP_SEPARATOR, cp)); - } - + System.out.println(info.applicationJar); return EXIT_OK; } } diff --git a/src/main/java/dev/jbang/cli/Run.java b/src/main/java/dev/jbang/cli/Run.java index 3ead319a2..76f53ac84 100644 --- a/src/main/java/dev/jbang/cli/Run.java +++ b/src/main/java/dev/jbang/cli/Run.java @@ -145,6 +145,7 @@ CmdGeneratorBuilder updateGeneratorForRun(CmdGeneratorBuilder gb) { .setArguments(userParams) .runtimeOptions(runMixin.javaRuntimeOptions) .mainClass(buildMixin.main) + .moduleName(buildMixin.module) .interactive(runMixin.interactive) .enableAssertions(runMixin.enableAssertions) .enableSystemAssertions(runMixin.enableSystemAssertions) diff --git a/src/main/java/dev/jbang/dependencies/ArtifactInfo.java b/src/main/java/dev/jbang/dependencies/ArtifactInfo.java index 5fad311c2..1813216c7 100644 --- a/src/main/java/dev/jbang/dependencies/ArtifactInfo.java +++ b/src/main/java/dev/jbang/dependencies/ArtifactInfo.java @@ -1,7 +1,5 @@ package dev.jbang.dependencies; -import java.io.InputStream; -import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; import java.util.Objects; @@ -46,14 +44,7 @@ public boolean isModule() { } public static boolean isModule(Path file) { - try { - URL url = new URL("jar:" + file.toUri().toURL() + "!/module-info.class"); - try (InputStream s = url.openStream()) { - return true; - } - } catch (Exception ex) { - return false; - } + return ModuleUtil.isModule(file); } public String getModuleName() { diff --git a/src/main/java/dev/jbang/dependencies/ModularClassPath.java b/src/main/java/dev/jbang/dependencies/ModularClassPath.java index 4efa0afd5..2ce25b73a 100644 --- a/src/main/java/dev/jbang/dependencies/ModularClassPath.java +++ b/src/main/java/dev/jbang/dependencies/ModularClassPath.java @@ -49,14 +49,6 @@ public String getClassPath() { return String.join(CP_SEPARATOR, getClassPaths()); } - public List getModulePaths() { - if (modulePaths == null) { - modulePaths = getArtifactPaths(artifacts.stream().filter(ArtifactInfo::isModule)); - } - - return modulePaths; - } - private List getArtifactPaths(Stream artifacts) { return artifacts .map(it -> it.getFile().toAbsolutePath().toString()) @@ -65,10 +57,6 @@ private List getArtifactPaths(Stream artifacts) { .collect(Collectors.toList()); } - public String getModulePath() { - return String.join(CP_SEPARATOR, getModulePaths()); - } - public String getManifestPath() { return artifacts.stream() .map(it -> it.getFile().toAbsolutePath().toUri()) diff --git a/src/main/java/dev/jbang/source/AppBuilder.java b/src/main/java/dev/jbang/source/AppBuilder.java index 9b3a322dd..0437201a9 100644 --- a/src/main/java/dev/jbang/source/AppBuilder.java +++ b/src/main/java/dev/jbang/source/AppBuilder.java @@ -115,7 +115,9 @@ public CmdGeneratorBuilder build() throws IOException { } } - return CmdGenerator.builder(project, ctx); + return CmdGenerator .builder(project, ctx) + .mainClass(project.getMainClass()) + .moduleName(project.getModuleName().orElse(null)); } public static boolean keepClasses() { diff --git a/src/main/java/dev/jbang/source/CmdGeneratorBuilder.java b/src/main/java/dev/jbang/source/CmdGeneratorBuilder.java index 2df059ed1..1b83ddbb2 100644 --- a/src/main/java/dev/jbang/source/CmdGeneratorBuilder.java +++ b/src/main/java/dev/jbang/source/CmdGeneratorBuilder.java @@ -21,6 +21,7 @@ public class CmdGeneratorBuilder { private List runtimeOptions = Collections.emptyList(); private String mainClass; + private String moduleName; private Boolean interactive; private Boolean enableAssertions; private Boolean enableSystemAssertions; @@ -61,6 +62,11 @@ public CmdGeneratorBuilder mainClass(String mainClass) { return this; } + public CmdGeneratorBuilder moduleName(String moduleName) { + this.moduleName = moduleName; + return this; + } + public CmdGeneratorBuilder interactive(Boolean interactive) { this.interactive = interactive; return this; @@ -117,6 +123,7 @@ private JarCmdGenerator createJarCmdGenerator() { .runtimeOptions(runtimeOptions) .mainClass(mainClass) .mainRequired(interactive != Boolean.TRUE) + .moduleName(moduleName) .assertions(enableAssertions == Boolean.TRUE) .systemAssertions(enableSystemAssertions == Boolean.TRUE) .classDataSharing( @@ -150,6 +157,9 @@ private void updateFromAlias(Alias alias) { if (mainClass == null) { mainClass(alias.mainClass); } + if (moduleName == null) { + moduleName(alias.moduleName); + } if (flightRecorderString == null) { flightRecorderString(alias.jfr); } diff --git a/src/main/java/dev/jbang/source/Project.java b/src/main/java/dev/jbang/source/Project.java index 522595e0f..79d58dcd4 100644 --- a/src/main/java/dev/jbang/source/Project.java +++ b/src/main/java/dev/jbang/source/Project.java @@ -14,6 +14,7 @@ import dev.jbang.dependencies.MavenRepo; import dev.jbang.dependencies.ModularClassPath; import dev.jbang.source.sources.JavaSource; +import dev.jbang.util.ModuleUtil; import dev.jbang.util.Util; /** @@ -218,7 +219,7 @@ protected String getStableId() { if (stableId == null) { Stream sss = mainSourceSet.getStableIdInfo(); if (moduleName != null) { - Stream s = Stream.of(moduleName); + Stream s = Stream.of(ModuleUtil.getModuleName(this)); sss = Stream.concat(sss, s); } stableId = Util.getStableID(sss); diff --git a/src/main/java/dev/jbang/source/TagReader.java b/src/main/java/dev/jbang/source/TagReader.java index c0a9d69f6..e022d1564 100644 --- a/src/main/java/dev/jbang/source/TagReader.java +++ b/src/main/java/dev/jbang/source/TagReader.java @@ -40,7 +40,7 @@ public abstract class TagReader { private static final String FILES_COMMENT_PREFIX = "FILES "; private static final String SOURCES_COMMENT_PREFIX = "SOURCES "; private static final String MAIN_COMMENT_PREFIX = "MAIN "; - private static final String MODULE_COMMENT_PREFIX = "MODULE "; + private static final String MODULE_COMMENT_PREFIX = "MODULE"; private static final String DESCRIPTION_COMMENT_PREFIX = "DESCRIPTION "; private static final String GAV_COMMENT_PREFIX = "GAV "; @@ -177,16 +177,19 @@ public Optional getModule() { Util.warnMsg( "Multiple //MODULE lines found, only one should be defined in a source file. Using the first"); } - if (!Util.isValidModuleIdentifier(mods.get(0))) { + if (mods.get(0).isEmpty()) { + return Optional.of(""); + } else if (Util.isValidModuleIdentifier(mods.get(0).substring(1))) { + return Optional.of(mods.get(0).substring(1)); + } else { throw new IllegalArgumentException( - "//MODULE line has wrong format, should be '//MODULE identifier]'"); + "//MODULE line has wrong format, should be '//MODULE [identifier]'"); } - return Optional.of(mods.get(0)); } } protected boolean isModuleDeclare(String line) { - return line.startsWith(MODULE_COMMENT_PREFIX); + return line.equals(MODULE_COMMENT_PREFIX) || line.startsWith(MODULE_COMMENT_PREFIX + " "); } public Optional getGav() { diff --git a/src/main/java/dev/jbang/source/buildsteps/CompileBuildStep.java b/src/main/java/dev/jbang/source/buildsteps/CompileBuildStep.java index fdd35cc00..9cfa21068 100644 --- a/src/main/java/dev/jbang/source/buildsteps/CompileBuildStep.java +++ b/src/main/java/dev/jbang/source/buildsteps/CompileBuildStep.java @@ -9,9 +9,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; -import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -24,7 +22,6 @@ import dev.jbang.cli.BaseCommand; import dev.jbang.cli.ExitException; -import dev.jbang.dependencies.ArtifactInfo; import dev.jbang.dependencies.MavenCoordinate; import dev.jbang.source.BuildContext; import dev.jbang.source.Builder; @@ -93,11 +90,12 @@ protected Project compile() throws IOException { if (project.getModuleName().isPresent()) { if (project.getMainSource() != null && !project.getMainSource().getJavaPackage().isPresent()) { - throw new ExitException(BaseCommand.EXIT_INVALID_INPUT, "Module code is missing a 'package' statement"); + throw new ExitException(BaseCommand.EXIT_INVALID_INPUT, + "Module code cannot work with the default package, adding a 'package' statement is required"); } if (!hasModuleInfoFile()) { // generate module-info descriptor and add it to list of files to compile - Path infoFile = generateModuleInfo(); + Path infoFile = ModuleUtil.generateModuleInfo(project, ctx.getGeneratedSourcesDir()); if (infoFile != null) { optionList.add(infoFile.toString()); } @@ -170,48 +168,7 @@ protected Path generatePom() throws IOException { return pomPath; } - protected Path generateModuleInfo() throws IOException { - Template infoTemplate = TemplateEngine .instance() - .getTemplate( - ResourceRef.forResource("classpath:/module-info.qute.java")); - - Path infoPath = null; - if (infoTemplate == null) { - // ignore - Util.warnMsg("Could not locate module-info.java template"); - } else { - // First get the list of root dependencies as proper maven coordinates - Set deps = project .getMainSourceSet() - .getDependencies() - .stream() - .map(MavenCoordinate::fromString) - .collect(Collectors.toSet()); - // Now filter out the resolved artifacts that are root dependencies - // and get their names - List moduleNames = project .resolveClassPath() - .getArtifacts() - .stream() - .filter(a -> deps.contains(a.getCoordinate())) - .map(ArtifactInfo::getModuleName) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - // Finally create a module-info file with the name of the module - // and the list of required modules using the names we just listed - String modName = ModuleUtil.getModuleName(project); - String infoFile = infoTemplate - .data("name", modName) - .data("dependencies", moduleNames) - .render(); - - infoPath = ctx.getGeneratedSourcesDir().resolve("module-info.java"); - Files.createDirectories(infoPath.getParent()); - Util.writeString(infoPath, infoFile); - } - - return infoPath; - } - - public static MavenCoordinate getPomGav(Project prj) { + private static MavenCoordinate getPomGav(Project prj) { if (prj.getGav().isPresent()) { return MavenCoordinate.fromString(prj.getGav().get()).withVersion(); } else { @@ -223,7 +180,8 @@ public static MavenCoordinate getPomGav(Project prj) { public static Path getPomPath(Project prj, BuildContext ctx) { MavenCoordinate gav = getPomGav(prj); - return ctx.getCompileDir().resolve("META-INF/maven/" + gav.getGroupId().replace(".", "/") + "/pom.xml"); + return ctx .getCompileDir() + .resolve("META-INF/maven/" + gav.getGroupId().replace(".", "/") + "/pom.xml"); } protected void searchForMain(Path tmpJarDir) { diff --git a/src/main/java/dev/jbang/source/generators/JarCmdGenerator.java b/src/main/java/dev/jbang/source/generators/JarCmdGenerator.java index c0b0b6d31..17fa284fc 100644 --- a/src/main/java/dev/jbang/source/generators/JarCmdGenerator.java +++ b/src/main/java/dev/jbang/source/generators/JarCmdGenerator.java @@ -28,6 +28,7 @@ public class JarCmdGenerator extends BaseCmdGenerator { private boolean classDataSharing; private String mainClass; private boolean mainRequired; + private String moduleName; public JarCmdGenerator runtimeOptions(List runtimeOptions) { if (runtimeOptions != null) { @@ -63,6 +64,11 @@ public JarCmdGenerator mainRequired(boolean mainRequired) { return this; } + public JarCmdGenerator moduleName(String moduleName) { + this.moduleName = moduleName; + return this; + } + public JarCmdGenerator(Project prj, BuildContext ctx) { super(prj, ctx); } @@ -115,7 +121,7 @@ protected List generateCommandLineList() throws IOException { } } if (!Util.isBlankString(classpath)) { - if (project.getModuleName().isPresent()) { + if (moduleName != null && project.getModuleName().isPresent()) { optionalArgs.addAll(Arrays.asList("-p", classpath)); } else { optionalArgs.addAll(Arrays.asList("-classpath", classpath)); @@ -147,8 +153,8 @@ protected List generateCommandLineList() throws IOException { String main = Optional.ofNullable(mainClass).orElse(project.getMainClass()); if (main != null) { - if (project.getModuleName().isPresent()) { - String modName = ModuleUtil.getModuleName(project); + if (moduleName != null && project.getModuleName().isPresent()) { + String modName = moduleName.isEmpty() ? ModuleUtil.getModuleName(project) : moduleName; fullArgs.add("-m"); fullArgs.add(modName + "/" + main); } else { diff --git a/src/main/java/dev/jbang/source/generators/JshCmdGenerator.java b/src/main/java/dev/jbang/source/generators/JshCmdGenerator.java index 93f76538f..cfc69ca34 100644 --- a/src/main/java/dev/jbang/source/generators/JshCmdGenerator.java +++ b/src/main/java/dev/jbang/source/generators/JshCmdGenerator.java @@ -14,7 +14,6 @@ import org.apache.commons.text.StringEscapeUtils; import dev.jbang.net.JdkManager; -import dev.jbang.net.JdkProvider; import dev.jbang.source.*; import dev.jbang.util.JavaUtil; import dev.jbang.util.Util; @@ -56,8 +55,7 @@ protected List generateCommandLineList() throws IOException { List optionalArgs = new ArrayList<>(); String requestedJavaVersion = project.getJavaVersion(); - JdkProvider.Jdk jdk = JdkManager.getOrInstallJdk(requestedJavaVersion); - String javacmd = JavaUtil.resolveInJavaHome("jshell", jdk); + String javacmd = JavaUtil.resolveInJavaHome("jshell", requestedJavaVersion); // NB: See https://github.com/jbangdev/jbang/issues/992 for the reasons why we // use the -J flags below @@ -107,7 +105,9 @@ protected List generateCommandLineList() throws IOException { fullArgs.addAll(jshellOpts(project.getRuntimeOptions())); fullArgs.addAll(jshellOpts(runtimeOptions)); - fullArgs.addAll(project.resolveClassPath().getAutoDectectedModuleArguments(jdk)); + fullArgs.addAll(project .resolveClassPath() + .getAutoDectectedModuleArguments( + JdkManager.getOrInstallJdk(requestedJavaVersion))); fullArgs.addAll(optionalArgs); if (project.isJShell()) { diff --git a/src/main/java/dev/jbang/util/JarUtil.java b/src/main/java/dev/jbang/util/JarUtil.java index 5443d0cf7..7a92e748e 100644 --- a/src/main/java/dev/jbang/util/JarUtil.java +++ b/src/main/java/dev/jbang/util/JarUtil.java @@ -66,19 +66,9 @@ private static void runJarCommand(Path jar, String action, Path src, Manifest ma private static void runJarCommand(List arguments, String requestedJavaVersion) throws IOException { arguments.add(0, resolveInJavaHome("jar", requestedJavaVersion)); - Util.verboseMsg("Jar: " + String.join(" ", arguments)); - // no inheritIO as jar complains unnecessarily about duplicate manifest entries. - ProcessBuilder pb = new ProcessBuilder(arguments); - if (Util.isVerbose()) { - pb.inheritIO(); - } - Process process = pb.start(); - try { - process.waitFor(); - } catch (InterruptedException e) { - throw new ExitException(1, e); - } - if (process.exitValue() != 0) { + Util.verboseMsg("Package: " + String.join(" ", arguments)); + String out = Util.runCommand(arguments.toArray(new String[] {})); + if (out == null) { throw new ExitException(1, "Error creating/updating jar"); } } diff --git a/src/main/java/dev/jbang/util/JavaUtil.java b/src/main/java/dev/jbang/util/JavaUtil.java index dc6fb83ad..7d4314d96 100644 --- a/src/main/java/dev/jbang/util/JavaUtil.java +++ b/src/main/java/dev/jbang/util/JavaUtil.java @@ -118,11 +118,7 @@ public static int getCurrentMajorJavaVersion() { } public static String resolveInJavaHome(@Nonnull String cmd, @Nullable String requestedVersion) { - return resolveInJavaHome(cmd, JdkManager.getOrInstallJdk(requestedVersion)); - } - - public static String resolveInJavaHome(@Nonnull String cmd, @Nonnull JdkProvider.Jdk jdk) { - Path jdkHome = jdk.getHome(); + Path jdkHome = JdkManager.getOrInstallJdk(requestedVersion).getHome(); if (jdkHome != null) { if (Util.isWindows()) { cmd = cmd + ".exe"; diff --git a/src/main/java/dev/jbang/util/ModuleUtil.java b/src/main/java/dev/jbang/util/ModuleUtil.java index 3977266df..d9f5c3d45 100644 --- a/src/main/java/dev/jbang/util/ModuleUtil.java +++ b/src/main/java/dev/jbang/util/ModuleUtil.java @@ -1,13 +1,49 @@ package dev.jbang.util; +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.annotation.Nullable; + +import dev.jbang.catalog.CatalogUtil; +import dev.jbang.dependencies.ArtifactInfo; +import dev.jbang.dependencies.MavenCoordinate; import dev.jbang.source.Project; +import dev.jbang.source.ResourceRef; + +import io.quarkus.qute.Template; /** * WARNING: This file MUST be compiled with Java 9+ */ public class ModuleUtil { + public static boolean isModule(Path file) { + try { + URL url = new URL("jar:" + file.toUri().toURL() + "!/module-info.class"); + try (InputStream s = url.openStream()) { + return true; + } + } catch (Exception ex) { + } + try { + // TODO This is a very specific test, we should do better + URL url = new URL("jar:" + file.toUri().toURL() + "!/META-INF/versions/9/module-info.class"); + try (InputStream s = url.openStream()) { + return true; + } + } catch (Exception ex) { + return false; + } + } + public static String getModuleName(Path file) { if (JavaUtil.getCurrentMajorJavaVersion() >= 9) { return ModuleUtil9.getModuleName(file); @@ -16,11 +52,73 @@ public static String getModuleName(Path file) { } } + @Nullable public static String getModuleName(Project project) { String modName = project.getModuleName().orElse(null); - if (modName == null || modName.isEmpty()) { - modName = project.getGav().orElse("jbangapp"); + if (modName != null && modName.isEmpty()) { + modName = project.getGav().orElse(CatalogUtil.nameFromRef(project.getResourceRef().getOriginalResource())); } return modName; } + + @Nullable + public static String getModuleMain(Project project) { + if (project.getModuleName().isPresent() && project.getMainClass() != null) { + return getModuleName(project) + "/" + project.getMainClass(); + } else { + return null; + } + } + + public static Path generateModuleInfo(Project project, Path targetDir) throws IOException { + Template infoTemplate = TemplateEngine .instance() + .getTemplate( + ResourceRef.forResource("classpath:/module-info.qute.java")); + + Path infoPath = null; + if (infoTemplate == null) { + // ignore + Util.warnMsg("Could not locate module-info.java template"); + } else { + // First get the list of root dependencies as proper maven coordinates + Set deps = project .getMainSourceSet() + .getDependencies() + .stream() + .map(MavenCoordinate::fromString) + .collect(Collectors.toSet()); + // Now filter out the resolved artifacts that are root dependencies + // and get their names + Stream depModNames = project.resolveClassPath() + .getArtifacts() + .stream() + .filter(a -> deps.contains(a.getCoordinate())) + .map(ArtifactInfo::getModuleName) + .filter(Objects::nonNull); + // And join this list of names with the JDK module names + List moduleNames = Stream .concat(ModuleUtil.listJdkModules().stream(), depModNames) + .collect(Collectors.toList()); + // Finally create a module-info file with the name of the module + // and the list of required modules using the names we just listed + String modName = ModuleUtil.getModuleName(project); + String infoFile = infoTemplate + .data("moduleName", modName) + .data("packageName", project.getMainSource().getJavaPackage().get()) + .data("dependencies", moduleNames) + .render(); + + infoPath = targetDir.resolve("module-info.java"); + Files.createDirectories(infoPath.getParent()); + Util.writeString(infoPath, infoFile); + } + + return infoPath; + } + + private static List listJdkModules() { + if (JavaUtil.getCurrentMajorJavaVersion() >= 9) { + return ModuleUtil9.listJdkModules(); + } else { + return null; + } + } } diff --git a/src/main/java/dev/jbang/util/Util.java b/src/main/java/dev/jbang/util/Util.java index 13367c790..2fedd2226 100644 --- a/src/main/java/dev/jbang/util/Util.java +++ b/src/main/java/dev/jbang/util/Util.java @@ -1711,7 +1711,8 @@ private static Path findNearestLocalFileWith(Path dir, String fileName, Function if (dir == null) { dir = getCwd(); } - while (dir != null) { + Path root = Settings.getLocalRootDir(); + while (dir != null && (root == null || !isSameFile(dir, root))) { Path file = dir.resolve(fileName); if (Files.isRegularFile(file) && Files.isReadable(file) && accept.apply(file)) { return file; @@ -1725,6 +1726,14 @@ private static Path findNearestLocalFileWith(Path dir, String fileName, Function return null; } + public static boolean isSameFile(Path f1, Path f2) { + try { + return Files.isSameFile(f1, f2); + } catch (IOException e) { + return f1.toAbsolutePath().equals(f2.toAbsolutePath()); + } + } + public static boolean isNullOrEmptyString(String str) { return str == null || str.isEmpty(); } diff --git a/src/main/java9/dev/jbang/util/ModuleUtil9.java b/src/main/java9/dev/jbang/util/ModuleUtil9.java index bf910476f..ae8c0fc21 100644 --- a/src/main/java9/dev/jbang/util/ModuleUtil9.java +++ b/src/main/java9/dev/jbang/util/ModuleUtil9.java @@ -4,7 +4,9 @@ import java.lang.module.ModuleFinder; import java.lang.module.ModuleReference; import java.nio.file.Path; +import java.util.List; import java.util.Set; +import java.util.stream.Collectors; /** * WARNING: This file MUST be compiled with Java 9+ @@ -21,4 +23,14 @@ public static String getModuleName(Path file) { } return null; } + + public static List listJdkModules() { + ModuleLayer ml = ModuleLayer.boot(); + return ml .modules() + .stream() + .filter(m -> m.isNamed() && m.getAnnotation(Deprecated.class) == null + && (m.getName().startsWith("java.") || m.getName().startsWith("jdk."))) + .map(m -> m.getName()) + .collect(Collectors.toList()); + } } diff --git a/src/main/resources/module-info.qute.java b/src/main/resources/module-info.qute.java index 2b14abbcb..079eb9512 100644 --- a/src/main/resources/module-info.qute.java +++ b/src/main/resources/module-info.qute.java @@ -1,5 +1,6 @@ -module {name} { +module {moduleName} { {#for item in dependencies} requires {item}; {/for} + opens {packageName}; } diff --git a/src/test/java/dev/jbang/BaseTest.java b/src/test/java/dev/jbang/BaseTest.java index 9eef0e9b4..333a2310e 100644 --- a/src/test/java/dev/jbang/BaseTest.java +++ b/src/test/java/dev/jbang/BaseTest.java @@ -44,6 +44,8 @@ void initEnv(@TempDir Path tempPath) throws IOException { // Except we make all tests use the same JDK installation folder to prevent // excessive downloads environmentVariables.set(Settings.JBANG_CACHE_DIR + "_JDKS", jdksTempDir.toString()); + // Make sure we don't go looking outside our temp dir + environmentVariables.set(Settings.JBANG_LOCAL_ROOT, tempPath.toString()); // Don't check fo rnew versions while running tests environmentVariables.set(Settings.ENV_NO_VERSION_CHECK, "true"); if (Util.isWindows()) { diff --git a/src/test/java/dev/jbang/cli/TestAliasNearest.java b/src/test/java/dev/jbang/cli/TestAliasNearest.java index 5fbe318b5..8d2e6cddf 100644 --- a/src/test/java/dev/jbang/cli/TestAliasNearest.java +++ b/src/test/java/dev/jbang/cli/TestAliasNearest.java @@ -93,7 +93,7 @@ public class TestAliasNearest extends BaseTest { "}"; @BeforeEach - void init() throws IOException { + void initEach() throws IOException { Files.write(jbangTempDir.resolve(Catalog.JBANG_CATALOG_JSON), global.getBytes()); Path cwd = Files.createDirectory(cwdDir.resolve("test")); Util.setCwd(cwd); diff --git a/src/test/java/dev/jbang/cli/TestAliasNearestWithBaseRef.java b/src/test/java/dev/jbang/cli/TestAliasNearestWithBaseRef.java index 581339ec6..96e913c4a 100644 --- a/src/test/java/dev/jbang/cli/TestAliasNearestWithBaseRef.java +++ b/src/test/java/dev/jbang/cli/TestAliasNearestWithBaseRef.java @@ -45,7 +45,7 @@ public class TestAliasNearestWithBaseRef extends BaseTest { "}"; @BeforeEach - void init() throws IOException { + void initEach() throws IOException { Files.write(jbangTempDir.resolve(Catalog.JBANG_CATALOG_JSON), global.getBytes()); Path cwd = Files.createDirectory(cwdDir.resolve("test")); Util.setCwd(cwd); diff --git a/src/test/java/dev/jbang/cli/TestAliasWithBaseRef.java b/src/test/java/dev/jbang/cli/TestAliasWithBaseRef.java index b2f7449e9..3ebebd51c 100644 --- a/src/test/java/dev/jbang/cli/TestAliasWithBaseRef.java +++ b/src/test/java/dev/jbang/cli/TestAliasWithBaseRef.java @@ -35,7 +35,7 @@ public class TestAliasWithBaseRef extends BaseTest { "}"; @BeforeEach - void init() throws IOException { + void initEach() throws IOException { Files.write(jbangTempDir.resolve(Catalog.JBANG_CATALOG_JSON), aliases.getBytes()); Util.setCwd(Files.createDirectory(cwdDir.resolve("test"))); } diff --git a/src/test/java/dev/jbang/cli/TestCatalog.java b/src/test/java/dev/jbang/cli/TestCatalog.java index 7c8781ada..063fe751c 100644 --- a/src/test/java/dev/jbang/cli/TestCatalog.java +++ b/src/test/java/dev/jbang/cli/TestCatalog.java @@ -35,7 +35,7 @@ public class TestCatalog extends BaseTest { static Path testCatalogFile = null; @BeforeEach - void init() throws IOException { + void initEach() throws IOException { catsFile = jbangTempDir.resolve("jbang-catalog.json"); testCatalogFile = cwdDir.resolve("test-catalog.json"); Files.write(testCatalogFile, testCatalog.getBytes()); diff --git a/src/test/java/dev/jbang/cli/TestCatalogNearest.java b/src/test/java/dev/jbang/cli/TestCatalogNearest.java index 4f5178e7a..578a87054 100644 --- a/src/test/java/dev/jbang/cli/TestCatalogNearest.java +++ b/src/test/java/dev/jbang/cli/TestCatalogNearest.java @@ -36,7 +36,7 @@ public class TestCatalogNearest extends BaseTest { "}"; @BeforeEach - void init() throws IOException { + void initEach() throws IOException { aliasesFile = cwdDir.resolve("aliases.json"); Files.write(aliasesFile, aliases.getBytes()); parentDotDir = Files.createDirectory(cwdDir.resolve(".jbang")); @@ -48,7 +48,7 @@ void init() throws IOException { CatalogUtil.addCatalogRef(testDotDir.resolve(Catalog.JBANG_CATALOG_JSON), "dotlocal", aliasesFile.toString(), "Local .jbang"); CatalogUtil.addCatalogRef(parentDotDir.resolve(Catalog.JBANG_CATALOG_JSON), "dotparent", - aliasesFile.toString(), "Patent .jbang"); + aliasesFile.toString(), "Parent .jbang"); CatalogUtil.addCatalogRef(jbangTempDir.resolve(Catalog.JBANG_CATALOG_JSON), "global", aliasesFile.toString(), "Global"); } diff --git a/src/test/java/dev/jbang/cli/TestRun.java b/src/test/java/dev/jbang/cli/TestRun.java index 7fa9138d4..dedefc7b1 100644 --- a/src/test/java/dev/jbang/cli/TestRun.java +++ b/src/test/java/dev/jbang/cli/TestRun.java @@ -488,7 +488,31 @@ void testHelloWorldGAVWithExplicitMainClass() throws IOException { assertThat(cmd, matchesPattern(".* -classpath .*picocli-4.6.3.jar.*")); assertThat(cmd, not(containsString(" -jar "))); + } + + @Test + void testHelloWorldGAVWithModule() throws IOException { + environmentVariables.clear("JAVA_HOME"); + String jar = "info.picocli:picocli-codegen:4.6.3"; + + CommandLine.ParseResult pr = JBang .getCommandLine() + .parseArgs("run", "--module", "--main", + "picocli.codegen.aot.graalvm.ReflectionConfigGenerator", jar); + Run run = (Run) pr.subcommand().commandSpec().userObject(); + + ProjectBuilder pb = run.createProjectBuilderForRun(); + Project code = pb.build(jar); + + String cmd = run.updateGeneratorForRun(CmdGenerator.builder(code)).build().generate(); + + assertThat(code.getMainClass(), nullValue()); + assertThat(cmd, endsWith("info.picocli.codegen/picocli.codegen.aot.graalvm.ReflectionConfigGenerator")); + assertThat(code.getResourceRef().getFile().toString(), + matchesPattern(".*jbang_tests_maven.*codegen-4.6.3.jar")); + + assertThat(cmd, matchesPattern(".* -p .*picocli-4.6.3.jar.*")); + assertThat(cmd, not(containsString(" -jar "))); } @Test diff --git a/src/test/java/dev/jbang/cli/TestTemplate.java b/src/test/java/dev/jbang/cli/TestTemplate.java index ee9649376..8699d653e 100644 --- a/src/test/java/dev/jbang/cli/TestTemplate.java +++ b/src/test/java/dev/jbang/cli/TestTemplate.java @@ -68,7 +68,7 @@ public class TestTemplate extends BaseTest { "}"; @BeforeEach - void init() throws IOException { + void initEach() throws IOException { Files.write(jbangTempDir.resolve(Catalog.JBANG_CATALOG_JSON), templates.getBytes()); Util.setCwd(Files.createDirectory(cwdDir.resolve("test"))); } diff --git a/src/test/java/dev/jbang/source/TestModule.java b/src/test/java/dev/jbang/source/TestModule.java index c9f58ab34..78e28b9af 100644 --- a/src/test/java/dev/jbang/source/TestModule.java +++ b/src/test/java/dev/jbang/source/TestModule.java @@ -43,6 +43,15 @@ public class TestModule extends BaseTest { " }\n" + "}\n"; + String srcWithEmptyModDep = "//MODULE\n" + + "//DEPS info.picocli:picocli:4.6.3\n" + + "package test;" + + "public class moduletest {\n" + + " public static void main(String... args) {\n" + + " System.out.println(\"Hello World\");\n" + + " }\n" + + "}\n"; + String srcWithoutMod = "//DEPS info.picocli:picocli:4.6.3\n" + "package test;" + "public class moduletest {\n" + @@ -101,10 +110,53 @@ public Project build() { } }.setFresh(true).build(); - String cmd = gen.mainClass("test.moduletest").build().generate(); + String cmd = gen.build().generate(); assertThat(cmd, endsWith(" -m testmodule/test.moduletest")); } + @Test + void testEmptyModule(@TempDir File output) throws IOException { + Path f = output.toPath().resolve("moduletest.java"); + Util.writeString(f, srcWithEmptyModDep); + + ProjectBuilder pb = Project.builder(); + Project prj = pb.build(f); + BuildContext ctx = BuildContext.forProject(prj); + + CmdGeneratorBuilder gen = new JavaSource.JavaAppBuilder(prj, ctx) { + @Override + protected Builder getCompileBuildStep() { + return new JavaCompileBuildStep() { + @Override + protected void runCompiler(List optionList) throws IOException { + assertThat(optionList, hasItems(endsWith("module-info.java"))); + + Path modInfo = ctx.getGeneratedSourcesDir().resolve("module-info.java"); + assertThat(modInfo.toFile(), anExistingFile()); + assertThat(Util.readFileContent(modInfo), containsString("requires info.picocli;")); + + super.runCompiler(optionList); + } + }; + } + + @Override + protected Builder getJarBuildStep() { + return new JarBuildStep(project, ctx) { + @Override + public Project build() { + assertThat(ctx.getCompileDir().resolve("module-info.class").toFile(), anExistingFile()); + // Skip building of JAR + return project; + } + }; + } + }.setFresh(true).build(); + + String cmd = gen.build().generate(); + assertThat(cmd, endsWith(" -m moduletest/test.moduletest")); + } + @Test void testModuleWithCustomModuleInfo(@TempDir File output) throws IOException { Path f = output.toPath().resolve("moduletest.java"); @@ -112,7 +164,9 @@ void testModuleWithCustomModuleInfo(@TempDir File output) throws IOException { Path mi = output.toPath().resolve("module-info.java"); Util.writeString(mi, "FAKE MODULE INFO"); - ProjectBuilder pb = Project.builder().additionalSources(Collections.singletonList(mi.toString())); + ProjectBuilder pb = Project .builder() + .mainClass("test.moduletest") + .additionalSources(Collections.singletonList(mi.toString())); Project prj = pb.build(f); BuildContext ctx = BuildContext.forProject(prj); @@ -132,7 +186,7 @@ protected void runCompiler(List optionList) { } }.setFresh(true).build(); - String cmd = gen.mainClass("test.moduletest").build().generate(); + String cmd = gen.build().generate(); assertThat(cmd, endsWith(" -m testmodule/test.moduletest")); }