Community
Participate
Working Groups
Build Identifier: M20110218-0911 A user library produced on a Windows machine, using external Jar file paths with C:\directory\file.jar was imported without first being fixed into an Eclipse on a MAC. This causes the contructor for UserLibraryManager to fail in init when the UserLibrary attempts to createFromString (line 188) because the JavaCore.newLibraryEntry method on line 4095 throws an AssertionFaildException Path for IClasspathEntry must be absolute: C:/directory/file.jar. I tried creating a plugin to allow me to call removeUserLibrary(strULName) but because I can't get the UserLibraryManager to construct the object I can't remove it from the list. The UserLibrary dialog is hosed due to this error (e.g., you can't see even the working entries, just the error information). I would recommend a few possible fixes. a) don't import entries that cause the exception attempting to reference the UserLibrary (perhaps show a dialog for the entries not being imported so the user knows a problem occurred) b) catch the exception thrown when initializing the User Library dialog and either show the errant entries in the UserLibraries dialog the way they are when the reference a valid path to a non-existant file so at least they could be edited or deleted, or show the error with a Remove invalid entry? OK/Cancel so the dialog can display the valid entries and the invalid ones are gone to be added manually with correct content by the user. It appears there is an encoding in the user library file (e.g., "WINDOWS-1252") -- if going to Linux or MAC OS, perhaps at least any leading X: could be removed during import so the exception isn't thrown and just a bad entry is displayed and able to be edited? Reproducible: Always Steps to Reproduce: 1.Create a userlibrary export file referencing a Windows OS style path: <?xml version="1.0" encoding="WINDOWS-1252" standalone="no"?> <eclipse-userlibraries version="2"> <library name="MyLibTest" systemlibrary="false"> <archive path="C:/SQLLIB/java/sqlj.zip"/> <archive path="C:/SQLLIB/java/Common.jar"/> </library> </eclipse-userlibraries> 2. On a MAC, import this file into the Window / Preferences / Java / Build Path / User Libraries dialog. 3.Try to reopen the User Library dialog -- you will get the error and not be able to remove the MyLibTest user library from the system. Nor can you add other user libraries.
Jay, please follow up.
Reproduced with I20110424-0800. Markus, do you think it's possible to address this while importing itself? If we want to handle it from the Core, then we might have to be content with an error log rather than display the error to the user.
(In reply to comment #2) > Reproduced with I20110424-0800. > > Markus, do you think it's possible to address this while importing itself? If > we want to handle it from the Core, then we might have to be content with an > error log rather than display the error to the user. Having said that, I still think it's better if Core handles the exception to cover the scenarios where the exported file might have been altered outside eclipse. Nathaniel, Talking about the encoding, the way it works is: if the location selected for export is inside the workspace, the encoding is taken from the workspace or the project settings. Otherwise, UTF-8 is considered as the default encoding.
Jay, please prepare a patch. Depending on the risk assessment, it might qualify for RC2.
I agree this should be fixed for RC2. I quickly investigated why the Windows-style Path "C:/SQLLIB/java/sqlj.zip" came into the system at all. Looks like it's because UserLibraryPreferencePage$LoadSaveDialog.loadLibraries(File) line: 704 now uses "Path.fromPortableString(..)" (to fix bug 78309 and bug 96222). I guess JDT Core should use a similar approach. Path.fromPortableString(..) resolves a "C:/..."=like path to an absolute path, so if everything works well afterwards, the AFE should not be thrown any more. It would be good to keep the import working even if one of the paths doesn't resolve to a file on disk. This way, it's much easier for the user to correct the situation after the import. Note that you also have to increase the version number and keep migration code for the old path format. Nathaniel: To fix the situation in an older build, please quit Eclipse and then manually edit ".metadata/.plugins/org.eclipse.core.runtime/.settings/org.eclipse.jdt.core.prefs" and remove the offending "org.eclipse.jdt.core.userLibrary.MyLibTest=..." entry.
I am working on a fix for RC2.
Created attachment 195942 [details] Propsed fix There are two parts to this fix: 1. As suggested by Markus, to use portable path for user libraries with backward compatibility. 2. Handle exception to cover other scenarios where the path might be invalid. Here I considered two alternatives in case of an exception: i) Remove the preference from the storage for the corrupt library and continue ii) Preserve the preference in the storage but do not consider the corresponding user library. I have decided go with the first, just as has been done in the case of an IOException with the existing code. The tests are passing on Mac and Windows XP and currently being run on Linux. I will update after the tests complete.
The changes are good in general. Minor comments. 1. The test throws an exception while setting an invalid entry through preferences. As of now, this exception is not being handled through the patch and I think this is fine, as this invalid entry couldn't be given through the UI. 2. Personally, I feel the following code in UserLibrary.java could be better laid ####### IPath entryPath = version.equals(VERSION_ONE) ? Path.fromOSString(pathString) : Path.fromPortableString(pathString); IPath sourceAttachPath = null; IPath sourceAttachRootPath = null; if (sourceAttachString != null) sourceAttachPath = version.equals(VERSION_ONE) ? Path.fromOSString(sourceAttachString) : Path.fromPortableString(sourceAttachString); if (sourceAttachRootString != null) sourceAttachRootPath = version.equals(VERSION_ONE) ? Path.fromOSString(sourceAttachRootString) : Path.fromPortableString(sourceAttachRootString); ######## as ####### if (entryPath.equals(VERSION_ONE)) { entryPath = ... // from path if (sourceAttachString != null) sourceAttachPath = ... if (sourceAttachRootString != null) sourceAttachRootPath = ... } else { // use portable strings entryPath = ... // from portable .... } ########
Thank you all very much -- I appreciate the work around proposed by Marcus. I don't know why grep wasn't finding this org.eclipse.jdt.core.prefs file (perhaps ignoring the . prefixed files as though they were hidden (I was searching on Windows with a cygwin based grep). The fix worked perfectly.
Looks good and works fine, though I agree with Satyam that a single version test would be nicer to read. Cosmetics: Missing space in "catch(ClasspathEntry.AssertionFailedException e)"
Unfortunately, you don't get an error flag right after importing, see bug 232815. But you get it when you visit the preference page the next time or when you try to build a project with a bad user lib on the build path.
Created attachment 195987 [details] Updated patch Patch incorporates comments from Satyam and Markus. Will release it after running few more tests.
Makes sure all copyrights are accurate. This is not the case in this last patch.
Created attachment 196039 [details] Same patch with updated copyrights
Thanks for the patch, Olivier. Released in HEAD for 3.7 RC2. Tests pass in Linux and Mac too.
Verified in I20110519-1138 on the Mac.
verified by Markus. Marking 'VERIFIED'.
