Add support for @file:MavenCredentials() annotation

[leonschreuder]

I've added rudimentary support for supplying credentials to a MavenRepository.

The basic usage is to define a new Annotation @file:MavenCredentials() supplying an id, a username and a password. The credentials are then applied on all MavenRepository definitions that have the same ID. Analog to how Maven links the credentials to repositories.

This fixes #143

The Annotation needs to be added to https://github.com/holgerbrandl/kscript-annotations to make the scripts compile. The download already runs successfully.

[holgerbrandl]

Thanks. However, didn't we agree that it would be better style to put them into a config as described in #143.

What about testing? Is there a public repo we could use for regression testing?

In general I think it might be better to add optional parameters to @mavenrepository in https://github.com/holgerbrandl/kscript-annotations/blob/master/src/main/kotlin/ScriptDirectives.kt#L47

With your the current impl, we could for instance not have differing credentials for multiple repos in the same script.

[leonschreuder]

Thanks. However, didn't we agree that it would be better style to put them into a config as described in #143.

Since I wanted to use the credentials from our Build environment and have the credentials be passed, but not be saved, I thought it would be a better solution to set the properties through environment variables in the script directly. I think having the option to use either saved credentials through a config or through direct credentials would be ideal.

What about testing? Is there a public repo we could use for regression testing?

I am unsure how we could test this other then verifying we pass the right config to aether as I did in the unittests. I tested it from my own environment of course, but I can do some research if there is some public repository that requires credentials for you/others to test on.

In general I think it might be better to add optional parameters to @mavenrepository in https://github.com/holgerbrandl/kscript-annotations/blob/master/src/main/kotlin/ScriptDirectives.kt#L47

I tried that first, but found it rather ugly passing 4 unnamed string-parameters. And since the Maven xml settings require a <server> with the credentials bound to an <id>, and this id then simply needs to match the <repository> <id>, I thought we could do this the same way, which I found rather elegant.
I can of course rewrite it back to passing optional parameters if you are not convinced.

With your the current impl, we could for instance not have differing credentials for multiple repos in the same script.

That is true, but is also true for the maven implementation. Unless I missed something in the maven implementation of course.

[leonschreuder]

So I searched around, but I couldn't find a publicly available test-artifactory or anything similar we could use. So in my opinion we have three options:

1. Writing "how to test manually" documentation and leave it at that.
2. And/or write tests that test up until calling aeather as I've already done.
3. Have Travis manually start up an Artifactory server instance, configure it, and run tests with that.

Option 3 would be best in my opinion, but is also harder to maintain and would make the travis-tests really slow. I don't know if this is acceptable in your view.

[holgerbrandl]

I think using environment properties for the build environment is another feature (and should imho be a different PR). Let's focus on the annoations first if this fine with you.

You can have named annoations arguments in kotlin, so we can do

@file:MavenRepository("imagej-releases", "http://some/where", user="foo", pw="bar")

which is legit and much more elegant compared to having the credentials free-floating around and being ambiguous.

To build a complete PR, we/you need to also adjust https://github.com/holgerbrandl/kscript-annotations accordingly for correct tooling and completion. Actually this must be the first step otherwise kscript will not compile the user-script. (... which makes me curious how you tested the PR in the first place?).

For testing optio 3 may work because we could simply use a dockerized artifactory in travis. But this would be a lot of work. So maybe we start withmanual tests (option 1) which are based on some docker-artifcatory which the developer would need to launch manually. I'd take care of the travis-ci inclusion later.

Does this sound reasonable to you?

[leonschreuder]

I did not know about the named annotations arguments. That is way more elegant!

I rewrote my PR to use the named arguments as you suggested, and added a PR in kscript-annotations as well: kscripting/kscript-annotations#1

(... which makes me curious how you tested the PR in the first place?).

I could test the resolving of the dependencies and their download with credentials through my company's Artifactory, but the missing annotation made the compiling of the script fail after the download of course.

So maybe we start with manual tests (option 1) which are based on some docker-artifcatory which the developer would need to launch manually.

I agree and will write a manual testing how-to next.

P.S. The build fails because I updated the version to "com.github.holgerbrandl:kscript-annotations:1.3" which matches that of the PR I created in the kscript-annotations plugin.

[holgerbrandl]

I've updated and released the annotations v1.3. Could you test your PR against it or should it work already? Please note that I have changed pass to password in the argument list of the annotation.

Since I have no pw-protected repo at my disposal, I'd be glad if you could outline how I could spin one up via docker or use public one (with some secret credentials which would not go into this repo).

I think we're on a very good track here. :-)

[leonschreuder]

Spinning up an Artifactory instance with credential-protected Artifacts, that works without manual configuration was more of a pain than I realized, but I got it to work.

I put a description and the relevant commands, with some reference links in the test/TestReadme.md.

The description is almost a script already, so hopefully you can integrate it in your automatic tests without to much additional work.

[leonschreuder]

Two of the tests fail, because the @file:CompilerOpts() annotation is not recognised by the compiler. I didn't change anything there, and I couldn't find it in the kscript-annotations. Do you know what is going wrong here?

[kscript] [ERROR] compilation of 'compiler_opts_with_includes_master.kt' failed
/tmp/tmp273567797891487621.tmp/scriptlet.9c8351b9ef4e6b8a.kt:1:7: error: unresolved reference: CompilerOpts

[leonschreuder]

The problem appears to be a missing annotation. Created PR: kscripting/kscript-annotations#2

[holgerbrandl]

Fixed issue with kscript-annotations. The new version is v1.4 as suggested in your PR.

[leonschreuder]

I upgraded the kscript-annotations to 1.4 and it all seems to work now.
As I wrote above, the manual testing description in the markdown file is almost a script already, so you should be able to test it easily and probably even add it to your test-suite without much work.

[leonschreuder]

Could you take a little time to check this PR? I'd like to use this in our Build-Environment and currently have to ask my Devs to patch their installation locally.

[holgerbrandl]

Sorry for the delay. I plan to work on it this weekend if my wife allows it. :-)

[exaV]

Is there anything I can do to speed this up? It would be really useful for us to be able to reference internal libraries in scripts

[holgerbrandl]

Thanks for your patience and even more so for providing this great PR. The provided manual test instructions worked well for me, all tests seemed fine, so I've merged the PR and released it in v2.8.0.