mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kone" <vinodk...@gmail.com>
Subject Re: Review Request 12618: Extended MesosNativeLibrary.java to load the native library from an absolute path.
Date Thu, 18 Jul 2013 23:52:55 GMT


> On July 17, 2013, 3:16 a.m., Ben Mahler wrote:
> > src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in, line 80
> > <https://reviews.apache.org/r/12618/diff/1/?file=322306#file322306line80>
> >
> >     What about having the load(path) overload should only try the path provided
since it's being explicitly specified?
> >     
> >     This way load() can call load(System.getenv("MESOS_NATIVE_LIBRARY")) when it's
set, otherwise it will try the System.loadLibrary("mesos");
> >     
> >     i.e.
> >     
> >     // Tries to load at the specified path.
> >     load(String path) {
> >       if (loaded) return;
> >     
> >       ClassLoader contextClassLoader =
> >         Thread.currentThread().getContextClassLoader();
> >       Thread.currentThread().setContextClassLoader(
> >         MesosNativeLibrary.class.getClassLoader());
> >     
> >       try {
> >         System.load(path);
> >       } catch (UnsatisfiedLinkError error) {
> >         System.err.println("Failed to load native Mesos library at " + path);
> >         throw error;
> >       } finally {
> >         Thread.currentThread().setContextClassLoader(contextClassLoader);
> >       }  
> >     }
> >     
> >     // Uses MESOS_NATIVE_LIBRARY if set in the environment. Otherwise tries
> >     // to load the "mesos" library.
> >     load() {
> >       if (loaded) return;
> >     
> >       // Use MESOS_NATIVE_LIBRARY if specified.
> >       if (System.getenv("MESOS_NATIVE_LIBRARY") != null) {
> >         load(System.getenv("MESOS_NATIVE_LIBRARY"));
> >         return;
> >       }
> >     
> >       ClassLoader contextClassLoader =
> >         Thread.currentThread().getContextClassLoader();
> >       Thread.currentThread().setContextClassLoader(
> >         MesosNativeLibrary.class.getClassLoader());
> >      
> >       try {
> >         System.loadLibrary("mesos");
> >       } catch (UnsatisfiedLinkError error) {
> >         System.err.println("Failed to load native Mesos library from " +
> >             System.getProperty("java.library.path"));
> >         throw error;
> >       } finally {
> >         Thread.currentThread().setContextClassLoader(contextClassLoader);
> >       }
> >     }
> >     
> >     
> >     I guess the annoyance here is having to duplicate the class loader code, but
the functions have a clearer behavior, right now the load(path) method does quite a lot, no?
> 
> Vinod Kone wrote:
>     Thanks for the code. That does look better. Fixed.
> 
> Benjamin Hindman wrote:
>     If we had an option type you could imagine passing in an optional path and using
System.loadLibrary when the path is none. You could accomplish this with just looking to see
if 'path' is null (which seems to be a standard Java thing to do without an option type).
That would look like:
>     
>     void load(String path) {
>       if (loaded) {
>         return;
>       }
>     
>       ClassLoader contextClassLoader =
>         Thread.currentThread().getContextClassLoader();
>       Thread.currentThread().setContextClassLoader(
>         MesosNativeLibrary.class.getClassLoader());
>     
>       try {
>         if (path != null) {
>           System.load(path);
>         } else {
>           System.loadLibrary("mesos")
>         }
>       } catch (UnsatisfiedLinkError error) {
>         if (path != null) {
>           System.err.println("Failed to load native Mesos library at " + path);
>         } else {
>           System.err.println("Failed to load native Mesos library from " +
>                              System.getProperty("java.library.path"));
>         }
>         throw error;
>       } finally {
>         Thread.currentThread().setContextClassLoader(contextClassLoader);
>       }  
>     }
>     
>     void load() {
>       load(System.getenv("MESOS_NATIVE_LIBRARY"));
>     }

Hmm...This looks nice too. BenM, are you ok with this?


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12618/#review23231
-----------------------------------------------------------


On July 18, 2013, 9:18 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12618/
> -----------------------------------------------------------
> 
> (Updated July 18, 2013, 9:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Used this in a subsequent review for Jenkins scheduler.
> 
> 
> Diffs
> -----
> 
>   src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in 56e97725ae0e94ebd10ce9bc9ba66981a07558fe

> 
> Diff: https://reviews.apache.org/r/12618/diff/
> 
> 
> Testing
> -------
> 
> make jenkins
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message