AlexanderAshitkin commented on code in PR #176:
URL: 
https://github.com/apache/maven-build-cache-extension/pull/176#discussion_r1732039886


##########
src/main/java/org/apache/maven/buildcache/CacheControllerImpl.java:
##########
@@ -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");

Review Comment:
   In the current implementation, the 'analyzeResult' returns partial success 
in lines 277-278 as expected.
   
   A more accurate implementation would be to always attach the generated 
sources in `save` and process partial success more accurately. A build without 
a package phase should restore generated sources only and rerun all the missing 
lifecycle phases starting with compile to build missing compile sources.
   
   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 
the cached builds are not usable should also be considered for improvement.
   
   Looking at this fix, it seems that saving pre-packaged builds has no value 
unless cache properly supports compile phase caching. As a quick fix, this 
change seems legitimate.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to