use a Gradle logger via LoggerAdapter

[aSemy]

Adapt a Gradle Logger to a [DokkaLogger], to be used by Dokka when generating documentation.

Using the Gradle logger means that the log-level will be controlled by the standard Gradle command line options, e.g. --info.

• part of Improve compatibility with the Gradle API, and follow best practices #2700
• split off from Dokka Gradle Plugin: Implement Gradle Worker Daemon for Dokka generation #2740

[IgnatBeresnev]

The tests are failing with

Caused by: java.lang.NoSuchMethodException: org.jetbrains.dokka.DokkaBootstrapImpl.configure(java.lang.String,org.jetbrains.dokka.utilities.DokkaLogger)

I spent a few hours trying to understand the reason, as I thought that we were using different versions of core for compiling the Gradle plugin and for testing it, which would've meant that our tests weren't checking changes in core and that would've been a huge problem.

Fortunately, this is not the case. It was difficult to understand which versions were used, but TLDR is the following: The Gradle plugin is published with the version for-integration-tests-SNAPSHOT, and it has a runtime dependency to core of the dokka_version version, which is 1.8.0-SNAPSHOT in our case now - and it's published before running the integration tests. So it's using the correct artifact/version.

The real reason, of course, is the proxy reflection magic in the current Gradle plugin.

It tries to match the same method by the name and the parameter types. However, the equals check for parameter types returns null because java.lang.Class is only compared by reference, and we are comparing the same class (DokkaLogger) which was loaded twice. Notice that String was only loaded once, so it explains why the current approach with BiFunction is working as it's also from java core.

So it looks like we can only add the logger once we migrate to workers in #2740, it's not possible to do without it :( Even if we match the methods, it'll fail during invocation with incompatible types, referring to the same difference in loaded classes (trying to pass an object of class1 to a method accepting class2, even though we know it's the same class).

Sorry for asking you to extract it and wasting your time - I thought it would simplify review and we'll be able to benefit from it earlier, didn't expect we'd get this problem :(

If you didn't delete this code from #2740 (doesn't look like that), I think we can close the PR. If you did, we can merge it after merging #2740

[aSemy]

Ah thanks for looking into it. I just assumed the tests were flaky.

I'm not sure I follow your explanation though... don't the integration tests patch KxS and Coroutines to use the for-integration-tests-SNAPSHOT Dokka Gradle Plugin? And why does biojava pass?

Anyway, there's no problem with closing this PR and combining it with #2740 - although merging this PR separately would make #2740 smaller?

[IgnatBeresnev]

And why does biojava pass?

It uses the Maven runner, so it doesn't have the reflection magic

don't the integration tests patch KxS and Coroutines to use the for-integration-tests-SNAPSHOT Dokka Gradle Plugin?

They do. The problem is not with the plugin version or the core version as I feared initially. The integration tests use correct artifacts and as expected.

The problem is caused by this line.

As far as I understand, it tries to add a bridge for "the same" method between classes loaded in different classloaders. So you call class1#configure and it gets re-routed to class2#configure, where class1 and class2 are different class instances of DokkaBootstrapImpl.

To find the configure method, it compares the method name (returns true) and the class types of the parameters. The problem is that the class for the second parameter (DokkaLogger) was loaded twice (see the debug screenshot and compare Class@xxx), and classes are compared by references, so the same configure method is seen as different and thus it fails with NoSuchMethodException.

It doesn't happen with classes from java core (String, BiFunction) for some reason, presumably because they are loaded once (see Class@xxx in the screenshot) and so the method can be found without a problem.

Hopefully I didn't just repeat myself and it added some clarity 😅

Anyway, there's no problem with closing this PR and combining it with #2740 - although merging this PR separately would make #2740 smaller?

It would, but if we merge this now I think the Gradle plugin as a whole will be broken, not just the tests

[aSemy]

Hopefully I didn't just repeat myself and it added some clarity 😅

Definitely not! It's a lot clearer now. Thanks very much for explaining.

I've pushed a change with some old code I had for duck-typing, maybe that fixes it. If not, I'll close this PR and it'll be picked up in #2740.