jbang icon indicating copy to clipboard operation
jbang copied to clipboard

//DEPS to jar file are handled incorrectly

Open quintesse opened this issue 2 months ago • 15 comments

~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

quintesse avatar Oct 11 '25 22:10 quintesse

Ok, actually the bug is older than that, that same line also causes errors with version v0.130.0

quintesse avatar Oct 11 '25 22:10 quintesse

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.

quintesse avatar Oct 11 '25 23:10 quintesse

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 avatar Oct 12 '25 09:10 quintesse

@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 avatar Nov 19 '25 18:11 wfouche

@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 avatar Nov 20 '25 14:11 quintesse

@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]

wfouche avatar Nov 20 '25 15:11 wfouche

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)

quintesse avatar Nov 20 '25 15:11 quintesse

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();
  }
}

wfouche avatar Nov 20 '25 15:11 wfouche

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)

quintesse avatar Nov 20 '25 15:11 quintesse

Ok, so using //DEPS ./aplu5.jar works while --deps ./aplu5.jar doesn't, right?

Correct.

--deps mydep.java

Invalid dependency locator: 'mydep.java'. Expected format is groupId:artifactId:version[:classifier][@type]

wfouche avatar Nov 20 '25 15:11 wfouche

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.

maxandersen avatar Nov 20 '25 19:11 maxandersen

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)

quintesse avatar Nov 20 '25 20:11 quintesse

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

maxandersen avatar Nov 20 '25 23:11 maxandersen

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).

quintesse avatar Nov 21 '25 11:11 quintesse

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)

maxandersen avatar Nov 22 '25 08:11 maxandersen