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 small 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. Plugin dependencies should be distinguishable from the project
dependencies in the `buildInfo.xml` for traceability. 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
