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

Use new public API to create SourceDirectorySet available since Gradle 5.0 #292

Merged
merged 4 commits into from
Jan 11, 2019

Conversation

zhangkun83
Copy link
Collaborator

Resolves #280

@zhangkun83 zhangkun83 requested a review from ejona86 January 10, 2019 23:14
String name = sourceSet.name
if (Utils.compareGradleVersion(project, "5.0") < 0) {
// TODO(zhangkun83): remove dependency on Gradle internal APIs once we drop support for < 5.0
sourceSet.extensions.create('proto', ProtobufSourceDirectorySet, sourceSet.name, fileResolver)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to use ProtobufSourceDirectorySet instead of DefaultSourceDirectorySet? It seems like using DefaultSourceDirectorySet here would make the else more clear. Backward compatibility?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we used DefaultSourceDirectorySet here, we would have to configure() it after it's created. Unfortunately, the only configure() available on Gradle 3.0 is

<T> void configure​(Class<T> type,
                   Action<? super T> action)

That means a custom type would still be necessary.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, because we are passing a Class, not an instance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wait ... I don't actually need configure() to configure it, since I already have the reference to the SDS.

TODO: ProtobufConfiguratorExts.kt is broken.
@zhangkun83
Copy link
Collaborator Author

@marcoferrer, I deleted ProtobufSourceDirectorySet in my latest commit, but that broke ProtobufConfiguratorExts.kt . Could you suggest how to fix it?

@marcoferrer
Copy link
Contributor

marcoferrer commented Jan 11, 2019

@zhangkun83, I think this should take care of it

fun SourceSet.proto(action: SourceDirectorySet.() -> Unit) {
    (this as? ExtensionAware)
        ?.extensions
        ?.getByName("proto")
        ?.let { it as? SourceDirectorySet } // <-- We need to cast the result of getByName
        ?.apply(action)
}

@zhangkun83
Copy link
Collaborator Author

Awesome. Thank you @marcoferrer!

@zhangkun83 zhangkun83 merged commit bcf3f1a into google:master Jan 11, 2019
@zhangkun83 zhangkun83 deleted the gradle6 branch January 14, 2019 22:09
"generate-proto-" + name, this.fileResolver, new DefaultDirectoryFileTreeFactory())
String srcSetName = "generate-proto-" + name
SourceDirectorySet srcSet
if (Utils.compareGradleVersion(project, "5.0") < 0) {
Copy link

Choose a reason for hiding this comment

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

Why did you use a 5.0?

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.

4 participants