Enabling caching for all proto tasks

[dcabasson]

Currently none of the proto task support gradle cache.

Please enable cache for all tasks :
https://docs.gradle.org/current/userguide/build_cache.html#sec:task_output_caching_details

This will require to properly annotate inputs and outputs as per :
https://docs.gradle.org/current/userguide/more_about_tasks.html#sec:up_to_date_checks

[zpencer]

I took a stab at this: https://github.com/google/protobuf-gradle-plugin/compare/master...zpencer:make-tasks-cacheable?expand=1

It looks like Gradle thinks there are overlapping output directories, but I can not find anything in the code that suggests multiple tasks are sharing the same output dirs:

Caching disabled for task ':generateProto': Gradle does not know how file
'build/generated/source/proto/main' was created (output property '$1'). Task output caching requires
exclusive access to output paths to guarantee correctness.

Ideas are welcome

[dcabasson]

Hey @zpencer and thanks for picking that up. I am trying to reproduce your issue - I am assuming you are using your testProject protobuf tasks should be cacheable (java-only project) test to assess that.

First hurdle was that I had to run a gradlew createClasspathManifest to have the right file generated. The error message hinted at running testClasses but that didn't generate the file for me.

I am now running into Java 9 issues - some of your tests seems to be running older versions of gradle that are not compatible with Java 9. Which version of Java do you usually run your tests with?

[dcabasson]

Ok, I switched back to Java 8 and that seems to work as far as the version of Java is concerned. It looks like a lot of tests are broken because you changed enableCacheExperimental to enableCacheableTasksExperimental but that doesn't seem to have been updated correspondingly in build_base.gradle

[dcabasson]

Ok, the issue is that you are manually creating the folders while Gradle is calculting the cache key. By the time gradle makes it decision to rerun the task or not, the folders have been created and Gradle decides to just rerun everything because the folders magically appeared.

I have put together a PR for you : zpencer#1

It seems to work in Gradle 4.3 - not sure about 4.0 and why there is task that is UP-TO-DATE rather than FROM-CACHE.

[dcabasson]

You will also have to look into the relocation issue - most of your dir paths are absolute, but the cache should still be matched even if the folder moves around. We are using CI and the folder is a randomly generated one that will change with every build, but the cache should still be hit.

[zpencer]

Thank you so much for the helping debug the issue :)

Regarding relocatability, from reading this it looks like it is a concept that applies to task inputs. So for the purposes of this plugin, there are really only two types of tasks with file inputs. What I think we need are:

• ProtobufExtract copies protos from .proto files and archives. I think the inputs of this task may be cached too, so its inputs should be annotated @PathSensitive(PathSensitivity.RELATIVE)
• GenerateProtoTask can take inputs from ProtobufExtract tasks, so those inputs should be @PathSensitive(PathSensitivity.RELATIVE).

Would this test case be sufficient to test relocatability?

1. Create two copies of the same project proj1 and proj2 that share the same cache directory.
2. Build proj1
3. Build proj2 and verify that ProtobufExtract and GenerateProto tasks are cache hits.

[dcabasson]

Yes, relocalibity is about task inputs. And yes, your test is valid and will ensure that things work correctly.
I am hoping it's as simple as you describe. but you are feeding all your commands as an input and I suspect those commands have full paths - I am not sure if you can apply a pathSensitivity to an @input annotated method. It's usually for @InputFile/@InputDir. But it's worth a shot and let me know if it's not working as intended and I can have a look :)

[zpencer]

You're right, I was hoping to avoid having to audit and annotate all the input objects, some of which require use of @Nested. But to work properly with relocation there's probably no way around it.

[zpencer]

I updated the branch to annotate the existing input objects rather than using the final commands. One question about the PathSensitivity.RELATIVE, where is it relative to? Is the root dir project.buildDir?

[dcabasson]

I think it is relative to the root dir of the project. Which means that if you relocate the project as a whole, the cache key will stay the same. Which is definitely a must in a CI context.

[vkryachko]

Hi, any update on this? thanks

[connorcartwright]

Bumping this @zpencer - is there any update or suggested work-around? 😃

[zpencer]

Hi all, apologies I have not had any time to continue work on this. PRs are welcome if anyone has some free cycles.

[wwadge]

Hi @zpencer was this task ever completed? Can you give me some pointers on the current state?

[zhangkun83]

Hi @wwadge, @zpencer is no longer working on this project. #293 is related to build cache. I don't think there have been other work related to this.

[bengaudiosi-toast]

This appears to still be an issue. I see the extractIncludeProto task running despite making changes to any protobuf files in a project I have.

[jameslamine]

I can confirm that extractIncludeProto is never cached in my builds. Is this intentional?

[runningcode]

Yes, it is intentional that the extractIncludeProto task is not cacheable. It is a copy task and does not benefit from being cacheable. You will lose time in caching it.

[jprinet]

Just to confirm what @runningcode mentioned, cache entries are zipped which introduces an overhead over simply moving files from one place to another which negates the benefit of caching.

It would be good to document that clearly in ProtobufExtract
Since Gradle 7.0, @DisableCachingByDefault can be added with a because value.

I couldn't create a PR as the Gradle version in use is still 6.9 and upgrading implies other modifications which are not worth it in that case.

[jameslamine]

If anyone has this same problem, here's how to manually enable caching.

We have a large amount of dependencies, so it's faster to read extractProto and extractIncludeProto from cache than it is to search through all dependencies for proto files to copy. I enabled caching with the following:

import com.google.protobuf.gradle.ProtobufExtract

tasks.withType(ProtobufExtract).all {
    it.outputs.cacheIf { true }
}

[ejona86]

Looks like the cache compatibility overall was fixed in #413.