[MBUILDCACHE-105] Executing 'clean process-test-classes' then 'clean verify' leads to empty jars #176
Conversation
reda-alaoui
changed the title
| @@ -255,6 +255,11 @@ private CacheResult analyzeResult(CacheContext context, List<MojoExecution> mojo | |||
| build.getCacheImplementationVersion()); | |||
| } | |||
|
|
|||
| if (lifecyclePhasesHelper.isLaterPhaseThanBuild("package", build)) { | |||
| LOGGER.warn("Cached build doesn't include phase 'package', cannot restore"); | |||
There was a problem hiding this comment.
In the current implementation, the 'analyzeResult' returns partial success in lines 277-278 as expected.
More accurate implementation could support pre-compile phases - 'save' and restore generated sources and continue with compile. This will allow restoring missing sources from the cache and potentially saving time on the code generation. Could be handy in IDEs which could faster compile the project after a branch switch (if generated sources are restored).
The second concern is that we still process builds that will never be used in the 'save'. Save and restore should be consistent, and the situation when cached builds are not usable should also be considered for improvement.
Though overall, as a quick fix, this change seems legitimate, it feels like restoring generated sources and commencing with compilation would be more optimal.
There was a problem hiding this comment.
@AlexanderAshitkin I agree with you.
At first I wanted to fix the extension to make it able to save cache state on phase < package. But there is this sentence in the documentation:
Once the extension is activated, the cache automatically kicks in on every package or higher phase.
I could not find the reason of this constraint, so I made sure this fix respects the latter. Also the fix should be quick to merge to prevent people from getting into trouble.
I also think this extension should not limit itself to phases >= package. We can still come back to that later.
src/test/java/org/apache/maven/buildcache/its/Issue105Test.java
Outdated
Show resolved
Hide resolved
|
Can this be merged? |
This PR makes sure any cached build that doesn't involve phase
packageis considered as invalid.Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[MBUILDCACHE-XXX] - Fixes bug in ApproximateQuantiles,where you replace
MBUILDCACHE-XXXwith the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean verifyto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.