Repository navigation
dependency notation does not support maven version range syntax containing ) and , #166
Description
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")
Activity
NikkyAI commentedon Sep 24, 2018
related: https://youtrack.jetbrains.com/issue/KT-27051
holgerbrandl commentedon Sep 25, 2018
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 commentedon Sep 25, 2018
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 commentedon Sep 25, 2018
Why don't you use jitpack to get temporary artifacts?
NikkyAI commentedon Sep 25, 2018
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 commentedon Sep 26, 2018
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 commentedon Sep 26, 2018
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 stepholgerbrandl commentedon Sep 26, 2018
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 commentedon Sep 26, 2018
well i have no experience using POMs directly, but i am currently trying to add
mvn versions:resolve-rangesto the maven invocation in kscriptNikkyAI commentedon Sep 26, 2018
apparently ranges like
[1.0,2.0]work, but open ranges eg.[1.0,)are breakingNikkyAI commentedon Sep 26, 2018
for some reason
[0.4.0,)getsregexed intowhich produces errors in maven[0.4.0,EDIT: apparently thats the value passed to the regex already, something in the annotation and //DEPS parsing does splitting on
,which seems to break thisNikkyAI commentedon Sep 26, 2018
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 therePS: the annotation parsing splits on
), replacing this with substringBeforeLast seems reasonable and makes)useable within the annotationfix maven open version ranges
holgerbrandl commentedon Sep 26, 2018
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 commentedon Sep 26, 2018
well i think not, they need to end in a
)i think,so smarter parsing ?
2 remaining items
[-]dependency notation does not support maven version range syntax containing `)`[/-][+]dependency notation does not support maven version range syntax containing `)` and `,`[/+]NikkyAI commentedon Sep 26, 2018
which are the lagacy usecases i should test for ?
currently the whole testsuite seems to pass
holgerbrandl commentedon Sep 26, 2018
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 commentedon Sep 26, 2018
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
@DependsOnannotationsbut 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
//DEPShandling for nowNikkyAI commentedon Sep 26, 2018
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
libsfolderalso a big change .. and not comfortable
i wonder if one could just replace + with
"[$it,)"i nthe POM builder and cover the new usecasei 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 ?convert + in version to open range for maven
holgerbrandl commentedon Sep 26, 2018
+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 commentedon Sep 26, 2018
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
//DEPSi thinki 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 buildingnot having to touch the rest of the parsing (although i will include the improved annotation parsing)
and it will work in comments and annotations
Support dynamic versions ending with `+` (#170) (fixes #166)
holgerbrandl commentedon Oct 26, 2018
@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
.m2cache already?E.g when using some non-yet-
~/.m2dependency such ascom.indeed:util-mmap:jar:1.0.+, it fails to resolve them for me.holgerbrandl commentedon Oct 26, 2018
... which might be an argument in favor of PR #49 instead of maven?
holgerbrandl commentedon Jun 24, 2019
Since the PR has been long merged, I'm also closing the corresponding ticket.