AlexanderAshitkin commented on code in PR #146:
URL:
https://github.com/apache/maven-build-cache-extension/pull/146#discussion_r1579540296
##########
src/main/java/org/apache/maven/buildcache/checksum/MavenProjectInput.java:
##########
@@ -617,7 +621,13 @@ private static boolean isReadable(Path entry) throws
IOException {
private SortedMap<String, String> getMutableDependencies() throws
IOException {
SortedMap<String, String> result = new TreeMap<>();
- for (Dependency dependency : project.getDependencies()) {
+ Set<Dependency> dependencies = Stream.concat(
Review Comment:
In general, detecting mutable dependencies in the build plugins looks
correct.
Implementation-wise:
1. The current implementation has a minor flaw because the dependencies are
merged into a single map. If the same dependency is present in the project and
the plugin and accounted for just once, it potentially creates a collision when
either of them is removed (removing one dependency will not change the checksum)
2. For traceability, plugin dependencies should be distinguishable from the
project dependencies in the `buildInfo.xml`. Creating a separate,
self-explaining `DigestItem` for them is better.
The logic of inspecting plugins should be moved one level up to
`calculateChecksum` and better be like this:
```txt
For every plugin:
- find mutable plugin dependencies (slightly refactor
`getMutableDependencies` to accept dependencies list)
- create 'DigetsItem's with plugin info (for traceability - to understand
the source of the particular component of the checksum)
- add the items to the overall checksum
- for all the above - add debug logs for troubleshooting
```
The benefit of this approach is the better traceability of the final
checksum.
--
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
