jbangdev / jbang

Unleash the power of Java - JBang Lets Students, Educators and Professional Developers create, edit and run self-contained source-only Java programs with unprecedented ease.
https://jbang.dev
MIT License
1.41k stars 157 forks source link

Dependency range with a + is not correct #1268

Open jmini opened 2 years ago

jmini commented 2 years ago

Describe the bug When resolving a version range (using a +), it just takes the latest.

Example 1.3.+ is converted to [1.3.,)

Gradle has a way better definition:

A prefix version range: e.g. 1.+, 1.3.+

  • Only versions exactly matching the portion before the + are included.
  • The range + on it’s own will include any version.

Source https://docs.gradle.org/current/userguide/single_versions.html

So 1.3.+ probably means something like [1.3.,1.4.)

To Reproduce

///usr/bin/env jbang "$0" "$@" ; exit $?
//DEPS org.eclipse.collections:eclipse-collections:10.3.+

import org.eclipse.collections.api.list.ImmutableList;
import org.eclipse.collections.api.factory.Lists;

class collection {

    public static void main(String[] args) {
        ImmutableList<String> list = Lists.immutable.of("One", "One", "Two", "Three");

        list.forEach(e -> System.out.println(e));
    }
}

When executed

$ jbang collection
[jbang] Resolving dependencies...
[jbang] org.eclipse.collections:eclipse-collections:jar:[10.3.,)
Done
[jbang] Dependencies resolved
[jbang] Building jar...

Inspecting the classpath with jbang info tools shows that the resolvedDependencies contains:

~/.m2/repository/org/eclipse/collections/eclipse-collections/11.0.0/eclipse-collections-11.0.0.jar
~/.m2/repository/org/eclipse/collections/eclipse-collections-api/11.0.0/eclipse-collections-api-11.0.0.jar

11.0.0 is the newest version available at time of writing.

Expected behavior

10.3.0 is the only version in maven central matching 10.3.+

As comparison, with a gradle project having:

dependencies {
    implementation 'org.eclipse.collections:eclipse-collections:10.3.+'
}

Extract of the commandgradle dependencies:

compileClasspath
+--- org.eclipse.collections:eclipse-collections:10.3.+ -> 10.3.0
     \--- org.eclipse.collections:eclipse-collections-api:10.3.0

JBang version Paste output of jbang version --verbose

[jbang] jbang version 0.90.1
Cache: ~/.jbang/cache
Config: ~/.jbang
Repository: ~/.m2/repository
0.90.1

Additional context Related issue: #1267

jmini commented 2 years ago

I disagree with this implementation:

https://github.com/jbangdev/jbang/blob/ef362429c1c9a60aec8200b3f3184cbb74160c98/src/main/java/dev/jbang/dependencies/DependencyUtil.java#L307-L314

And the test: https://github.com/jbangdev/jbang/blob/ef362429c1c9a60aec8200b3f3184cbb74160c98/src/test/java/dev/jbang/dependencies/DependencyResolverTest.java#L31-L34

Or at least it is not compatible with Gradle definition of the + notation.

jmini commented 2 years ago

A workaround is to use [10.3.0,10.4.0) instead of 10.3.+.

And because of issue #1268, the @Grab notation must be used:

///usr/bin/env jbang "$0" "$@" ; exit $?
//DEPS org.apache.groovy:groovy:4.0.0

import org.eclipse.collections.api.list.ImmutableList;
import org.eclipse.collections.api.factory.Lists;

import groovy.lang.Grab;
import groovy.lang.Grapes;
import groovy.lang.GrabResolver;

@GrabResolver("mavenCentral")
@Grapes({
    @Grab(group="org.eclipse.collections", module="eclipse-collections", version="[10.3.0,10.4.0)")
})
class collection {

    public static void main(String[] args) {
        ImmutableList<String> list = Lists.immutable.of("One", "One", "Two", "Three");

        list.forEach(e -> System.out.println(e));
    }
}

Is correctly resolved.

Check with jbang info tools collection and check the resolvedDependencies. It contains:

jmini commented 2 years ago

In method dev.jbang.source.ScriptSource.extractDependencies(String)

Probably the code line.split(" // ")[0].split("[ ;,]+") should be rewritten to take the first element, parse it and check if there is more to parse.

@maxandersen WDYT ? I could start a PR in this direction if you give a go for this approach.

Or supporting multiple deps in the same // DEPS line is a bad idea… Personally I am not doing this, but probably some users do it like this.

maxandersen commented 2 years ago

We do support multiple deps on one line. If not via //DEPS then via --deps.

About version ranges the current implementation was done to have at least one way to "loosen" the deps. It currently mimick what we do for //JAVA.

Happy to see if we can enable a "richer" version range syntax - just need to be something also somewhat expressible on cli.