Bug 12463 - Plugin.find(IPath path, Map override) searches wrong directories
Summary: Plugin.find(IPath path, Map override) searches wrong directories
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Resources (show other bugs)
Version: 2.0   Edit
Hardware: PC Windows 2000
: P2 critical (vote)
Target Milestone: 2.0.1   Edit
Assignee: Debbie Wilson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2002-03-28 14:36 EST by Konrad Kolosowski CLA
Modified: 2002-08-01 12:35 EDT (History)
4 users (show)

See Also:


Attachments
Attached patch for fix. Same as in-line patch in previous comment. (2.04 KB, patch)
2002-08-01 10:25 EDT, Dorian Birsan CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Konrad Kolosowski CLA 2002-03-28 14:36:50 EST
For the path $nl$/file and override being %nl%=ja_JP locale, the method 
searches directories nl/ja/JP and nl.  It should search nl/ja_JP, nl/ja, and nl.
I suppose Plugin.find(IPath path) does the same thing.
Comment 1 Konrad Kolosowski CLA 2002-03-28 14:37:57 EST
Happens in 20020321 integration build.
Comment 2 DJ Houghton CLA 2002-04-01 17:24:23 EST
For directories under NL, the underscores are replaced by slashes. So the 
search order will be:
  /nl/ja/JP/
  /nl/ja/
  /nl/
We have a document/spec describing this behaviour. Debbie, please forward me 
this doc so we can make it available for users. Thanks.
Comment 3 Konrad Kolosowski CLA 2002-04-01 18:17:44 EST
It does not search
  /nl/ja/
, searches only
  /nl/ja/JP/
  /nl/
Comment 4 Debbie Wilson CLA 2002-04-16 15:50:33 EDT
It does appear that this is now working correctly.  Here's what I did to verify:

- using build 20020412
- rename <home>\eclipse\plugins\org.eclipse.core.resources\os\win32
\core_2_0_5.dll to xxxcore_2_0_5.dll (so it can't be found)
- move the .options file found under org.eclipse.core.runtime to 
<home>\eclipse\.options
- edit this file as follows:
# Turn on debugging for the class loader.
org.eclipse.core.runtime/loader/debug=true
...
# Display when plugins are activated
org.eclipse.core.runtime/loader/debug/activateplugin=true
# Debug all class loader actions (finding resource, classes, libraries)
org.eclipse.core.runtime/loader/debug/actions=true
# Debug success case only
org.eclipse.core.runtime/loader/debug/success=true
# Debug failure case only
org.eclipse.core.runtime/loader/debug/failure=true
- run Eclipse with the command line parameter -debug
- the log file (or your console if you use the parameter -consolelog) contained 
the following (along with a ton of other stuff):

Loader [org.eclipse.core.resources_2.0.20]^dcfbd6a findLibrary(core_2_0_5.dll)
Loader [org.eclipse.core.resources_2.0.20]^dcfbd6a not found 
c:\20020412a\eclipse\plugins\org.eclipse.core.resources\ws\win32\core_2_0_5.dll
Loader [org.eclipse.core.resources_2.0.20]^dcfbd6a not found 
c:\20020412a\eclipse\plugins\org.eclipse.core.resources\os\win32\x86
\core_2_0_5.dll
Loader [org.eclipse.core.resources_2.0.20]^dcfbd6a not found 
c:\20020412a\eclipse\plugins\org.eclipse.core.resources\os\win32\core_2_0_5.dll
Loader [org.eclipse.core.resources_2.0.20]^dcfbd6a not found 
c:\20020412a\eclipse\plugins\org.eclipse.core.resources\nl\en\CA\core_2_0_5.dll
Loader [org.eclipse.core.resources_2.0.20]^dcfbd6a not found 
c:\20020412a\eclipse\plugins\org.eclipse.core.resources\nl\en\core_2_0_5.dll
Loader [org.eclipse.core.resources_2.0.20]^dcfbd6a not found 
c:\20020412a\eclipse\plugins\org.eclipse.core.resources\core_2_0_5.dll

So, we are now looking in all the subdirectories.  Note, however that we don't 
look in the nl directory alone.



Comment 5 Konrad Kolosowski CLA 2002-04-18 19:27:06 EDT
This is still broken.  I do not think your test is good for testing Plugin.find
(IPath path, Map override).

If you just launch Eclipse, the Plugin.find(IPath path, Map override) with a 
the path containing "$nl$" is never invoked.  The plugin.find() is invoked for 
paths: platform.ini, platform.properties, product.ini, product.properties, 
welcome.xml, prod.gif, and about.gif, but this is not going to test the NL 
lookup sequence.

If you do not have a better test, I would suggest placing a breakpoint at the 
beginning of Plugin.find(IPath path, Map override) and launching help.
Comment 6 Debbie Wilson CLA 2002-04-22 16:02:36 EDT
One case remainded in Plugin.findNL(...) which would drop everything from the 
last '_' off the nl string instead of everything from the last '/'.
Comment 7 Dorian Birsan CLA 2002-07-31 16:44:36 EDT
There is a regression caused by the introduction of the static var 
NL_JAR_VARIANTS. This field is constructed based on the current locale, 
so that the nl value from the "override" parameter in findNL() is never used.

Here is a possible patch:

Index: PluginDescriptor.java
===================================================================
RCS 
file: /home/eclipse/org.eclipse.core.runtime/src/org/eclipse/core/internal/plugi
ns/PluginDescriptor.java,v
retrieving revision 1.32
diff -u -r1.32 PluginDescriptor.java
--- PluginDescriptor.java	23 Jul 2002 13:54:34 -0000	1.32
+++ PluginDescriptor.java	31 Jul 2002 20:39:06 -0000
@@ -45,6 +45,8 @@
 	// Places to look for library files 
 	private static String[] WS_JAR_VARIANTS = buildWSVariants();
 	private static String[] OS_JAR_VARIANTS = buildOSVariants();
+	// Next static var is not really needed. 
+	// At best, for performance reasons, one could create a hashtable of 
nl_jar_variants, indexed by locale.
 	private static String[] NL_JAR_VARIANTS = buildNLVariants();
 	private static String[] JAR_VARIANTS = buildVanillaVariants();
 
@@ -65,7 +67,12 @@
 	return (String[])result.toArray(new String[result.size()]);
 }
 private static String[] buildNLVariants() {
-	String nl = BootLoader.getNL();	
+	return buildNLVariants(BootLoader.getNL());
+}
+private static String[] buildNLVariants(String nl) {	
+	// Theoretically, we could create a hash table of all the nl variants
+	// built, indexed by the nl string. This would avoid extra computation 
+	// and memory allocation. 
 	ArrayList result = new ArrayList();
 	IPath base = new Path("nl"); //$NON-NLS-1$
 	
@@ -909,6 +916,7 @@
 
 private URL findNL(URL install, IPath path, Map override) {
 	String nl = null;
+
 	if (override != null)
 		try {
 			// check for override
@@ -923,8 +931,9 @@
 		return null;
 
 	URL result = null;
-	for (int i=0; i<NL_JAR_VARIANTS.length; i++) {
-		IPath filePath = new Path(NL_JAR_VARIANTS[i]).append(path);
+	String[] nl_jar_variants = buildNLVariants(nl);
+	for (int i=0; i<nl_jar_variants.length; i++) {
+		IPath filePath = new Path(nl_jar_variants[i]).append(path);
 		result = findInPlugin(install, filePath);
 		if (result != null)
 			return result;
@@ -980,4 +989,4 @@
 	}
 	return null;
 }
-}
+}
\ No newline at end of file
Comment 8 DJ Houghton CLA 2002-08-01 10:19:53 EDT
Please adjust the severity as how important you feel this PR is to be fixed.
Then we can adjust the target milestone accordingly. Thanks.

Thanks for the patch. For future reference, Eclipse patches should be attached 
to Bugzilla as file attachments and not pasted into the editor as they cannot 
be used when copied from the web client. (due to whitespace, etc)
Comment 9 Dorian Birsan CLA 2002-08-01 10:25:49 EDT
Created attachment 1782 [details]
Attached patch for fix. Same as in-line patch in previous comment.
Comment 10 Dorian Birsan CLA 2002-08-01 10:29:33 EDT
Updated severity to "critical". Without a fix, the help infocenter support 
would be useless for any language other than the current locale. Infocenters 
are meant to deliver localized help in many languages.
Comment 11 Kari Halsted CLA 2002-08-01 10:39:10 EDT
Currently, this is preventing the national language infocenter scenario of the 
help system from working as designed. In the scenario, help in multiple 
languages could be installed on a remote server, and accessed from various 
clients in the language of the client locale. Currently, only English is being 
served, even if the client locale is something else. Really need fixed in 2.0.1.
Comment 12 DJ Houghton CLA 2002-08-01 11:10:34 EDT
Will fix for 2.0.1.
Comment 13 Debbie Wilson CLA 2002-08-01 12:08:18 EDT
Fix applied to HEAD stream.  Includes changes to the following:
org.eclipse.core.runtime/org.eclipse.core.internal.plugins/PluginDescriptor
org.eclipse.core.tests.runtime/org.eclipse.core.test.plugintests/PluginFindTests

Fix will be applied to 2.0.1 stream.
Comment 14 DJ Houghton CLA 2002-08-01 12:35:05 EDT
Fixed and released into 2.0.1 stream.