Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use maven directly to download additional dependencies #1644

Merged
merged 4 commits into from
Dec 28, 2020

Conversation

adangel
Copy link
Contributor

@adangel adangel commented Nov 27, 2020

Use maven instead of aether to resolve additional dependencies. Aether is still used, but now indirectly by maven. Using maven directly allows to reuse all the configurations like repositories, mirrors, proxies and the local repository.

This fixes #1625 . A local test showed, that the additional dependencies like dokka-base, javadoc-plugin (and their dependencies) are downloaded correctly. If they are in the local repository already, now download is done again. This probably fixes #1408 as well.

Note: I've changed the code to only consider remote plugin repositories for downloading these artifacts, as this seems more correct - dokka-base/javadoc-plugin are in fact plugin dependencies and not project dependencies. This could be easily changed back, see comment.

I then removed the now unneeded aether dependencies. This seems to fix #1626. At least, if running maven with wagon-logging enabled, the httpclient logs are showing up now. I didn't test whether the TTL config is actually applied though. I assume, that one of these aether dependencies provided another plexus component for "org.sonatype.aether.RepositorySystem" which was picked up by maven in general and replaced the already existing component - thus maven didn't use anymore its bundled httpclient but the httpclient provided through transitive dependencies of aether.

Note: I was not able to do a full test of my use case: dokka-maven-plugin by default fetches e.g. dokka-base with the same version - on master 1.4.10.2-SNAPSHOT. That would mean, I need to publish these already into my local repo - thus the download would not be tried again, because it's already in my local repository. So I temporarily changed this to download the stable version 1.4.10.2 for dokka-base/javadoc-plugin instead. With that setting, I could test and see, that these additional artifacts are downloaded into the correct local maven repo. However, these stable versions don't seem to be compatible anymore with the current master (java.lang.NoSuchMethodError: 'org.jetbrains.dokka.plugability.ExtensionPoint org.jetbrains.dokka.CoreExtensions.getPreMergeDocumentableTransformer()'), so I actually never fully executed dokka-maven-plugin... [Updated: see comment below]

Aether is still used, but now indirectly by maven.
Using maven directly allows to reuse all the configurations
like repositories, mirrors, proxies and the local repository.
This throws a nice exception in case a artifact is missing.
Otherwise missing plugins are just ignored and dokka is
executed without them.
@adangel
Copy link
Contributor Author

adangel commented Nov 27, 2020

I could now test the complete use case and it works. The plugins are downloaded via maven's own wagon transport, allowing to set TTL and other http parameters.

First I didn't have all snapshots in my test snapshot repo, so javadoc-plugin was not found. I've added a resolutionErrorHandler to break the build if any artifact could not be resolved. Otherwise dokka still executes, but just without the missing plugins giving undesired results.

Copy link
Contributor

@kamildoleglo kamildoleglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! I only noticed a few minor issues, if you have time to fix them, please do ☺️

runners/maven-plugin/src/main/kotlin/DokkaMojo.kt Outdated Show resolved Hide resolved
runners/maven-plugin/src/main/kotlin/DokkaMojo.kt Outdated Show resolved Hide resolved
runners/maven-plugin/src/main/kotlin/DokkaMojo.kt Outdated Show resolved Hide resolved
runners/maven-plugin/src/main/kotlin/DokkaMojo.kt Outdated Show resolved Hide resolved
runners/maven-plugin/src/main/kotlin/DokkaMojo.kt Outdated Show resolved Hide resolved
@adangel
Copy link
Contributor Author

adangel commented Dec 14, 2020

@kamildoleglo Thanks for the review! I've pushed a fix.

@kamildoleglo kamildoleglo merged commit e55f3b0 into Kotlin:master Dec 28, 2020
@kamildoleglo
Copy link
Contributor

Thanks for this PR ☺️

@adangel adangel deleted the issue-1625-maven-session branch February 11, 2021 17:45
adangel added a commit to adangel/pmd that referenced this pull request Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants