Provide Kotlin samples alongside existing Groovy samples in Gradle plugin documentation

[jnizet]

On top of #14680, which allows setting the classpath from the gradle runner. Only the last 2 commits of this PR are new. The first ones is part of #14680.

This PR translate all the gradle plugin samples to Kotlin.

It also makes the following changes:

• refactor the tests to keep them DRY
• use a pluginsManagement block in settings for snapshot and milestone versions and gradle version >= 4.10
• add a section showing how to apply the dependency management plugins declaratively

fixes #14559

[wilkinsona]

Thanks for the PR. There’s a lot of good stuff in here, but the changes are quite a bit broader than #14559. We prefer to focus issues and pull requests as narrowly as possible. That’s particularly important here as the proposed changes rely on enhancements that have appeared in different versions of Gradle. For example, the plugins block only started supporting snapshots in Gradle 4.9 (IIRC). In Boot 2.1 we support Gradle 4.4 and later and I think it would be best for the documentation to provide at least Groovy examples that work with that version. If that’s not practical with the Groovy/Kotlin tabs then we may need to reconsider. It’ll be easier to do so with smaller, more isolated sets of changes.

Can we please take a step back and consider each chunk of changes in isolation? Some of them (moving from compile to implementation for example, may even belong in 2.0.x.

[jnizet]

Oh sorry for that.

All the groovy tests pass with gradle 4.4. But indeed, using the pluginManagement stuff in settings.gradle for snapshots only works since 4.10.

Using the plugins block instead of using apply to apply the plugin is important for Kotlin, because otherwise, none of the extensions provided by the plugin are available in the DSL. That's why I chose to use that consistently: almost the other Kotlin samples would fail if the plugin was not applied using the plugins block.

So maybe I can

• make a separate PR that only removes the usages of the deprecated configurations
• rebase this PR on the new PR (to avoid using the deprecated configurations in the new kotlin samples)
• add an introduction saying that the Groovy samples are compatible with gradle 4.4 and above, and that all the Kotlin samples are compatible with gradle 4.10 and above
• leave all the samples as is, except the 2 snapshot and milestone samples of the getting started section. For these samples, we can provide two sets of samples: one only for groovy as it was before, that must be used for gradle versions less than 4.10, and the ones I introduced that can be used for version 4.10 and above.

Please tell me what you think.

[jnizet]

I added a commit showing what I have in mind. It also fixes all the non-documentation tests that I broke by refactoring GradleBuild a bit too agressively (duh!).

[wilkinsona]

The approach described in the four bullets above sounds good to me. Thanks.

[jnizet]

Changes done. Ready for review.

[wilkinsona]

Thanks. Things are definitely better now, but e9b962f is still a bit too broad to be merged as-is. For example, the change to how the plugin classpath looks like a good thing to do, but it's unrelated to applying Kotlin examples in the documentation. It's also confusing to me in its current form as sometimes the classpath is configured in the test, and sometimes the classpath is configured in the build script. I'd prefer that the classpath is configured consistently across the whole module. Do you have time to break things up further? I'd really like to keep the changes in this PR focussed solely on adding Kotlin examples.

Also, I wonder if a Suite-based approach could be taken for testing the Groovy and Kotlin examples that are included in the documentation. This would align them with the approach that's been taken in the integration tests where GradleCompatibilitySuite is used to run against different versions of Gradle. The suite would run the tests using both a .gradle file and a .gradle.kts file and would avoid the need for creating a separate test class for testing the two languages for each example.

[jnizet]

the change to how the plugin classpath looks like a good thing to do, but it's unrelated to applying Kotlin examples in the documentation

I think there is a misunderstanding here. I didn't do this change just for the sake of it. I did it for two reasons (the second one being the most important):

• applying a plugin using the imperative apply() method (unless you're using it from a parent project to apply it to subprojects) is now considered legacy (see https://docs.gradle.org/current/userguide/plugins.html#sec:old_plugin_application). Gradle folks could probably explain the rationale better than I can (ping @eriwen)
• applying the plugin using the imperative apply() method, specifying its classpath in the buildScript block, doesn't make it possible to use the Kotlin DSL in the same idiomatic way as you would use the Groovy DSL.

Let's take an example to illustrate the second point. Consider the following example using the Groovy DSL:

buildscript {
	dependencies {
		classpath files(pluginClasspath.split(','))
	}
}

apply plugin: 'org.springframework.boot'
apply plugin: 'java'

springBoot {
	buildInfo()
}

This works fine in Groovy. Doing the same thing in Kotlin would be:

buildscript {
	dependencies {
		classpath(files((project.property("pluginClasspath") as String).split(",")))
	}
}

apply(plugin = "org.springframework.boot")
apply(plugin = "java")

springBoot {
	buildInfo()
}

But unfortunately, the above Kotlin build script would not compile. Because the springBoot method wouldn't exist. The springBoot method only exists if the Spring Boot plugin, which adds the springBoot extension to the project, is applied from a parent project, or if it is applied declaratively, using the plugins block (see https://guides.gradle.org/migrating-build-logic-from-groovy-to-kotlin/#applying_plugins).

That's the main reason I'm using the plugins block. I could of course keep the groovy samples as they are, but that would make them inconsistent with their Kotlin equivalent, which I would find weird and confusing.

Let's get back to the Kotlin sample. If I want to use the plugins block to apply the plugin, in order to be able to use the idiomatic DSL (i.e. springBoot { } and not some ugly workaround like the<SpringBootExtension>().apply { } or configure<SpringBootExtension>() { }), since the plugin version we're documenting doesn't exist in the plugins portal, we need to use the TestKit to set the classpath. We can't set it from the build script. That's why I set it in the test and not in the build script.

sometimes the classpath is configured in the test, and sometimes the classpath is configured in the build script

I hope the explanation above is clear enough. AFAIK, there is no way to use the build script to set the classpath if we want to use the Kotlin DSL in the idiomatic way. So we need to apply the classpath from the test for the Kotlin samples.
For the sake of consistency, all the documentation samples, inclusing the Groovy ones, use the classpath set from the test, and don't set the classpath in the build script anymore.
I could do the same for all the other samples of the project, but then the changes would be even broader, and would go much further than the documentation samples. So I left the non-documentation samples untouched.

I wonder if a Suite-based approach could be taken for testing the Groovy and Kotlin examples that are included in the documentation.

I'll try to look into that and see if I can do something similar. But I'm not sure I will be able to do that before some time: I'll be on holiday for a week starting next Friday 🎉

Regarding the classpath, I hope the above explanations help understand why I did thing the way I did them. If you're not convinced, please advise on what I should do.

FYI: the whole Gradle user guide (in the nightly release of Gradle) now consistently use the plugins block instead of the apply method in both Groovy and Kotlin samples. And I opened an issue (after discussing it on the gradle Slack) to be able to use the plugins block to apply the dependency management plugin bundled with the boot plugin.

[jnizet]

I refactored the documentation tests to use a Suite.

[eriwen]

Yes, that's correct @jnizet. Thanks very much for your PRs here.

My understanding is that in order to stay type-safe, Gradle's Kotlin DSL generates accessors (e.g. springBoot {}) and this only is possible when using the plugins {} block. (I'm now wishing Gradle had a really good resource to explain why this is).

Perhaps there are other parts of the PR that @wilkinsona is mentioning such as some formatting that would need to be split into a separate PR.

All that said, would it be inadvisable to use configure<SpringBootExtension> {} in examples? This would be consistent regardless of how the plugin is applied. I suppose that question also is for @eskatos, as it is his team's job to set the example for Kotlin DSL.

[wilkinsona]

Thanks for the additional explanation, @jnizet. I don't think there's much of a misunderstanding, and I realise that you aren't just changing things go the sake of it.

What I'm really looking for is a more fine-grained set of changes. If a change has merit on its own, I'd like it to at least be in its own commit and perhaps even in its own PR depending on the scope of the change. I consider a change to have individual merit if it's beneficial in isolation and is something that we could ship on its own.

[jnizet]

So, how should I split this PR? Here are the options that I can think of based on your previous comment.

• Option 1: make a first PR where the classpath is not set in the groovy build scripts anymore, but set in the test, and do that for the build scripts of the documentation samples only, to avoid making the change too broad?
• Option 2: make a first PR where the classpath is not set in the groovy build scripts anymore, but set in the test, and do that for all the build scripts, including non-documentation build scripts, in order to avoid the confusion of having the classpath sometimes configured in the test, and sometimes configured in the build script?
• Option 3: Something else (but then please elaborate)?

[wilkinsona]

Option 2 makes most sense to me. It's a worthwhile, isolated change that also keeps things consistent.

[jnizet]

Cool, thanks. I'll try doing it quickly.

[jnizet]

I've made a separate PR which sets the classpath from the gradle runner: #14680.
This PR has been rebased on #14680.

[wilkinsona]

Thanks for the changes in #14680. I've merged those into master. Could you please rebase this on master and force push it so that it just contains the two commits related to the Kotlin DSL examples?

[jnizet]

Rebase done @wilkinsona

[wilkinsona]

Thanks very much, @jnizet. I've merged the changes into master. If you'd like to take a look, I tweaked things a little bit in a3d2f3f, primarily to centralise the handling of the DSL's filename extension.

[jnizet]

Great news. Thanks. 🎉

The polish is great. Thanks for letting me know.

I was afraid putting the extension management in GradleBuild would force me to touch all the non-documentation tests, but I should have looked at their implementation more closely.

[pioterj]

I can already see this at https://docs.spring.io/spring-boot/docs/2.1.0.BUILD-SNAPSHOT/gradle-plugin/reference/html/. Great job, @jnizet, thank you!