AlexanderAshitkin commented on code in PR #181:
URL:
https://github.com/apache/maven-build-cache-extension/pull/181#discussion_r1734736488
##########
src/main/java/org/apache/maven/buildcache/DefaultNormalizedModelProvider.java:
##########
@@ -81,7 +82,7 @@ private Model normalizedModelInner(MavenProject project) {
// does not make sense to add project version to calculate hash
resultModel.setVersion(NORMALIZED_VERSION);
resultModel.setModules(prototype.getModules());
-
+
resultModel.setProperties(normalizeProperties(prototype.getProperties()));
Review Comment:
Tracking the cache properties by itself doesn't seem helpful because the
checksum produced using these hints is what matters.
Consider 2 cases:
1) `maven.build.cache.exclude.glob.1` doesn't exclude anything. Removing
this property doesn't affect the cache checksum and artifacts - there is no
point in having different cache records for the change. In this case
effectively the same build will be stored under two different checksums.
2) `maven.build.cache.exclude.glob.1` excludes something. In that case, the
excluded elements will contribute to the checksum, and there will be a new
cache record. In this case adding the property to the checksum is redundant.
Currently, I can only see a potential regression in the form of a reduced
cache hit rate in this change.
--
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
