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

Add Kotlin DSL extension function for configuring proto source directory set in Android builds #433

Conversation

voidzcy
Copy link
Collaborator

@voidzcy voidzcy commented Sep 9, 2020

The plugin is missing the proto Kotlin DSL extension for Android builds.

getSourceSets().all { sourceSet ->
String name = sourceSet.name
SourceDirectorySet sds = project.objects.sourceDirectorySet(name, "${name} Proto source")
sourceSet.extensions.add('proto', sds)
sds.srcDir("src/${name}/proto")
sds.include("**/*.proto")
}

For Android builds, project.android.sourceSets returns AndroidSourceSets. So in Kotlin DSL it expects the receiver of proto extension function to be AndroidSourceSet, which is missing now.

Note the argument of the extension function should be an action for SourceDirectorySet instead of for AndroidSourceDirectorySet. This is because the plugin created SourceDirectorySet for both Android and non-Android projects.

Majority part of this change is for adding the integration test for Android builds with Kotlin DSL. The test project includes configuring custom proto source directories. Reviewer may only need to review build.gradle, src/main/kotlin/com/google/protobuf/gradle/ProtobufConfiguratorExts.kt, src/test/groovy/com/google/protobuf/gradle/ProtobufKotlinDslPluginTest.groovy and testProjectAndroidKotlinDsl/build.gradle.kts.

Resolves #429.

@voidzcy voidzcy marked this pull request as ready for review October 20, 2020 01:17
@voidzcy voidzcy marked this pull request as draft November 16, 2020 00:28
@voidzcy voidzcy force-pushed the bugfix/add_android_kotlin_dsl_proto_source_directory_set_extension_function branch from 24ed5f0 to 474d575 Compare November 16, 2020 01:02
…o bugfix/add_android_kotlin_dsl_proto_source_directory_set_extension_function
@ejona86
Copy link
Collaborator

ejona86 commented Nov 18, 2020

@voidzcy, is this still supposed to be a draft?

@voidzcy
Copy link
Collaborator Author

voidzcy commented Nov 18, 2020

@voidzcy, is this still supposed to be a draft?

Yeah, I haven't tested it yet. Also, this changes adds android build tool for compileOnly dependency. Do we want it?

@ejona86
Copy link
Collaborator

ejona86 commented Nov 18, 2020

Also, this changes adds android build tool for compileOnly dependency.

I don't know Kotlin well enough to know the impact of that extension. I don't know what situations would trigger a NoSuchClassDefError or similar. If it is "safe" and "if that's the way you do it," then that can be fine.

@voidzcy voidzcy marked this pull request as ready for review December 14, 2020 05:42
@voidzcy voidzcy force-pushed the bugfix/add_android_kotlin_dsl_proto_source_directory_set_extension_function branch 4 times, most recently from 2e18731 to f202f01 Compare December 14, 2020 07:02
@voidzcy voidzcy force-pushed the bugfix/add_android_kotlin_dsl_proto_source_directory_set_extension_function branch from f202f01 to d3bbaef Compare December 14, 2020 07:07
@voidzcy voidzcy requested a review from ejona86 December 14, 2020 08:28
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

Successfully merging this pull request may close these issues.

No "proto" extension function for AndroidSourceSet (Gradle Kotlin DSL)
2 participants