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

Update to stable kotlin dsl #275

Merged
merged 6 commits into from
Dec 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ buildscript {
Stand-alone examples are available for each of gradle's supported languages.
* [Groovy](https://github.com/google/protobuf-gradle-plugin/tree/master/examples/exampleProject) ___(Default)___
* Run `../../gradlew build` under the example directory to test it out.
* [Kotlin (experimental)](https://github.com/google/protobuf-gradle-plugin/tree/master/examples/exampleKotlinDslProject)
* [Kotlin](https://github.com/google/protobuf-gradle-plugin/tree/master/examples/exampleKotlinDslProject) ___(Experimental)___
* Run `./gradlew build` under the Kotlin example directory to test it out. This example is set up with Gradle 4.10, the minimum required version.


Expand Down
9 changes: 7 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ buildscript {
classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.6'
classpath "com.gradle.publish:plugin-publish-plugin:0.9.7"
classpath "com.github.ben-manes:gradle-versions-plugin:0.12.0"
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.2.0"
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.3.10"
}
}

repositories {
maven { url "https://plugins.gradle.org/m2/" }
maven { url "https://repo.gradle.org/gradle/libs-releases-local/" }
google()
// Needed for resolving 'kotlinx-metadata-jvm'
// A transitive dependency of gradle Kotlin DSL
maven { url "https://kotlin.bintray.com/kotlinx/" }
}

configurations {
Expand Down Expand Up @@ -68,7 +71,7 @@ task createClasspathManifest {
dependencies {
compileOnly gradleApi()
compileOnly localGroovy()
compileOnly "org.gradle:gradle-kotlin-dsl:1.0-rc-6"
compileOnly "org.gradle:gradle-kotlin-dsl:1.0.4"

compile 'com.google.guava:guava:18.0'
compile 'com.google.gradle:osdetector-gradle-plugin:1.4.0'
Expand All @@ -83,6 +86,8 @@ dependencies {
}
testCompile 'commons-io:commons-io:2.5'

// This kotlin version needs to be compatible with the
// android plugin being used in the test runtime.
testProjectRuntime "org.jetbrains.kotlin:kotlin-gradle-plugin:1.2.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bumping this dependency version is what was causing the failures in the ci. The test runtime kotlin version needs to be compatible with the older version of the android plugin. I didnt need to update it in the first place so its fine being set to 1.2.0


// Add android plugin to the test classpath, so that we can jump into class definitions,
Expand Down
9 changes: 3 additions & 6 deletions examples/exampleKotlinDslProject/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import org.gradle.kotlin.dsl.provider.gradleKotlinDslOf

// A minimal example Java project that uses the protobuf plugin.
// To build it:
// $ ../gradlew clean build
// $ ./gradlew clean build
plugins {
java
idea
id("com.google.protobuf") version "0.8.7-SNAPSHOT"
id("com.google.protobuf") version "0.8.8-SNAPSHOT"
}

repositories {
Expand Down Expand Up @@ -61,10 +61,7 @@ protobuf {
generateProtoTasks {
ofSourceSet("main").forEach {
it.plugins {
// Apply the "grpc" plugin whose spec is defined above, without
// options. Note the braces cannot be omitted, otherwise the
// plugin will not be added. This is because of the implicit way
// NamedDomainObjectContainer binds the methods.
// Apply the "grpc" plugin whose spec is defined above, without options.
id("grpc")
}
}
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.google.protobuf.gradle;

import kotlin.Unit;
import kotlin.jvm.functions.Function1;
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.kotlin.dsl.NamedDomainObjectContainerExtensionsKt;
import org.gradle.kotlin.dsl.NamedDomainObjectContainerScope;

public class KtDslCompatibilityUtils {

/**
* This method is used to delegate the NamedDomainObjectContainer configuration to
* to which ever kotlin-dsl ext implementation is available on the classpath.
*
* Since NamedDomainObjectContainerExtensionsKt.invoke is an inline extension function, our
* usages in 'ProtobufConfiguratorExts.kt' would use the byte code for which ever kotlin-dsl
* version we compiled against. This caused issues with providing compatibility with
* kotlin-dsl 1.0.0-rc3 and 1.0.4 (Stable).
*
* Since the kotlin compiler creates a static implementations of all extensions, we can create a
* delegating utility function that ensures we are not inline-ing our kotlin-dsl compilation
* target byte code.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I still don't understand why this additional layer of indirection would prevent the problem. Doesn't this method inline the invoke() call too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In-lining can only be performed by the Kotlin compiler, and this is file is compiled via the java compiler. The method is resolvable from java due to Kotlins interop features. Since inline methods dont exist in java, Kotlin will create a static implementation of the method for usage from java sources.

This work around solves the compatibility issue because the method contract didnt change between ktdsl versions, only its implementation.

*
* @param container Container to apply the scope configuration
* @param block A kotlin lambda to apply to the NamedDomainObjectContainerScope
* @return A NamedDomainObjectContainer with the block lambda applied to is NamedDomainObjectContainerScope
*/
static <T> NamedDomainObjectContainer<T> configureNamedDomainObjectContainer(
NamedDomainObjectContainer<T> container,
Function1<? super NamedDomainObjectContainerScope<T>, Unit> block){

return NamedDomainObjectContainerExtensionsKt.invoke(container, block);
}
}
153 changes: 131 additions & 22 deletions src/main/kotlin/com/google/protobuf/gradle/ProtobufConfiguratorExts.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,61 +4,170 @@ import org.gradle.api.NamedDomainObjectContainer
import org.gradle.api.Project
import org.gradle.api.plugins.ExtensionAware
import org.gradle.api.tasks.SourceSet
import org.gradle.kotlin.dsl.*

import org.gradle.kotlin.dsl.NamedDomainObjectContainerScope
import org.gradle.kotlin.dsl.closureOf
import org.gradle.kotlin.dsl.get

/**
* Applies the supplied action to the project's instance of [ProtobufConfigurator].
*
* @since 0.8.7
* @usage
* ```
* protobuf {
* ...
* generatedFilesBaseDir = files(...)
* }
* ```
*
* @receiver [Project] The project for which the plugin configuration will be applied
* @param action A configuration lambda to apply on a receiver of type [ProtobufConfigurator]
*
* @return [Unit]
*/
fun Project.protobuf(action: ProtobufConfigurator.()->Unit) {
project.convention.getPlugin(ProtobufConvention::class.java).protobuf.apply(action)
}

/**
* Applies the supplied action to the [ProtobufSourceDirectorySet] extension on
* a receiver of type [SourceSet]
*
* @since 0.8.7
* @usage
* ```
* sourceSets {
* create("sample") {
* proto {
* srcDir("src/sample/protobuf")
* }
* }
* }
* ```
*
* @receiver [SourceSet] The source set for which the [ProtobufSourceDirectorySet] extension
* will be configured
*
* @param action A configuration lambda to apply on a receiver of type [ProtobufSourceDirectorySet]
* @return [Unit]
*/
fun SourceSet.proto(action: ProtobufSourceDirectorySet.() -> Unit) {
(this as? ExtensionAware)
?.extensions
?.getByType(ProtobufSourceDirectorySet::class.java)
?.apply(action)
}

fun ProtobufConfigurator.protoc(closure: ExecutableLocator.() -> Unit) {
protoc(closureOf(closure))
/**
* Uses the supplied action to configure the [ExecutableLocator] for protoc.
*
* @since 0.8.7
* @usage
* ```
* protobuf {
* protoc {
* artifact = "com.google.protobuf:protoc:3.6.1"
* }
* }
* ```
*
* @receiver [ProtobufConfigurator] The protobuf plugin configuration instance
* for the project.
*
* @param action A lambda with receiver of type [ExecutableLocator] used
* for configuring the locator for protoc
*
* @return [Unit]
*/
fun ProtobufConfigurator.protoc(action: ExecutableLocator.() -> Unit) {
protoc(closureOf(action))
}

fun ProtobufConfigurator.plugins(closure: NamedDomainObjectContainerScope<ExecutableLocator>.() -> Unit) {
fun ProtobufConfigurator.plugins(action: NamedDomainObjectContainerScope<ExecutableLocator>.() -> Unit) {
plugins(closureOf<NamedDomainObjectContainer<ExecutableLocator>> {
this(closure)
KtDslCompatibilityUtils.configureNamedDomainObjectContainer<ExecutableLocator>(this, action)
})
}

fun <T : Any> NamedDomainObjectContainerScope<T>.id(id: String, closure: (T.() -> Unit)? = null) {
closure?.let { create(id, closureOf(it)) }
?: create(id)
}

fun <T : Any> NamedDomainObjectContainerScope<T>.remove(id: String) {
remove(this[id])
}

fun ProtobufConfigurator.generateProtoTasks(closure: ProtobufConfigurator.GenerateProtoTaskCollection.()->Unit) {
generateProtoTasks(closureOf(closure))
fun ProtobufConfigurator.generateProtoTasks(action: ProtobufConfigurator.GenerateProtoTaskCollection.()->Unit) {
generateProtoTasks(closureOf(action))
}

fun GenerateProtoTask.builtins(closure: NamedDomainObjectContainerScope<GenerateProtoTask.PluginOptions>.()->Unit) {
fun GenerateProtoTask.builtins(action: NamedDomainObjectContainerScope<GenerateProtoTask.PluginOptions>.()->Unit) {
builtins(closureOf<NamedDomainObjectContainer<GenerateProtoTask.PluginOptions>> {
this(closure)
KtDslCompatibilityUtils.configureNamedDomainObjectContainer<GenerateProtoTask.PluginOptions>(this, action)
})
}

fun GenerateProtoTask.plugins(closure: NamedDomainObjectContainerScope<GenerateProtoTask.PluginOptions>.()-> Unit) {
fun GenerateProtoTask.plugins(action: NamedDomainObjectContainerScope<GenerateProtoTask.PluginOptions>.()-> Unit) {
plugins(closureOf<NamedDomainObjectContainer<GenerateProtoTask.PluginOptions>> {
this(closure)
KtDslCompatibilityUtils.configureNamedDomainObjectContainer<GenerateProtoTask.PluginOptions>(this, action)
})
}

/**
* An extension for creating and configuring the elements of an instance of [NamedDomainObjectContainer].
*
* @since 0.8.7
* @usage
* ```
* protobuf {
* plugins {
* id("grpc") {
* artifact = "io.grpc:protoc-gen-grpc-java:1.15.1"
* }
* }
* }
* ```
*
* @receiver [NamedDomainObjectContainerScope] The scope of the [NamedDomainObjectContainer]
* on which to create or configure an element.
*
* @param id The string id of the element to create or configure.
* @param action An optional action that will be applied to the element instance.
*
* @return [Unit]
*/
fun <T : Any> NamedDomainObjectContainerScope<T>.id(id: String, action: (T.() -> Unit)? = null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two methods arent new. I just moved them down in the file and documented their usage

action?.let { create(id, closureOf(it)) }
?: create(id)
}


/**
* An extension for removing an element by id on an instance of [NamedDomainObjectContainer].
*
* @since 0.8.7
* @usage
* ```
* protobuf {
* generateProtoTasks {
* ofSourceSet("main").forEach {
* it.builtins {
* remove("java")
* }
* }
* }
* }
* ```
*
* @receiver [NamedDomainObjectContainerScope] The scope of the [NamedDomainObjectContainer]
* on which to remove an element.
*
* @param id The string id of the element to remove.
*
* @return [Unit]
*/
fun <T : Any> NamedDomainObjectContainerScope<T>.remove(id: String) {
remove(this[id])
}

/**
* The method generatorProtoTasks applies the supplied closure to the
* instance of [ProtobufConfigurator.GenerateProtoTaskCollection].
*
* Since [ProtobufConfigurator.JavaGenerateProtoTaskCollection] and [ProtobufConfigurator.AndroidGenerateProtoTaskCollection]
* each have unique methods, and only one instance in allocated per project, we need a way to statically resolve the
* each have unique methods, and only one instance is allocated per project, we need a way to statically resolve the
* available methods. This is a necessity since Kotlin does not have any dynamic method resolution capabilities.
*/

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import spock.lang.Specification
* Unit tests for kotlin dsl extensions.
*/
class ProtobufKotlinDslPluginTest extends Specification {
private static final List<String> GRADLE_VERSIONS = ["4.10"]
private static final List<String> GRADLE_VERSIONS = ["4.10", "5.0"]

void "testProjectKotlinDsl should be successfully executed (java-only project)"() {
given: "project from testProjectKotlinDslBase"
Expand Down
2 changes: 1 addition & 1 deletion testProjectKotlinDslBase/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ tasks {
from(sourceSet.output)

val compileTaskName = sourceSet.getCompileTaskName("java")
dependsOn(tasks.getByName(compileTaskName))
dependsOn(project.tasks.getByName(compileTaskName))
Copy link
Contributor Author

@marcoferrer marcoferrer Nov 17, 2018

Choose a reason for hiding this comment

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

An additional extension was added to the task container in 1.0.3. So in order to deal with ambiguity in method resolution I had to explicitly qualify the tasks property.

}
}

Expand Down