fix: alias settings not being properly applied (#1686)

* test: make sure Alias tests cover all fields

* fix: enablePreview is now treated the same as other boolean options

* fix: not all alias settings were properly applied

This fixes for example the issue where setting the "java" option didn't
actually do anything.

* test: added bunch of tests for `ProjectBuilder`

These tests the main ways a project can be built: from a source file,
from a JAR or from a GAV. In all three cases we also test their options
being overridden by aliases.

* fix: make sure resource strings use proper path separators

* fix: make sure changes to System.properties are always reset

itests/alltags.java

@@ -0,0 +1,28 @@
+
+//REPOS dummy=http://dummy
+//DEPS dummy.org:dummy:1.2.3
+
+//SOURCES Two.java
+//SOURCES nested/*.java
+
+//FILES res/resource.properties renamed.properties=res/resource.properties
+//FILES META-INF/application.properties=res/resource.properties
+
+//COMPILE_OPTIONS --enable-preview --verbose
+//RUNTIME_OPTIONS --add-opens java.base/java.net=ALL-UNNAMED
+//NATIVE_OPTIONS -O1 -d
+
+//JAVA 11+
+//MAIN mainclass
+//MODULE mymodule
+//DESCRIPTION some description
+//GAV example.org:alltags:1.2.3
+//CDS
+//PREVIEW
+//MANIFEST one=1 two=2 three=3
+//JAVAAGENT
+
+public class alltags {
+ public static void main(String... args) {
+ }
+}

itests/jbang-catalog.json

@@ -19,6 +19,87 @@
  },
  "resource": {
  "script-ref": "res/resource.java"
+ },
+ "alltags": {
+ "script-ref": "alltags.java",
+ "description": "twodesc",
+ "arguments": ["2"],
+ "runtime-options": ["--two"],
+ "sources": ["helloworld.java"],
+ "files": ["res/test.properties"],
+ "dependencies": ["twodep"],
+ "repositories": ["tworepo"],
+ "classpaths": ["twocp"],
+ "properties": {"two":"2"},
+ "java": "twojava",
+ "main": "twomain",
+ "module": "twomodule",
+ "compile-options": ["--ctwo"],
+ "native-image": true,
+ "native-options": ["--ntwo"],
+ "jfr": "twojfr",
+ "debug": {"twod":"2"},
+ "cds": true,
+ "interactive": true,
+ "enable-preview": true,
+ "enable-assertions": true,
+ "enable-system-assertions": true,
+ "manifest-options": {"twom":"2"},
+ "java-agents": [{"agent-ref":"twojag","options":"twoopts"}]
+ },
+ "hellojar": {
+ "script-ref": "hellojar.jar",
+ "description": "twodesc",
+ "arguments": ["2"],
+ "runtime-options": ["--two"],
+ "sources": ["helloworld.java"],
+ "files": ["res/test.properties"],
+ "dependencies": ["twodep"],
+ "repositories": ["tworepo"],
+ "classpaths": ["twocp"],
+ "properties": {"two":"2"},
+ "java": "twojava",
+ "main": "twomain",
+ "module": "twomodule",
+ "compile-options": ["--ctwo"],
+ "native-image": true,
+ "native-options": ["--ntwo"],
+ "jfr": "twojfr",
+ "debug": {"twod":"2"},
+ "cds": true,
+ "interactive": true,
+ "enable-preview": true,
+ "enable-assertions": true,
+ "enable-system-assertions": true,
+ "manifest-options": {"twom":"2"},
+ "java-agents": [{"agent-ref":"twojag","options":"twoopts"}]
+ },
+ "pgmgav": {
+ "script-ref": "org.eclipse.jgit:org.eclipse.jgit.pgm:5.9.0.202009080501-r",
+ "description": "twodesc",
+ "arguments": ["2"],
+ "runtime-options": ["--two"],
+ "sources": ["helloworld.java"],
+ "files": ["res/test.properties"],
+ "dependencies": ["info.picocli:picocli:4.6.3"],
+ "repositories": ["mavencentral", "tworepo"],
+ "classpaths": ["twocp"],
+ "properties": {"two":"2"},
+ "java": "twojava",
+ "main": "twomain",
+ "module": "twomodule",
+ "compile-options": ["--ctwo"],
+ "native-image": true,
+ "native-options": ["--ntwo"],
+ "jfr": "twojfr",
+ "debug": {"twod":"2"},
+ "cds": true,
+ "interactive": true,
+ "enable-preview": true,
+ "enable-assertions": true,
+ "enable-system-assertions": true,
+ "manifest-options": {"twom":"2"},
+ "java-agents": [{"agent-ref":"twojag","options":"twoopts"}]
  }
  },
  "templates": {},

src/main/java/dev/jbang/catalog/Alias.java

@@ -6,8 +6,11 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.function.Function;
 
+import javax.annotation.Nonnull;
+
 import com.google.gson.annotations.SerializedName;
 
 import dev.jbang.cli.ExitException;
@@ -56,17 +59,30 @@ public class Alias extends CatalogItem {
 
  public static class JavaAgent {
  @SerializedName(value = "agent-ref")
+ @Nonnull
  public final String agentRef;
+ @Nonnull
  public final String options;
 
- private JavaAgent() {
- this(null, null);
- }
-
- public JavaAgent(String agentRef, String options) {
+ public JavaAgent(@Nonnull String agentRef, @Nonnull String options) {
  this.agentRef = agentRef;
  this.options = options;
  }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+ JavaAgent javaAgent = (JavaAgent) o;
+ return agentRef.equals(javaAgent.agentRef) && options.equals(javaAgent.options);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(agentRef, options);
+ }
  }
 
  public Alias() {

src/main/java/dev/jbang/catalog/CatalogItem.java

@@ -1,5 +1,7 @@
 package dev.jbang.catalog;
 
+import java.nio.file.Paths;
+
 import dev.jbang.util.Util;
 
 abstract class CatalogItem {
@@ -16,7 +18,12 @@ public CatalogItem(Catalog catalog) {
  public String resolve(String scriptRef) {
  String ref = scriptRef;
  if (!Util.isAbsoluteRef(ref)) {
- ref = catalog.getScriptBase() + "/" + ref;
+ String base = catalog.getScriptBase();
+ if (Util.isRemoteRef(base) || !Util.isValidPath(base)) {
+ ref = base + "/" + ref;
+ } else {
+ ref = Paths.get(base).resolve(ref).toString();
+ }
  }
  return ref;
  }

src/main/java/dev/jbang/cli/Alias.java

@@ -85,7 +85,7 @@ class AliasAdd extends BaseAliasCommand {
  String name;
 
  @CommandLine.Option(names = { "--enable-preview" }, description = "Activate Java preview features")
- boolean enablePreviewRequested;
+ Boolean enablePreviewRequested;
 
  @CommandLine.Parameters(paramLabel = "params", index = "1..*", arity = "0..*", description = "Parameters to pass on to the script")
  List<String> userParams;

src/main/java/dev/jbang/cli/BaseBuildCommand.java

@@ -31,7 +31,7 @@ public abstract class BaseBuildCommand extends BaseCommand {
  Path buildDir;
 
  @CommandLine.Option(names = { "--enable-preview" }, description = "Activate Java preview features")
- boolean enablePreviewRequested;
+ Boolean enablePreviewRequested;
 
  PrintStream out = new PrintStream(new FileOutputStream(FileDescriptor.out));
 

src/main/java/dev/jbang/dependencies/ArtifactResolver.java

@@ -12,6 +12,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Properties;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -382,8 +383,10 @@ private Settings newEffectiveSettings() {
  DefaultSettingsBuilderFactory factory = new DefaultSettingsBuilderFactory();
  DefaultSettingsBuilder builder = factory.newInstance();
 
+ Properties props = new Properties();
+ props.putAll(System.getProperties());
  SettingsBuildingRequest settingsBuilderRequest = new DefaultSettingsBuildingRequest();
- settingsBuilderRequest.setSystemProperties(System.getProperties());
+ settingsBuilderRequest.setSystemProperties(props);
 
  if (withUserSettings) {
  // find the settings

src/main/java/dev/jbang/source/ProjectBuilder.java

@@ -53,7 +53,7 @@ public class ProjectBuilder {
  private File catalogFile;
  private Boolean nativeImage;
  private String javaVersion;
- private boolean enablePreview;
+ private Boolean enablePreview;
 
  // Cached values
  private Properties contextProperties;
@@ -172,7 +172,7 @@ public ProjectBuilder nativeImage(Boolean nativeImage) {
  return this;
  }
 
- public ProjectBuilder enablePreview(boolean enablePreviewRequested) {
+ public ProjectBuilder enablePreview(Boolean enablePreviewRequested) {
  this.enablePreview = enablePreviewRequested;
  return this;
  }
@@ -278,7 +278,7 @@ private Project createJarProject(ResourceRef resourceRef) {
  && DependencyUtil.looksLikeAGav(resourceRef.getOriginalResource())) {
  prj.getMainSourceSet().addDependency(resourceRef.getOriginalResource());
  }
- return importJarMetadata(updateProject(prj));
+ return updateProject(importJarMetadata(prj, moduleName != null && moduleName.isEmpty()));
  }
 
  private Project createJbangProject(ResourceRef resourceRef) {
@@ -353,12 +353,12 @@ public Project build(Source src) {
  return updateProject(updateProjectMain(src, prj, getResourceResolver()));
  }
 
- private Project importJarMetadata(Project prj) {
+ private Project importJarMetadata(Project prj, boolean importModuleName) {
  Path jar = prj.getResourceRef().getFile();
  if (jar != null && Files.exists(jar)) {
  try (JarFile jf = new JarFile(jar.toFile())) {
  String moduleName = ModuleUtil.getModuleName(jar);
- if (moduleName != null && "".equals(prj.getModuleName().orElse(null))) {
+ if (moduleName != null && importModuleName) {
  // We only import the module name if the project's module
  // name was set to an empty string, which basically means
  // "we want module support, but we don't know the name".
@@ -410,6 +410,7 @@ private Project updateProject(Project prj) {
  updateAllSources(prj, replaceAllProps(additionalSources));
  ss.addResources(allToFileRef(replaceAllProps(additionalResources)));
  ss.addCompileOptions(compileOptions);
+ ss.addNativeOptions(nativeOptions);
  prj.putProperties(properties);
  prj.getManifestAttributes().putAll(manifestOptions);
  if (moduleName != null) {
@@ -426,7 +427,9 @@ private Project updateProject(Project prj) {
  if (nativeImage != null) {
  prj.setNativeImage(nativeImage);
  }
- prj.setEnablePreviewRequested(enablePreview);
+ if (enablePreview != null) {
+ prj.setEnablePreviewRequested(enablePreview);
+ }
  return prj;
  }
 
@@ -589,6 +592,9 @@ private void updateFromAlias(Alias alias) {
  if (manifestOptions.isEmpty()) {
  manifestOptions(alias.manifestOptions);
  }
+ if (enablePreview == null) {
+ enablePreview(alias.enablePreview);
+ }
  }
 
  public static boolean isAlias(ResourceRef resourceRef) {

src/main/java/dev/jbang/source/RefTarget.java

@@ -6,6 +6,10 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.util.Objects;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import dev.jbang.cli.ExitException;
 import dev.jbang.util.Util;
@@ -16,19 +20,23 @@
  * //FILES target=source directives in scripts and templates.
  */
 public class RefTarget {
+ @Nonnull
  protected final ResourceRef source;
+ @Nullable
  protected final Path target;
 
- protected RefTarget(ResourceRef source, Path target) {
+ protected RefTarget(@Nonnull ResourceRef source, @Nullable Path target) {
  assert (source != null);
  this.source = source;
  this.target = target;
  }
 
+ @Nonnull
  public ResourceRef getSource() {
  return source;
  }
 
+ @Nullable
  public Path getTarget() {
  return target;
  }
@@ -52,6 +60,29 @@ public void copy(Path destroot) {
  }
  }
 
+ @Override
+ public boolean equals(Object o) {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+ RefTarget refTarget = (RefTarget) o;
+ return source.equals(refTarget.source) && Objects.equals(target, refTarget.target);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(source, target);
+ }
+
+ @Override
+ public String toString() {
+ return "RefTarget{" +
+ "source=" + source +
+ ", target=" + target +
+ '}';
+ }
+
  public static RefTarget create(String ref, Path dest, ResourceResolver siblingResolver) {
  return new RefTarget(siblingResolver.resolve(ref), dest);
  }

src/main/java/dev/jbang/source/TagReader.java

@@ -392,6 +392,13 @@ public static RefTarget toFileRef(String fileReference, ResourceResolver sibling
 
  try {
  ResourceRef ref = siblingResolver.resolve(src);
+ if (ref == null) {
+ throw new ExitException(EXIT_INVALID_INPUT,
+ String.format("Could not find '%s' when resolving '%s' in %s",
+ src,
+ fileReference,
+ siblingResolver.description()));
+ }
  if (dest != null && dest.endsWith("/")) {
  p = p.resolve(ref.getFile().getFileName());
  }