RFR: 7134701 [macosx] Support legacy native library names

[Daniel D. Daugherty]

On 3/28/12 3:52 AM, Michael McMahon wrote:
> On 28/03/12 08:40, Alan Bateman wrote:
>> On 28/03/2012 04:51, Scott Kovatch wrote:
>>> With this patch in place I can load the applet at Runescape.com now. 
>>> I can't log in yet due to an AWT bug I probably haven't patched yet, 
>>> but this is better than what it was.
>>>
>>> Minecraft.net is still giving me problems because it's bailing out 
>>> before this change takes place. Here's the method in question:
>>>
>>> private static void doLoadLibrary(final String lib_name) {
>>>     AccessController.doPrivileged(new PrivilegedAction<Object>() {
>>>         public Object run() {
>>>             String library_path = 
>>> System.getProperty("org.lwjgl.librarypath");
>>>             if (library_path != null) {
>>>                 System.load(library_path + File.separator +
>>>                     System.mapLibraryName(lib_name));
>>>             } else {
>>>                 System.loadLibrary(lib_name);
>>>             }
>>>             return null;
>>>         }
>>>     });
>>> }
>>>
>>> It's failing because System.mapLibraryName is constructing a library 
>>> named liblwjgl.dylib, but lwjgl only has 'liblwjgl.jnilib'. Apple's 
>>> JDK 6 (and I suspect macosx-port) gives me 'liblwjgl.jnilib' for 
>>> System.mapLibraryName("lwjgl").
>>>
>>> I can't even get this far with Apple's JDK 6 on minecraft.net, so 
>>> it's hard to say this is a pure regression. I'm hesitant to suggest 
>>> that we have mapLibraryName start returning a library named 
>>> lib<xxx>.jnilib at this point.
>>>
>>> Any ideas for a workaround? I think your ClassLoader/System change 
>>> shown here should still go in, but in this particular case we still 
>>> have problems. I don't know how much code is out there that uses 
>>> mapLibraryName in this way.
>>>
>>> -- Scott
>> I just checked a Mac OSX 10.6.8 system with 6u29 installed and 
>> System.mapLibraryName("foo") returns foo.jnilib. Do you know if this 
>> was always the case? If so then I think there is an argument to be 
>> made that the default should be .jnilib and .dynlib be the fallback, 
>> especially if it's going to break anyone using mapLibraryName (my 
>> guess is that mapLibraryName usages are rare, at least compared to 
>> System.loadLibrary).
>>
>> -Alan
>>
>>
>>
> Maybe, we should be doing what Dan Daugherty suggested yesterday and 
> re-mapping the native
> filename in the case where a full absolute path is passed in (which is 
> the code-path
> when System.load() is called above)
>
> My reasoning (for rejecting that) was that an absolute path is a 
> request for an explicit name
> eg. System.load("/abs/path/libfoo.dylib") and it seems odd to go 
> modifying the path that
> the user provided (as opposed to the System.loadLibrary("foo") case, 
> where we have to
> construct the path internally). But, I hadn't considered the case 
> above where the path
> is constructed by calling System.mapLibraryName().
>
> So, I think a better approach might be to just to check for the old 
> ".jnilib" suffix in all
> the cases, rather than changing System.mapLibraryName(). Otherwise, 
> we'll have an inconsistency
> forever more between the output of that method and the library suffix 
> that we want people
> to use.
>
> I've attached the jdk8 diffs for doing this.
>
> - Michael

This difference caught my eye:

+            File libfile = new File(name);

and

+        final File libfile = ClassLoaderHelper.mapAlternativeName(file);


Not a big deal, but it wasn't clear to me why you used 'final'
for the second libfile and not the first one...

Dan