Summary: | DeltaProcessor misses state changes in archive files | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Terry Parker <tparker> | ||||||||||||
Component: | Core | Assignee: | Jay Arthanareeswaran <jarthana> | ||||||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||||||
Severity: | major | ||||||||||||||
Priority: | P3 | CC: | acsipak, melickm, Olivier_Thomann, srikanth_sankaran, stephan.herrmann | ||||||||||||
Version: | 3.8 | Flags: | jarthana:
review+
|
||||||||||||
Target Milestone: | 3.8 M3 | ||||||||||||||
Hardware: | Other | ||||||||||||||
OS: | Linux | ||||||||||||||
Whiteboard: | |||||||||||||||
Attachments: |
|
Description
Terry Parker
2011-09-12 18:03:32 EDT
Created attachment 203194 [details]
Patch to DeltaProcessor.java
Always reset the resolved classpath on any change to an archive jar, not just when it contains a chained jar.
Created attachment 203195 [details]
Add a ResetInvalidArchivesCache() function
Add a ResetInvalidArchivesCache() function and invoke it from ClasspathEntry.validateClasspathEntry()
Jay, please follow up. (In reply to comment #0) > Also, I'd be happy to add a regression test for this issue if you point me at > the appropriate place to add it. > The tests usually go to JavaElementDeltaTests or ClasspathTests, which you can find in the project org.eclipse.jdt.core.tests.model. Olivier, this is indeed a regression from the bug 354332. But I think we could still keep the fixes we made already. Perhaps, should we reopen the bug 354332 and make it depend on this one? (In reply to comment #4) > Olivier, this is indeed a regression from the bug 354332. But I think we could > still keep the fixes we made already. Perhaps, should we reopen the bug 354332 > and make it depend on this one? Yes, bug 354332 should be reopened to add the dependency on this one. If you think we should keep the fix in, then close it after this is done and work on a fix for this one. Now do you think we should request a rebuild for M2 or we can have M2 with this issue in ? (In reply to comment #5) > Now do you think we should request a rebuild for M2 or we can have M2 with this > issue in ? Considering that what is failing is not a common use case and that we have workaround, I don't think we need an M2 rebuild. This should be fixed before M3. (In reply to comment #0) > Also, I'd be happy to add a regression test for this issue if you point me at > the appropriate place to add it. Any luck with the test? The patch looks good and it would be nice to have a regression test. I have just dug my way out from under my end-of-quarter tasks. I'll take a look at adding tests tomorrow. Created attachment 204894 [details]
Patch to fix the DeltaProcessor regeression and fix validateClasspathEntry(), with tests
(In reply to comment #10) > Created attachment 204894 [details] > Patch to fix the DeltaProcessor regeression and fix validateClasspathEntry(), > with tests Patch looks good and thanks. But one of the new tests is failing when the entire ClasspathTests is run. On debug I found that the delta processor sees the lib2.jar as already existing. I would suggest we use bug specific names in the test, such as Bug357425, lib357425_a.jar, lib357425_b.jar etc. to make the tests pass. I could do this, but I can't alter the patch. Created attachment 205212 [details]
Update to name the test jar files uniquely
I renamed that test as well to testBug357425() and moved it to the bottom of the file.
(In reply to comment #12) > Created attachment 205212 [details] > Update to name the test jar files uniquely > > I renamed that test as well to testBug357425() and moved it to the bottom of > the file. This patch passes all the tests. Jay, is anything else pending for this bug ? If not can you close it ? Please remember to set the iplog and review flags appropriately and make sure the copy right mentions the contribution. (In reply to comment #13) > This patch passes all the tests. Jay, is anything else pending for this > bug ? If not can you close it ? Please remember to set the iplog and review > flags appropriately and make sure the copy right mentions the contribution. Thanks, Srikanth. I will release the fix soon. Created attachment 205507 [details]
Patch with copyrights
Git patch, same as last one, but with updated copyright information.
Released in HEAD for 3.8 M3 with commit 30fef0e89bd163d63b2dda342f5f4be753476e00 Terry, once again, thanks for the contribution. I filed bug 361922 for possible improvements on the test coverage side. Verified for 3.8 M3 using build N20111022-2000 *** Bug 375249 has been marked as a duplicate of this bug. *** (In reply to comment #6) > (In reply to comment #5) > > Now do you think we should request a rebuild for M2 or we can have M2 with this > > issue in ? > > Considering that what is failing is not a common use case and that we have > workaround, I don't think we need an M2 rebuild. Jay, 1. Any chance to backport the fix to version 3.7? (This bug is related to 3.8.2, but the bug still appears in 3.7.2, see Bug 375249, which you marked as duplicate.) 2. You mentioned a workaround, but I couldn't quite make out what you meant. Could you please elaborate on the topic? (The workarounds I desrcribed in Bug 375249 are nor stable nor close to comfortable.) Thanks for the reply! (In reply to comment #20) > 1. Any chance to backport the fix to version 3.7? (This bug is related to > 3.8.2, but the bug still appears in 3.7.2, see Bug 375249, which you marked as > duplicate.) For reasons Satyam mentioned in bug 375249, comment #12, bug 375249 is probably not a duplicate of this one. Could you confirm if any of the 3.8 versions addresses your concern? > 2. You mentioned a workaround, but I couldn't quite make out what you meant. > Could you please elaborate on the topic? (The workarounds I desrcribed in Bug > 375249 are nor stable nor close to comfortable.) The workaround is same as the one you mentioned in bug 375249, comment #3. Sorry I didn't realize it wasn't clearly mentioned in this bug. (In reply to comment #21) > The workaround is same as the one you mentioned in bug 375249, comment #3. > Sorry I didn't realize it wasn't clearly mentioned in this bug. To elaborate, try these steps: 1. Close the project in question. 2. Delete the invalidArchivesCache 3. Restart the workspace and reopen the project It's possible that the restarting the workspace could be avoided, but we would want to rule out other causes. Before reopening the workspace, ensure that the JAR in question is a valid archive. |