Generating protobuf classes without cleaning the generated classes first interacts poorly with gradle caching when java_multiple_files is true or multiple proto files are used

[doctorpangloss]

Issue

If you do something that removes files from the generated Java output directory, like removing a message definition, but you don't clean first, gradle will cache the not-removed-but-intended-to-be-removed .java file along with your new definitions. From that point forward, cleaning will not remove the file, even if you regenerate the proto .java files, because the result is cached.

In other words, the way the plugin defines outputs interacts poorly with the cache.

Solutions

1. Don't use the directory as the outputs for a cacheable task, use the exact files that are expected to be generated. Otherwise you'll cache files that should be deleted, if the user did not clean before hand.
2. Correctly delete the contents of the target directory. Sometimes messages are removed from a protobuf file during development, for example, so it should be cleaned.
3. Error on option java_multiple_files = true;.

Reproduction

0. Add org.gradle.caching=true to your gradle.properties.
1. Create a .proto file that looks like this.

option java_multiple_files = true;
message ShouldExist {
  int32 field = 1;
}

message ShouldBeRemoved {
  int32 field = 1;
}

2. Run generateProto. Observe you get: ShouldExist.java and ShouldBeRemoved.java.
3. Remove ShouldBeRemoved from your .proto file.
4. Run generateProto. Observe you still have ShouldExist.java and ShouldBeRemoved.java. Observe this task now caches the whole generated files directory, including ShouldBeRemoved.java.
5. Run clean. Observe you have no generated files.
6. Run generateProto. Observe you still have ShouldExist.java and ShouldBeRemoved.java, even though ShouldBeRemoved.java should be removed. Why? It was cached!
7. Add a comment or any other cache-busting change to your .proto file.
8. Run clean
9. Run generateProto. Observe you only have ShouldExist.java.

Notes

This problem also exists if you use multiple proto files and delete one. It's not just limited to option java_multiple_files. No matter what, you have to correctly register outputs for inputs if you want gradle caching to work correctly.

[voidzcy]

This is a known issue (#331 (comment)). You observation is exactly what #331 was describing. It does surprise users when they delete some message definitions in a proto file and rebuild without clean.

We would go with solution 2, which is to wipe out the directory for generated code in the proto generation task (#332 was filed to deprecate generatedFilesBaseDir to stop users setting the output directory to some shared directory with their own code). But before that, you can just do a clean build if you are aware of this limitation.

[doctorpangloss]

No matter what, you have to correctly register outputs for inputs if you want gradle caching to work correctly.

[doctorpangloss]

You have to fix this bug! It's infuriating. It is the biggest pain point with this plugin. Do you have a timeline to correctly register outputs?

[jd3nn1s]

It's worth emphasizing how troublesome this behavior is with the gradle cache. If a clean is not done before a build that is configured to push to a shared gradle cache, then the cache is poisoned for all users. It is necessary to clear or disable the cache. Cleaning the build after the cache is poisoned is not enough.

[akuter-bc]

This still seems to be an issue???

[rougsig]

@akuter-bc yes in general. Outputs should be correctly registered now.

[doctorpangloss]

While I really, really appreciate this plugin, maybe there needs to be a test authored, that fails, that demonstrates the issue?

It's a pretty big issue. Like why use Gradle then if caching is broken? Why not use Make? It's as if you turned off 99% of Bazel.

I think the right approach at this stage is:

Error on option java_multiple_files = true;

[ejona86]

Closing since this is a dup of #331 and it is confusing to have both, even if this is more caching-directed.

We know how to reproduce this issue, and I have worked some on a fix. It isn't a hard fix, but it is non-trivial and requires attention to detail.

I had worked on fixing this but was delayed and didn't want to delay the 0.9.0 release, and now there's fall-out from the 0.9.0 release. Honestly, Bazel goes out of its way to prevent this sort of issue while Gradle doesn't have a restrictive runtime environment so it is easy to do things wrong and not notice. That's not surprising as the build cache was a later addition to Gradle. Part of the trouble, though, is also this issue is super-old and Gradle has advanced a lot since the plugin was initially written.

This issue is caused by generatedFilesBaseDir (and its predecessor generatedFileDir) which existed at least (46935b3) in 2013, before this project was adopted by Google in 2015. And that predates the Gradle build cache.