-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix "m2e plugin sometimes 'loses' resources" (#1511) #1629
Conversation
The Java Builder may delete files from the project output directory that need to be re-created by the m2e Maven Builder. With commit 8e5cd49 (a fix for eclipse-m2e#1275), any changes to the project output directory were ignored, leading to unexpected errors when running an application after modifying the project POM: resources were missing from the target classpath, leading all sorts of unexpected program behavior. This change allows marking these changes as relevant, as long as the following conditions are met: - The expected resource no longer exists (i.e., it was deleted by another builder, plugin or outside process) - The modification applies to a resource in the classes or test-classes folder (i.e., outputLocation/testOutputLocation) eclipse-m2e#1511 eclipse-m2e#1275
CI failure: "Baseline problems found! Project version ..." can be fixed with #1630 |
org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/builder/MavenBuilderImpl.java
Outdated
Show resolved
Hide resolved
... addressing reviewer concerns
Please include any version changes in your PR here. |
@@ -219,7 +225,11 @@ private boolean hasRelevantDelta(IMavenProjectFacade projectFacade, IResourceDel | |||
IPath fullPath = delta.getFullPath(); | |||
if(buildOutputLocation.isPrefixOf(fullPath)) { | |||
//anything in the build output is not interesting for a change as it is produced by the build | |||
//lets see if there are more interesting parts... | |||
// ... unless a classpath resource that existed before has been deleted, possibly by another builder | |||
if(!resource.exists() && isOutputOrTestOutput.test(fullPath)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if(!resource.exists() && isOutputOrTestOutput.test(fullPath)) { | |
if(isOutputOrTestOutput.test(fullPath) && !resource.exists()) { |
checking the prefix is probably a bit cheaper than checking all resources for existence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks good in general now and it seems a valid assumption that if the output folder is gone something must be happen.
But thinking more about it I think this is more fundamentally problem even though it is fixing things for resource plugin others might behave odd, one example is the maven-bundle-plugin or bnd-maven-plugins that might also place resources there.
Because of this, please apply the following changes:
hasRelevantDelta
should return an enum that has the values:DELTA
(the current 'true' return value),FULL
(when detecting the new output folder is gone),IRRELEVANT
(the current 'false' return value.- In
setupProjectBuildContext
a value ofFULL
should result in anull
build delta passed so we make sure a full build is executed and not an incremental one.
@laeubi I think your recently requested changes are beyond the scope of the bug, and therefore better suited for a separate refactoring. I also feel that it's more reasonable if you implement these changes the way you envision them, it will save us both unnecessary back and forth. |
The Java Builder may delete files from the project output directory that need to be re-created by the m2e Maven Builder.
With commit 8e5cd49 (a fix for #1275), any changes to the project output directory were ignored, leading to unexpected errors when running an application after modifying the project POM: resources were missing from the target classpath, leading all sorts of unexpected program behavior.
This change allows marking these changes as relevant, as long as the following conditions are met:
#1511 #1275