olamy commented on PR #389: URL: https://github.com/apache/maven-build-cache-extension/pull/389#issuecomment-6575256179
I think there's a potential problem with how tracked parameters are compared. When we save a build, we read the parameter values from the mojo after it has run. On the next build, we compare them with a freshly configured mojo that hasn't run yet. If a plugin changes its own fields while running, the two values never match and the cache never gets used. maven-compiler-plugin does exactly that: - testCompile adds `--patch-module` and `--add-reads` to compilerArgs for any project with a module-info.java - when a toolchain is in use, it sets `fork=true` and sets executable to the toolchain's javac path So with this PR, JPMS projects and projects using toolchains would rebuild every time, even with no cache config at all. The `--module-version` handling in normalizeMojoProperty is this same issue showing up for one argument. This could already happen to people who set up reconciliation themselves. But it's now on by default, so a lot more builds will hit it. A simpler fix might be to read the tracked values in beforeMojoExecution, when the mojo is configured but hasn't run yet. Then both sides of the comparison come from the same state, and we can drop the `--module-version` special case. (The ITs don't catch this because they only check for "Found cached build", which is logged before the parameter check.) -- 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]
