kscripting / kscript

Scripting enhancements for Kotlin
MIT License
2.08k stars 126 forks source link

Allow passing custom classpath vars? #127

Open kevinmost opened 6 years ago

kevinmost commented 6 years ago

Hi! I really am enjoying using kscript. However, one thing that I think is impossible at the moment is being able to pass a custom classpath. Running vanilla kotlinc allows this (obviously, as kscript ultimately pipes through to kotlinc after some other work), and so does the new jshell in Java 9. It's useful to be able to add a local JAR to the classpath (both when quickly prototyping when you don't want to deploy a JAR to a Maven repo, and maybe when you're using kscript on some work project where your JAR isn't published to a public repo).

In an ideal world, I'd be able to pass both a custom classpath and //DEPS and have them merged into one big classpath. ex: kscript --interactive "//DEPS com.googlecode.libphonenumber:libphonenumber:8.9.6" --classpath="path/to/my.jar", or similar.

Let me know if you think this is a welcome feature. I'd be happy to discuss possible other formats for the classpath arg as well (maybe it could be part of the // directives, so "//DEPS [gradle deps] //CLASSPATH [local deps]"). I would be happy to contribute a pull request as well if there's interest.

Thanks for reading!

holgerbrandl commented 6 years ago

Thanks for the suggestion.

The problem with local classpath references for sure is, that they limit the portability of the scripts, which somehow contradicts kscript's design goal of

building self-contained mini-applications with kotlin

However, since we allow for local script references via //INCLUDE/@file:Include the box of Pandora was opened already, so we may as well support custom classpath elements.

To make kscriptlets self-contained and for consistency with the other configuration, we may want to add a custom classpaths rather by comment-directive/annotation instead of using a CLI argument.

I've checked https://github.com/Kotlin/KEEP/blob/scripting/proposals/scripting-support.md#embedded-scripting for guidance concerning naming such an additional annotation, but there does not seem to be any proposal yet (@ligee?).

You're right that //CLASSPATH/file::Classpath seems a natural and sensible choice for naming the annotation/directive.

There are different ways to implement it, I personally think that vararg style for the annotation may be the most efficient. For the directive we may also support splitting by , and ; (to be in sync with DependsOn.

So sure, feel welcome to provide a PR. We will need a test jar in the resources to allow for proper testing in the unit tests. Please note that the annotations are in a different repo, so we'd need 2 PRs I guess.

ligee commented 6 years ago

@holgerbrandl the KEEP will be updated soon, reflecting the changes introduced with Kotlin 1.2.50, but currently there is no fixed naming scheme for such annotations: the scripting support allows the script definition author to provide a list of annotations he wants to process on the configuration phase, along with configuration implementation. In the examples there are some annotations used for this purpose, e.g. DependsOn, but they are just examples. Although we think that it would be beneficial to have a REPL with build-in dependencies support out of the box, we haven't spent enough time on investigating possible use cases to propose a good solution right now.

holgerbrandl commented 6 years ago

Thanks @ligee for the great summary/clarification.

mykola-dev commented 6 years ago

any news on this? my use case: i have gradle project with many java dependencies. i packed it with shadowJar into a single jar file and want to use it as std lib for all my kotlin scripts

holgerbrandl commented 6 years ago

In this case you could also simply install it into your m2 repo. See https://maven.apache.org/guides/mini/guide-3rd-party-jars-local.html#

This issue is still pending since I was under the impression that @kevinmost was considering to put together a pr.

holgerbrandl commented 6 years ago

From slack:

is it possible to do @file:KotlinOpts("-classpath deps.jar") in the kscript?

holgerbrandl commented 6 years ago

As suggested in #171 one also may simply use //KOTLIN_OPTS -cp build/libs/myproject.jar to provide an additional custom classpath.

Since it seems a rather custom use-case, this may even make this issue obsolete?

holgerbrandl commented 6 years ago

Closed due to inactivity. Feel welcome to keep commenting on this thread.

DALDEI commented 5 years ago

Activity: none - because I get stuck using kscript from step 0 due to this -- so I cannot generate any 'activity'

holgerbrandl commented 5 years ago

@DALDEI what's wrong with the suggested workaround of adding //KOTLIN_OPTS -cp build/libs/myproject.jar to the script instead?

zhuker commented 5 years ago

@DALDEI what's wrong with the suggested workaround of adding //KOTLIN_OPTS -cp build/libs/myproject.jar to the script instead?

the KOTLIN_OPTS -cp doesn't seem to work for me

holgerbrandl commented 5 years ago

Indeed @zhuker, it's not as easy as I thought. The correct workaround is

//KOTLIN_OPTS -cp build/libs/jartester-1.0-SNAPSHOT.jar
//COMPILER_OPTS  -cp build/libs/jartester-1.0-SNAPSHOT.jar

Same with annotation config. See referenced commit for a regression test which I have added to prepare for a more solid support of local jars in DependsOn or a via a dedicated separate annotation.

The reason for the double declaration in the workaround is that the kotlinops are not passed to the compiler, and vice versa. I still think that this is the correct approach here because compiler and runtime options are typically different.

DALDEI commented 5 years ago

I took a couple stabs at this and some related issues a few months back -- got about half way and stopped. My 'overview' analysis is that the project has reached a point where initially 'good ideas' in the implementation have become entrenched and make some type of otherwise-simple enhancements difficult. I may take another stab at this at some point, but I stopped because it was too invasive and I wanted to rethinking things (such as, is it worth the effort ). Specifically Im referring to how command options are processed internally, and how subprocess are invoked. Currently, most places in the code that handle command processing, including places that don't necessarily need to be sub-process invocations, handle 'options' by local string concatenation then passing the concatenated string along where eventually its joined to one-big-command line and let the shell handle the argument splitting. This is a very convenient simplification -- I would have likely done the same. But its causing difficulties in many areas now that are hard to fix without also changing this fundamental design pattern. Example: handling multiple sources of classpaths should be as easy as creating a single -classpath argument out of an arbitrary list of class path entries -- simply concatenate them with the classpath separator (":" or ";" os specific). I tried this but didn't get far as I discovered most places are taking or generating '-classpath cp1:cp2:cp3' as a single string rather then as a list of class paths. Combing those together later is not so easy -- (one isuse of many- spaces in paths) Thinking that this would be an easy fix, it was in many places, but not all -- example: comments that include entire command lines or compiler options -- the entire comment line simply passed along as-is. Again, something I would likely have done myself.
Each individual instance is manageable -- although some would require fairly fancy parsing to get right, but atleast its isolated locations. Together, the task is much bigger -- I stopped after a few hours to rethink. Fixing this issue at its source leads to resolving multiple current issues as well as future ones -- Example, I discovered the primary reason in several cases for running a JVM as a sub-process when in-proc would have been much more efficient, is due to not having the arguments parsed upfront -- instead the shell is used in a process - I believe mainly so that it parses the command arguments to java/kotliin. In one shot, fixing this issue would enable eliminating multiple forks/subprocess calls, add the ability to concatenate class paths, make it easier for finer grained compiler option processing and overall easier to maintain and update the codebase long term.

I'd be willing to help with this, but i wanted to get some overall feedback (and assistance:) before starting the task over with a new goal. A related consideration -- given the state of Kotlin Script today -- is this the direction to continue ? or is effort better spent at integrating the ongoing enhancents to Kotlin script ? -- such as template classes etc.

holgerbrandl commented 5 years ago

Hi @DALDEI thanks for sharing your thoughts. I agree with most of your arguments.

handle 'options' by local string concatenation

Indeed, the project was born out of necessity and followed a very pragmatic path since then. Since the current version is a rewrite of an initial bash implementation of kscript, some constructs like string concatenation of options are legacy which we could refactor out slowly but steadily. And in fact this is happening, just look at the rather recent Script type to encapsulate the user script.

JVM as a sub-process

Actually, we use process substitution, so the kscript process vanishes once it has done is job to start the user script. Which I think is pretty elegant. However, some users/contributor have their arguments to disagree, see #102.

Since kscript is intended to work within a bash environment, it needs to interfere with it (mainly via environment variables but also via sub-processes). That's why there are occasions where we actually use subprocesses (mainly bash and gradle).

I'd be willing to help with this, It would be great if you could keep supporting the project. Since I'm working on it just as a hobby I'd personally prefer changes that actually change the tool behaviour. If this also comes along with some cleanup of legacy constructs, it would be even better.

However, code changes that improve just the elegance of the internal constructs but don't add any functionality could be done later imho. This is simply due time-constraints on my end because I maintain kscript just as a hobby at night. Although along these lines, I'd prefer PRs to be atomic, i.e. addressing one problem/feature at a time. Testing becomes a nightmare otherwise.

holgerbrandl commented 5 years ago

With respect to the topic of this ticket, the question for me would be if we want to reuse DependsOn or introduce another annotation to allow for local non-maven dependencies? Clearly, this should be in sync with kotlin-scripting.

@ligee How are/would be local jar dependencies supported in kotlin-scripting?

sergeych commented 2 years ago

I have no ide why this simple feature can not be added for such a long time. As for me, kts COULD be a nice tool to automate development task, but with current, strange at least, idea to put all the code we'll need to use in the script in either local or public repositry. it is still times less effort to write such scripts with other languages. I really love kts and had invested about 50 hours to try ti make use of it still we have to use ruby, javascript and bash like to automate our tasks. And all this due to this design flaw of otherwise almost brilliant product.

To whom is not clear the necessity of the feature. When automating development processes, the simple things fit the bash script nicely. There is little use tok cope with kts to implement several calls, few loops and conditionals. But very often the task being automated needs more complex logic. Its where kts should be at hand. But instead, we are effectively limited to one script file where we should implement everything but the already published libraries. Moreover, there is no good support in IDE for kts still (I use IJ, it fails most often with it). So the normal course of action should be write your complex automation logic in a library sub project, and use a uberjat from it in scripts. Nice and smooth. Instead, we have to pack and distribute to some sort of maven anything we need in a script. As for me, it does not fit professional development. I can write it all in say ruby or python or js, and save hours. I have about 6 projects in active development and about 12 that sometimes needs attention, and having a maven backed development automation script for every one of them sounds like a poor joke. and to deploy to maven every change in the logic I use in scripts...

So once again we are banned from using otherwise excellent kscript in real life. What a pity.

holgerbrandl commented 2 years ago

Sorry to spam you again regarding this question, but @ligee are local jar dependencies supported in kotlin-scripting? If so, kscript could adopt the same pattern.

@sergeych Feel welcome to provide a PR in the kscript4 branch (FYI @aartiPl )

ligee commented 2 years ago

Ah, sorry, I somehow missed the question. Yes, local dependencies are supported via https://github.com/JetBrains/kotlin/blob/master/libraries/scripting/dependencies/src/kotlin/script/experimental/dependencies/FileSystemDependenciesResolver.kt, so if it is configured accordingly (e.g. as in MainKts), the @DependsOn will accept not only maven coordinates but also file paths. I haven't checked how you configure dependencies resolving in kscript.

holgerbrandl commented 2 years ago

That's perfect, as dependencies are just forwarded to the kotlin scripting dependency resolver we should/could do the same. I've checked but it seems that SystemDependenciesResolver.kt (and so FileSystemDependenciesResolver) are not yet part of kotlin-script-runtime-1.6.20. Are they included in a different maven artifact?

ligee commented 2 years ago

kotlin-script-runtime is obsolete, please do not use it (unless for some compatibility). The artefacts to use are kotlin-scripting-dependencies and kotlin-scripting-dependencies-maven[-all] (-all if you want to have fat jar with all maven/aeter dependencies packed in one jar). Please, check the implementation of the main-kts, in particular this declaration - https://github.com/JetBrains/kotlin/blob/1.6.20/libraries/tools/kotlin-main-kts/src/org/jetbrains/kotlin/mainKts/scriptDef.kt#L150 and it's usage.