//DEPS to jar file are handled incorrectly
~The new directives parser does not handle jar files correctly! A line like //DEPS some.jar is treated as a source dependency, which results in all kinds of issues.~
DEPS to jar files are incorrectly being treated as source dependencies instead of binary dependencies.
See https://github.com/jbangdev/jbang/issues/2240#issuecomment-3393683103
Ok, actually the bug is older than that, that same line also causes errors with version v0.130.0
Ok wow, even v0.120.0 treats //DEPS some.jar as a source project but then completely by accident generates the correct result!
The new code definitely handles it wrong but, again by accident, exposes the problem.
Btw, to explain how the code can handle this correctly "by accident":
So the code will look at dependencies and decide how to handle them. This means separating them into "binary dependencies", like GAVs which are the most common ones, and "source dependencies", like "mydep.java" which have to be built first. After building the resulting JAR will be added to the classpath of the main project.
So the detection of binary vs source deps is broken and will put .jar files into the list of source dependencies. And what will happen is that the .jar will be passed onto a new child project to be built. But that child project is a project like any other and can handle all the things you can normally throw at JBang, so that child project will take a look at the jar and say "that's a jar, nothing to do here, I'll just return that jar as my result".
Add, in a very roundabout way, things will work.
The fix is simply to make sure jars get detected as binary dependencies.
The issue with a DEP to a missing jar is a bit more involved and has to do with the fact that we no longer treat missing resources as an error (which would throw an exception and abort everything) but as special cases that code might want to handle. When that change was made I had to add code to handle that case all over the code, but source dependencies slipped through the crack (probably because it's not something we use a lot so there aren't that many tests).
So this is how a missing jar dependency can lead to finding an issue with missing source dependencies :-)
@quintesse , I was completely unaware that his functionality is supposed to be supported by JBang.
//DEPS some.jar
I don't see a reference to it in the documentation:
- https://www.jbang.dev/documentation/jbang/latest/dependencies.html
Is it documented somewhere, or does it still need to be documented?
@wfouche I think it should be documented, yes.
I will expand a bit as to the why I think it should, or not, be documented: Thing is being able to add JARs there has always been somewhat of a "suspect" feature. //DEPS are normally reserved for things that can have their own dependencies, which .jar files can't... on the other hand there are many artifact that don't have any dependencies, and what are they? Nothing more than .jar files without dependencies. So that seems a reason to allow it.
On the other hand we also have --classpath/--cp which is specifically meant for adding jars to the classpath. But there are no //CLASSPATH or //CP directives.
So why is this? Well, some of it is because --classpath-/--cp maps directly to the corresponding -cp/-classpath/--class-path options on the java commands. And that also supports passing folders, which is something that //DEPS doesn't support.
@quintesse , //DEPS ./myjar.jar does not work when used with the --deps command-line option.
Invalid dependency locator: './myjar.jar'. Expected format is groupId:artifactId:version[:classifier][@type]
Not sure what you mean when you say //DEPS doesn't work when used with the command line option?
Can you give a more detailed explanation on what you're trying to do? (With code + full command line)
The test program below works fine when run as: jbang run turtle.java and if all //DEPS are removed from the script then jbang run --deps ./aplu5.jar turtle.java fails.
///usr/bin/env jbang "$0" "$@" ; exit $?
// -- DEPS ch.aplu.turtle:aplu5:0.1.9
//DEPS ./aplu5.jar
import ch.aplu.turtle.Turtle;
import ch.aplu.turtle.TurtleFrame;
// To make aplu5.jar available to Jython, install it in
// the local Maven repository by following these steps:
//
// (1 - Download file) $ wget https://www.java-online.ch/download/aplu5.jar
//
// (2 - Install file ) $ mvn install:install-file -Dfile=./aplu5.jar -DgroupId=ch.aplu.turtle -DartifactId=aplu5 -Dversion=0.1.9 -Dpackaging=jar
//
public class turtle {
public static Turtle joe = null;
public static void drawSquare(int sideLength)
{
for (int i = 0; i < 4; i++)
{
joe.forward(sideLength);
joe.right(90);
}
}
public static void drawing()
{
joe.setPenColor("red");
// repeat 10 times
for (int i = 0; i < 10; i++)
{
drawSquare(150);
joe.left(36); // rotate 36 degrees for the next square
}
}
public static void main(String[] args)
{
TurtleFrame frame = new TurtleFrame("Turtle Demo", 500, 500);
turtle.joe = new Turtle(frame);
turtle.drawing();
}
}
Ok, so using //DEPS ./aplu5.jar works while --deps ./aplu5.jar doesn't, right?
Could you do a small test to see if --deps mydep.java works for you? (Just add an empty class mydep {} to the file)
Ok, so using
//DEPS ./aplu5.jarworks while--deps ./aplu5.jardoesn't, right?
Correct.
--deps mydep.java
Invalid dependency locator: 'mydep.java'. Expected format is groupId:artifactId:version[:classifier][@type]
Another reason I keep not being a fan of //deps supporting jars.
It really is --class-path and not --deps is supposed to map to.
What is that "other reason"?
In the end it just makes the most sense, all dependencies turn into jars on the classpath. So saying that oh you can put Maven artifacts in there and other scripts (that get compiled and have their own dependencies) but not jars, is just strange to me. Sure you could have different directives for all (eg //DEPS gav, //JARS jar and //SUBPROJECT sub.java), but then you're just making life more difficult for users having to remember multiple directives... which in the end all do the same! (putting jar files on the class path. in fact the implementation would basically be a copy&paste of all the code that handles //DEPS and --deps but with the names changed)
the reason is the ordering is no longer easy to explain and there is no longer a clear link between command line arguments and jbang directives.
like, whats the semantics of //DEPS a:b:c:RELEASE,./abc.jar,other:thing:1.2
But you would have the same problem with multiple directives.
Like, what's the semantics of:
//DEPS a:b:c:RELEASE
//JARS ,./abc.jar
//DEPS other:thing:1.2
You can say, but we can document that //DEPS will always be put on the classpath before (or after) //JARS. Sure, but so can we document the same for //DEPS a:b:c:RELEASE,./abc.jar,other:thing:1.2. The only difference is that suddenly you're dealing with 3 different directives instead of simply one. You haven't solved any problem and only succeeded in making things more complex for the user (IMO).
The only reason you get 3 is because you expliciltiy split them, the right way and (at least to me) easier way to explain it would be:
//DEPS a:b:c:RELEASE,other:thing:1.2
//CLASSPATH ,./abc.jar
And that will actually at least have the //DEPS order be true in code and CLASSPATH would be having same semantics as using --classpath on command line
Where as with it all on one line its not.
Also remember we already have this for many other directives.
COMPILE_OPTIONS and RUNTIME_OPTIONS are separated out for similar reasons. Yes, we could guess which is what but we don't.
How about when we need to deal with classpath vs module deps ?
I guess your point is that it all ends up on the same java -cp argument anyway...and i can kinda get that but I feel like we are ending up weakening our ability to validate/check these both as humans and via code...at least its obvious that our current implementation does not handle this .jar situation well (latest one being #2330)