Skip to content

Conversation

seregamorph
Copy link

@seregamorph seregamorph commented Apr 29, 2024

Context

This PR is a proof of concept, just for possible discussion, curious about opinions of Gradle maintainers as well. In the prototype the BuildCacheKey is exposed in the org.gradle.api.internal.cache package assuming there is no guarantee of forward compatibility unless it's clarified.

Some tasks may need Gradle calculated cache key to align standard cache semantics, not to duplicate artifact hash itself.
E.g. there are web artifact compiling tasks, that produce output with URL query parameters like ...?v={version} where version can be random UUID or git commit or timestamp or whatever. The similar approach can be helpful if the output creates some manifest with unique build id.

The idea of this change is to reuse the Gradle build cache in the task.

Advantages:

  • reproducible between sequential builds between local and CI/CD environment (if there is a remote cache)
  • different if there are different task Inputs
  • aligned with Gradle cache mechanism

But it has a major drawback: if the build is executed without build cache, Gradle skips cache calculation. So the task output will not be reproducible between --build-cache and --no-build-cache

Alternative solutions

Manually calculate build cache key in the task implementation - taking into calculation at least:

  • all inputs respecting absolute/relative file name sensitivity
  • buildSrc/task implementation hash
  • any implicit parameters like build properties

Contributor Checklist

  • Review Contribution Guidelines.
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
  • Provide unit tests (under <subproject>/src/test) to verify logic.
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes.
  • Ensure that tests pass sanity check: ./gradlew sanityCheck.
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@bot-gradle bot-gradle added from:contributor PR by an external contributor to-triage labels Apr 29, 2024
@seregamorph seregamorph changed the title Expose BuildCacheKey Expose BuildCacheKey to task execution Apr 29, 2024
@seregamorph seregamorph changed the title Expose BuildCacheKey to task execution Expose BuildCacheKey to task execution [PoC] Apr 29, 2024
@gitstream-cm gitstream-cm bot requested a review from lptr April 29, 2024 09:33
Copy link

gitstream-cm bot commented Apr 29, 2024

⚠️ This PR appears to be lacking tests. Consider adding tests to cover the new functionality.

@seregamorph seregamorph force-pushed the expose-cache-key branch 2 times, most recently from 501ef19 to d05ea64 Compare April 29, 2024 09:53
Copy link

gitstream-cm bot commented Apr 29, 2024

📊 Changes by Platform: this PR is 91% new code

2 platforms were affected (:warning: if possible, only one platform should have significant changes in a PR)

See details
Platform Added Lines % of Total Line Changes Deleted Lines % of Total Line Changes Files Changed % of Total Files Changed
null 58 73% 7 9% 2 50%
core-execution 15 19% 0 0% 2 50%

@ov7a ov7a added in:build-cache 👋 team-triage Issues that need to be triaged by a specific team and removed to-triage labels May 7, 2024
@ov7a
Copy link
Member

ov7a commented May 7, 2024

This issue needs a decision from the team responsible for that area. They have been informed. Response time may vary.

@seregamorph
Copy link
Author

Thanks, @ov7a

@lptr
Copy link
Member

lptr commented May 23, 2024

We now always calculate the cache key for tasks where we capture inputs, so in theory we could expose it unconditionally. That said, the cache key is an internal implementation detail of Gradle. While it sounds like it could be useful to expose, it has different requirements than the various features folks want to use it for. We also want to retain the ability to change how the cache key is calculated without the potential of breaking user code.

What we could do instead is to expose a way to calculate a secure hash from various inputs. That could be a feature that Gradle provides, with a contract that we could carry along in a backwards compatible way. If that's something you'd want to have, please create a feature request. Though current bandwidth limitations make it unlikely that we'll be able to ship it anytime soon.

@lptr lptr closed this May 23, 2024
@ov7a ov7a removed the 👋 team-triage Issues that need to be triaged by a specific team label May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from:contributor PR by an external contributor in:build-cache
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants