Make shadowJar task cacheable #524
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
johnrengelman
approved these changes
Nov 10, 2019
cbeams
added a commit
to cbeams/bisq
that referenced
this pull request
Nov 19, 2019
After the upgrade from Gradle 4.10.2 to 5.6.4 in commit 5fe71fa, all of Bisq's shadowJar tasks started failing Gradle's incremental build checks. This meant that repeated invocations of `gradle build` went from a handful of seconds to more than a minute, because shadowJar tasks had to be re-executed on every build. For example, with --info enabled, one would see entries like this in the build output: > Task :seednode:shadowJar Custom actions are attached to task ':seednode:shadowJar'. Caching disabled for task ':seednode:shadowJar' because: Caching has not been enabled for the task Task ':seednode:shadowJar' is not up-to-date because: Output property 'archiveFile' file [...]libs/seednode.jar has changed. This problem was solved by in GradleUp/shadow#524 and made available in the project's recent 5.0.2 release. This patch simply bumps the shadow plugin version to that value, and gets us back to snappy incremental builds, e.g.: $ gradle build BUILD SUCCESSFUL in 7s
cbeams
added a commit
to cbeams/bisq
that referenced
this pull request
Nov 19, 2019
After the upgrade from Gradle 4.10.2 to 5.6.4 in commit 5fe71fa, all of Bisq's shadowJar tasks started failing Gradle's incremental build checks. This meant that repeated invocations of `gradle build` went from a handful of seconds to more than a minute, because shadowJar tasks had to be re-executed on every build. For example, with --info enabled, one would see entries like this in the build output: > Task :seednode:shadowJar Custom actions are attached to task ':seednode:shadowJar'. Caching disabled for task ':seednode:shadowJar' because: Caching has not been enabled for the task Task ':seednode:shadowJar' is not up-to-date because: Output property 'archiveFile' file [...]libs/seednode.jar has changed. This problem was solved by in GradleUp/shadow#524 and made available in the project's recent 5.0.2 release. This patch simply bumps the shadow plugin version to that value, and gets us back to snappy incremental builds, e.g.: $ gradle build BUILD SUCCESSFUL in 7s
cbeams
added a commit
to cbeams/bisq
that referenced
this pull request
Nov 19, 2019
After the upgrade from Gradle 4.10.2 to 5.6.4 in commit 5fe71fa, all of Bisq's shadowJar tasks started failing Gradle's incremental build checks. This meant that repeated invocations of `gradle build` went from a handful of seconds to more than a minute, because shadowJar tasks had to be re-executed on every build. For example, with --info enabled, one would see entries like this in the build output: > Task :seednode:shadowJar Custom actions are attached to task ':seednode:shadowJar'. Caching disabled for task ':seednode:shadowJar' because: Caching has not been enabled for the task Task ':seednode:shadowJar' is not up-to-date because: Output property 'archiveFile' file [...]libs/seednode.jar has changed. This problem was solved by in GradleUp/shadow#524 and made available in the project's recent 5.0.2 release. This patch simply bumps the shadow plugin version to that value, and gets us back to snappy incremental builds, e.g.: $ gradle build BUILD SUCCESSFUL in 7s
Goooler
reviewed
Dec 2, 2024
Comment on lines
+194
to
+212
| @Input | ||
| String getPattern() { | ||
| return pattern | ||
| } | ||
|
|
||
| @Input | ||
| String getPathPattern() { | ||
| return pathPattern | ||
| } | ||
|
|
||
| @Input | ||
| String getShadedPattern() { | ||
| return shadedPattern | ||
| } | ||
|
|
||
| @Input | ||
| String getShadedPathPattern() { | ||
| return shadedPathPattern | ||
| } |
There was a problem hiding this comment.
These getters are immutable and unused, can we remove them? I'm planning to remove them and migrate includes and excludes to org.gradle.api.provider.SetProperty.
There was a problem hiding this comment.
Yeah, if these are being removed in favor of property objects, then yes, these can be removed. They were only added in order to expose the inputs from the relocator.
There was a problem hiding this comment.
To be clear, the new property objects should also be exposed as inputs (i.e. a getter with an @Input annotation) in order to maintain proper cacheability after these are removed.
There was a problem hiding this comment.
Can you take a review for #1079?
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes make the
shadowJartask compatible with the build cache.The task will still be cacheable if any of the following transformers are used:
But the other out-of-the-box transformers are not cacheable yet (if they are added, the task will be marked as un-cacheable) but I can add caching for those with a future PR. User-defined transformers will also make the task un-cacheable unless they declare the inputs of the transformer and annotate it with
@CacheableTransformer.Similarly,
SimpleRelocatorhas been marked as cacheable, but any custom relocators will disable caching unless the class is annotated with@CacheableRelocator.We've seen fairly significant task execution times for projects with large dependency graphs, so making this task usable with the build cache could provide a decent performance improvement.
Please pay special attention to the test coverage and let me know if there are any potential edge cases not covered. I'm happy to discuss anything if it doesn't make sense or seems wrong.