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

GenerateProto depends on compile tasks #578

Open
rougsig opened this issue Jul 10, 2022 · 4 comments
Open

GenerateProto depends on compile tasks #578

rougsig opened this issue Jul 10, 2022 · 4 comments

Comments

@rougsig
Copy link
Collaborator

rougsig commented Jul 10, 2022

Background
Test project structure (link):

├── instances
│   ├── app
├── work-with-proto
│   ├── implementation pure-kotlin
│   │   ├── api lib-core
│   ├── implementation pure-java
│   ├── implementation proto-kotlin
│   │   ├── api lib-core
│   ├── implementation proto-java
│   ├── implementation pure-android
│   ├── implementation proto-android

:generateProto executed a bunch of unnecessary tasks:
:lib-core:compileJava, :proto-android:compileDebugJavaWithJavac, :proto-java:compileJava,
:proto-kotlin:compileKotlin, :proto-kotlin:compileJava, :pure-android:compileDebugJavaWithJavac,
:pure-java:compileJava, :pure-kotlin:compileKotlin, :pure-kotlin:compileJava

It just compile all dependencies, without any reason.

How it works now
Protobuf gradle plugin create compileProtoPath configuration that extends from implementation and compileOnly.
It's quite user-friendly, because all you project dependencies provides proto files by default. No need to duplication dependency for compileProtoPath configuration.
i.e.

dependencies {
  implementation project(":with-proto")
  
  // no need write string below.
  // compileProtoPath configuration will take all implementation dependencies automatically.
  // compileProtoPath project(":with-proto")
}

What wrong
Gradle scan from test project for generate proto task.
Generate proto task resolves compileProtoPath configuration. compileProtoPath configuration resolves implementation and compileOnly configuration. As result we have compiled or downloaded all project's dependencies.

This is not good. It costs build time a lot. (that looks real, if project have a lot of dependencies #551).

Expected result
Generate proto task does not compile and download all of the project's dependencies. It should only work with dependencies marked as path dependencies.

Changes
The compileProtoPath configuration does not extends from implementation and compileOnly configurations.

Breaking changes
If compileProtoPath configuration does not extends from implementation and compileOnly configurations, user should declarate all dependencies for generate proto task with compileProtoPath configuration. i.e.

dependencies {
  implementation project(":with-proto")
  
  // User should add all path dependencies with compileProtoPath configuration.
  compileProtoPath project(":with-proto")
}
@ejona86
Copy link
Collaborator

ejona86 commented Jul 11, 2022

Note that protobuf configuration is a completely different beast. protobuf configuration is for dependencies for which you want to generate code (i.e., it contains a proto and nobody else is generating code for this proto). implementation is how you'd add a dependency for your .proto files. In a Java-centric environment, you'd never use protobuf.

Note that even if we avoid the unnecessarily dependencies like :lib-core:compileJava, those tasks are still a dependency for compileJava and the generated code. Consider for example if :lib-core has .proto files. Then we need those proto files for GenerateProto and we need the previously-generated code for those proto files for JavaCompile.

@rougsig
Copy link
Collaborator Author

rougsig commented Jul 11, 2022

Yes, it should work as you describe. The protobuf configuration only contains *.proto files, but now protobuf contains *.class files and other stuff.

@ejona86
Copy link
Collaborator

ejona86 commented Jul 11, 2022

Yes, it should work as you describe. The protobuf configuration only contains *.proto files, but now protobuf contains *.class files and other stuff.

No, that's not what I'm saying. I'm saying that protobuf configuration is like adding to sourceSet.srcDir. It is completely unrelated to dependencies in the normal sense.

now protobuf contains *.class files and other stuff.

No matter the approach, .proto dependencies often contain class files, because in the normal case the same .jar files contain the .proto files and .class files.

@ejona86
Copy link
Collaborator

ejona86 commented Jul 11, 2022

rougsig and I discussed separately and protobuf was intended to be compileProtoPath. We are in agreement that would function.

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

No branches or pull requests

2 participants