Community
Participate
Working Groups
Created attachment 101204 [details] error log BuildId: I20080516-1333 Steps to reproduce: 1. Install below RC1 projects: dtp-1.6RC1-200805200500.zip eclipse-SDK-3.4RC1-win32.zip emf-runtime-2.4.0RC1.zip GEF-SDK-3.4.0RC1.zip xsd-runtime-2.4.0RC1.zip wtp-S-3.0RC1-20080519195353.zip 2. New->Project->Web->Dynamic Web Project 3. Enter project name and leave other settings as default value. Click "finish" Expect result: Dynamic Web project can be created successfully Actual result: NPE thrown out. Error Log: Please see the attachment
Created attachment 101205 [details] screenshot
I can reproduce this when using emf-runtime-2.4.0RC1.zip, but not when using emf-runtime-I200805121800.zip We'll investigate and narrow down where the problem is. We might just have to re-compile with this final version of EMF ... or, maybe there's some other bug? Thanks for reporting so quickly. (These compressed schedules are hard! :)
This bug is valid for all project creation wizards, even for the Static Web project. I believe the problem is introduced by the Common project, so I move it to this component. I haven't observed this problem during the RC1 smoke testing, so it should be introduced with a later change.
Please note that the missing metadata file is due to an EMF change: https://bugs.eclipse.org/bugs/show_bug.cgi?id=231430 We are working with the EMF team to find a proper solution.
I've opened bug 233322 to request EMF to revert their fix for RC1 and re-introduce it in early RC2 driver, to give us time to adjust our code and test.
Created attachment 101351 [details] Add checks to see if the component file has content In discussion with Ed Merks, their bug fix makes resource.isLoaded() return true even if the file doesn't exist. That makes our code need to have an additional check - resource.getContents().isEmpty() - to see if there really was something there.
I'm ok with this for now... but we need to keep investigating the impact of returning a loaded EMF Resource even if the file doesn't exist.
Before marking approved, I think we need to investigate a bit more, and have a bit more discussion with EMF team to see if they have the right solution for original problem.
I'm okay with deferring the change to EMF 2.5, but I think this change has revealed bad assumptions in the downstream code. I.e., isLoaded should never be false after calling load and even in this case where it was, isLoaded() == false does not mean "resource doesn't exist". So if some logic is relying on the detecting when a resource doesn't exist, it should check that accurately with the new APIs provided in the URI converter in EMF 2.4 for exactly that purpose.
I agree with your comments regarding a better way to check existence of the file before we attempt to load - but what about any other load failure? will it be the same? - An exception occured loading - we should react, but will "isLoaded()" still return true? Is this change only different in the "file doesn't exists" case?
The goal of the change in EMF was to ensure that after load() (either version) is called isLoaded will always be true. Currently, the only case for which this wasn't true was the case that the one argument version wasn't able to create an input stream and now that hole is patched. Ideally WTP would be able to accommodate this change, but if not for this release, we can defer this to 2.5. Note that on linux, you could fail to create an input stream because you don't have group read permission. I believe the WTP logic would attempt overwrite the existing file in this case because it's assuming this state implies the file doesn't exits; this might even succeed if the directory has write access, though I'm not sure about that. So there seems to be a lurking bug here.
Thanks Ed for agreeing to wait on pushing your change into this release. That's the safest thing to do, even if we do need to improve our logic (it's not caused any problems "in the field" so far, so hate to introduce change at this point, without longer discussions and investigations). I'm "resetting" this bug, and changing title since it's no longer a "blocking" problem but an issue to address in maintenance or future release. Chuck and Carl, feel free to correct me if you have another opinon and want to introduce some change now.
I put in an initial patch, but we need to review *ALL* of our uses of this.
Since main part of this 'critical' bug has been fixed, I'm marking "fixed" but opened a cloned bug 247162 to track the "checking of all cases" that Carl mentioned in previous comment.
