dependency notation does not support maven version range syntax containing ) and ,

[NikkyAI]

EDIT: Maven supports version ranges natively: examples: http://www.mojohaus.org/versions-maven-plugin/examples/resolve-ranges.html
an it works in kscript once the annotation and comment parsing is not messing up on , and )

old issue content:

i am writing a library and tool that i try to make compatilble / consumable by kscript
the versions are pushed to maven by jenkins , each git commit increments the build number

ideally i would like to use

@file:DependsOnMaven("moe.nikky.voodoo:dsl:0.4.0+")

instead of

@file:DependsOnMaven("moe.nikky.voodoo:dsl:0.4.0-132")

[NikkyAI]

related: https://youtrack.jetbrains.com/issue/KT-27051

[holgerbrandl]

Could you point me to some reference docs about dynamic versions? What is the format? Is it "just" about supporting dashes and pluses in the version field of the artifact locator?

[NikkyAI]

gradle docs are not very helpful: https://docs.gradle.org/current/userguide/declaring_dependencies.html#sub:declaring_dependency_with_dynamic_version
seems like they have a open issue about documenting the syntax: gradle/gradle#5603

something else that might be interesting might be dependency locking: https://docs.gradle.org/current/userguide/dependency_locking.html

[holgerbrandl]

Why don't you use jitpack to get temporary artifacts?

[NikkyAI]

i did that but it kept reusing the same -SNAPSHOT version that maven cached and update that only somewhat randomly
aalso jitpack had issues buildingthe software, took way too long compared to a friends jenkins and maven repo

jitpack is a nice concept that did not end up working for me sadly

[holgerbrandl]

Indeed for continuous rebuilding it is sometimes a bit sluggish.

Coming back to the dynamic dependencies. What is the format? Is it "just" about supporting dashes and pluses in the version field of the artifact locator?

In order to support it we would need at least some (unit-)test as the dependency lookup is likely to be replace with a different backend (kotlin-scripting-api, aether from existing PR).

[NikkyAI]

could this be used ? http://ant.apache.org/ivy/history/latest-milestone/ivyfile/dependency.html

or apparently maven itself supports a different notation of version ranges
http://www.mojohaus.org/versions-maven-plugin/examples/resolve-ranges.html

[0.4.0,) would be my usecase, but it breaks currently, i guess because there is no resolve-ranges step

[holgerbrandl]

Would maven support such versions? Since it is the current resolver backend, it would be hard to implement it if not. Or we would need to change until we change to something like kotlin-main-api or aether (see PR).

[NikkyAI]

well i have no experience using POMs directly, but i am currently trying to add mvn versions:resolve-ranges to the maven invocation in kscript

[NikkyAI]

apparently ranges like [1.0,2.0] work, but open ranges eg. [1.0,) are breaking

[NikkyAI]

for some reason [0.4.0,) gets regexed into [0.4.0, which produces errors in maven

EDIT: apparently thats the value passed to the regex already, something in the annotation and //DEPS parsing does splitting on , which seems to break this

[NikkyAI]

so spliting versions with , breaks the input of maven version ranges, once i fixed that (an the tests) everything else works, i think i should PR this and continue the discussion there

PS: the annotation parsing splits on ), replacing this with substringBeforeLast seems reasonable and makes ) useable within the annotation

[holgerbrandl]

Thanks for the PR. I guess we can not simply change the delimiter from , to ; because of backwar compatibility reasons. But do we have to? Can these dynamic versions actually end with a comma? Your examples seem to indicate the opposite?

[NikkyAI]

well i think not, they need to end in a ) i think,
so smarter parsing ?

[NikkyAI]

https://github.com/holgerbrandl/kscript/blob/d6104195c86bbeec4bf9ca51022e53efb2f82790/src/main/kotlin/kscript/app/Script.kt#L148-L152
this currently breaks

//DEPS moe.nikky.voodoo-rewrite:core:[0.4.0,)

would it be acceptavble to remove that failure case ?

[NikkyAI]

which are the lagacy usecases i should test for ?
currently the whole testsuite seems to pass

[holgerbrandl]

You changed the existing tests in your PR which are by design legacy use-cases. They must not be touched, otherwise it may break the tool for other use-cases.

[NikkyAI]

okay well i will try to design around them..
i wonder how creative i have to get with parsing and regex
there is lilits to what you can do in the comment without quotes framing each dependency

//DEPS group:name-core:[1.0,3.0),group:name-api:[1.0,3.0)

is not really easy to parse, i could also just ignore it and just fix it for @DependsOn annotations
but that feels bad as well.. and you would have to warn people about what won't work in which case..

PS: i undid the changes to tests and to the //DEPS handling for now

[NikkyAI]

so i hjust discovered that idea bootstrapping is also broken forversion ranges, kind of obvious really
now.. i would love to translate them properly, but thats probably impossible, maybe you could change it so that mvn copies the dependencies in the libs folder
also a big change .. and not comfortable

i wonder if one could just replace + with "[$it,)" i nthe POM builder and cover the new usecase

i hope kotlin's scripting will use ivy version matching though, then the hack can be removed

is there any reason why someone would want to have a literal + in a version on maven? is that even legal ?

[holgerbrandl]

is not really easy to parse, i could also just ignore it and just fix it for @dependsOn annotations
but that feels bad as well.. and you would have to warn people about what won't work in which case..

+1 It's hard to parse. +1 consistency at this level would be nice, although I'd prefer annotations as well.

So if not even idea is supporting them in their gradle/maven sync, does kscript really need/want it?

[NikkyAI]

i think idea would support it.. i mean kscript does nto let me create the project because it runs through codepaths that are broken because //DEPS i think
i just got a bit fed up because of that
see my last comment on the closed PR

i have a new attempt of just supporting 1.0+ and converting that into a maven open range during POM building
not having to touch the rest of the parsing (although i will include the improved annotation parsing)
and it will work in comments and annotations

[holgerbrandl]

@NikkyAI Is is possible that dynamic versions can just be resolved when running kscript (including the patch from above), if at least one version is sitting in the local .m2 cache already?

E.g when using some non-yet-~/.m2 dependency such as com.indeed:util-mmap:jar:1.0.+, it fails to resolve them for me.

[holgerbrandl]

... which might be an argument in favor of PR #49 instead of maven?

[holgerbrandl]

Since the PR has been long merged, I'm also closing the corresponding ticket.