AlexanderAshitkin commented on code in PR #177:
URL:
https://github.com/apache/maven-build-cache-extension/pull/177#discussion_r1732057978
##########
src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java:
##########
@@ -267,6 +286,12 @@ private CacheRestorationStatus restoreProject(
// mojoExecutionScope.seed(
// org.apache.maven.api.MojoExecution.class, new
DefaultMojoExecution(cacheCandidate));
mojoExecutionRunner.run(cacheCandidate);
+ } else if
(reconciliationExecutionMojos.contains(cacheCandidate)) {
Review Comment:
I'm not convinced that this logic is valid. If you have build 1 with the
plugins [(X, parameters PX1), (Y, parameters PX1)], with the flag on the new
cached build might be a result of [(X, parameters PX2), (Y, parameters PY1)].
The issue here is that without tracking inputs/outputs, we can't guarantee that
reusing PY1 is valid if the first plugin changed parameters. It's possible that
it relies on outputs from the first plugin and could yield a different result
without the cache. This logic seems flawed to me at the moment. It might lead
to all sorts of corruptions inadvertently. it probably could run for certain
plugins in presence of additional metadata, but not in a general case.
This optimization could work for a leaf plugins (plugins which do not
contribute and do not impact subsequent plugins) but that will require an
additional plugin metadata
--
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
