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

Make plugin more "kotlin-dsl" friendly #219

Closed
mkobit opened this issue Mar 25, 2018 · 10 comments
Closed

Make plugin more "kotlin-dsl" friendly #219

mkobit opened this issue Mar 25, 2018 · 10 comments

Comments

@mkobit
Copy link
Contributor

mkobit commented Mar 25, 2018

The kotlin-dsl project is an alternative way to write statically typed build scripts for Gradle in Kotlin.
As of right now, using this plugin requires a bit of knowledge of how the kotlin-dsl generates code and also how to manipulate Groovy types through Kotlin.

Example:

plugins {
  id("com.google.protobuf") version "0.8.5"
}

tasks {
  "wrapper"(Wrapper::class) {
    gradleVersion = "4.6"
  }
}

// protbuf {} in kotlin-dsl a generated accessor that is different than its Groovy counterpart
protobuf.protobuf.run {
}
@mkobit
Copy link
Contributor Author

mkobit commented Mar 26, 2018

More complete example of attempting to convert the grpc-java example:

protobuf.protobuf.run {
  protoc(delegateClosureOf<ExecutableLocator> {
    artifact = "com.google.protobuf:protoc:3.5.1-1"
  })
  plugins(delegateClosureOf<NamedDomainObjectContainer<ExecutableLocator>> {
    this {
      "grpc" {
        artifact = "io.grpc:protoc-gen-grpc-java:${grpcVersion}"
      }
    }
  })
  generateProtoTasks(delegateClosureOf<ProtobufConfigurator.GenerateProtoTaskCollection> {
    all().forEach {
      it.plugins(delegateClosureOf<NamedDomainObjectContainer<GenerateProtoTask.PluginOptions>> {
        this {
          "grpc"()
        }
      })
    }
  })
}
  • The protobuf.protobuf would be ideally replaced by just allowing for protobuf {}
  • The delegateClosureOf things can mostly be dealt with by changing the methods from Closure accepting to Action<? extends THING> - the kotlin-dsl uses a Kotlin compiler plugin to make those SAM-receiver style methods
  • the this {} can't entirely be solved, yet. Adding a getPlugins() getter-style that returns the NamedDomainObjectContainer<ExecutableLocator> (I think) would make users be able to use this workaround. https://github.com/gradle/kotlin-dsl/issues/459 is also tracking the overall fix.

@zpencer
Copy link
Contributor

zpencer commented Jun 7, 2018

PRs are very welcome :) If minor non API breaking changes here makes it easier for for the kotlin DSL then I'm all for it.

@mkobit
Copy link
Contributor Author

mkobit commented Jun 18, 2018

  • Is there any API leeway from not being 1.0 yet?
  • For API breaking changes, are you mostly concerned with consumers in build scripts or are you concerned with users who have compiled/built tools on top of this plugin (like possibly Android)?
  • Also, are test* project directories enough to verify this change?

Hoping to understand the scope before I take a crack at this.

@zpencer
Copy link
Contributor

zpencer commented Jun 18, 2018

  • Even though the plugin is not 1.0, there are users relying on the plugin to build their projects so we should not break them.
  • I'm mostly concerned about existing build scripts
  • The test* projects are enough to ensure the code gen outputs are visible to the targets. I would have to compare the specific changes with the projects to see if there is coverage.

@marcoferrer
Copy link
Contributor

If adding kotlin as a dependency isnt out of the question, alot of the work can be achieved with clever extension functions. This would allow the groovy api to remain untouched. I can give it a shot for curiosity sake if your interested

@mkobit
Copy link
Contributor Author

mkobit commented Aug 23, 2018

That might not be a bad idea. My thinking was that this would be a good time to try and address some other things (like afterEvaluate need, ability to use configuration avoidance APIs in newer versions of Gradle, using other configuration APIs (for example gradle/gradle#5664 (comment))).

If the main concern is not breaking the build script DSL for consumers, I think that can probably be satisfied. I was hoping to take a crack at some pieces of this in the next couple weeks since the kotlin-dsl is getting close to 1.0, but if others get to it first that would be awesome as well.

@marcoferrer
Copy link
Contributor

Ill try to open a pr a little early just for review purposes. But this is generally the approach I was thinking.

fun Project.protobuf(configuration: ProtobufConfigurator.() -> Unit) {
    project.convention.getPlugin(ProtobufConvention::class.java).protobuf.apply(configuration)
}

fun ProtobufConfigurator.protoc(closure: ExecutableLocator.()->Unit){
    protoc(delegateClosureOf(closure))
}

fun ProtobufConfigurator.plugins(closure: NamedDomainObjectContainer<ExecutableLocator>.()->Unit){
    plugins(delegateClosureOf<NamedDomainObjectContainer<ExecutableLocator>> {
        this(closure)
    })
}

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

@anti-social
Copy link

Can somebody help to configure protobuf plugin for generating grpc service?

The solution from #219 (comment) doesn't work with gradle 4.10. It produces next error:

org.gradle.api.UnknownDomainObjectException: PluginOptions with name 'grpc' not found.

@anti-social
Copy link

Copied some functions from #262 ProtobufConfiguratorExts.kt, took the example and now it works fine with gradle 4.10. Excuse for troubling.

@marcoferrer
Copy link
Contributor

@zhangkun83 This can be closed. Its resolved via #275

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants