michael-o edited a comment on pull request #26: URL: https://github.com/apache/maven-shade-plugin/pull/26#issuecomment-864394719
So I reviewed this PR, the JIRA issue as well as MSHADE-206 and MNG-5899 and I see a few issues here: * The description on the JIRA issue is wrong * From my PoV this PR only fixes the symptom, not the cause * Looking at this plugin's source code this change (regardless of cause or symptom) can only be applied if a DPR is created, i.e., this has to happen unconditionally in `rewriteDependencyReducedPomIfWeHaveReduction()` attaching a new POM with modified deps, but not modifying the dep list is inconsistent. * So why does this only solve the symptom? If you look at the discussion in MSHADE-206 and MNG-5899 it refers to an old commit in Maven which introduced a hack with a simple cache which does not re-read POMs even if they have changed in-flight. There is no cache validation/eviction. Without haven't do any live testing and knowing how reasonable the cache performance gains are, one could consider dropping the cache. I always prefer consistent behavior over performance. I would like to ask the following: Please branch off Maven master, drop the hacky cache and retry w/o this PR and let me know whether this fixes the issue already for you. If so, we need to reconsider the cache. Maybe this new parameter can be introduced, but only as `@Deprecated` because it fixes the symptom only. @rfscholte Since you left a new comments on MNG-5899, do you think we could safely drop the cache and see whether we will see complains about performance drop? -- 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
