> On 2012-01-24 02:46:34, Benjamin Hindman wrote: > > configure.ac, line 118 > > <https://reviews.apache.org/r/3580/diff/3/?file=70253#file70253line118> > > > > Can we change the comment to "linker flags for JNI, when building > > Python egg" please?
Done. > On 2012-01-24 02:46:34, Benjamin Hindman wrote: > > configure.ac, lines 226-227 > > <https://reviews.apache.org/r/3580/diff/3/?file=70253#file70253line226> > > > > Is there any clean way to wrap these two so they don't go beyond 80 > > characters? The rest of the file should meet this constraint (except for > > maybe one line), and I'd like to keep it as much as possible. Refactored (using temp var). > On 2012-01-24 02:46:34, Benjamin Hindman wrote: > > configure.ac, line 242 > > <https://reviews.apache.org/r/3580/diff/3/?file=70253#file70253line242> > > > > Mind fixing this indentation? Done. > On 2012-01-24 02:46:34, Benjamin Hindman wrote: > > configure.ac, lines 276-278 > > <https://reviews.apache.org/r/3580/diff/3/?file=70253#file70253line276> > > > > So if these three are all AC_ARG_VAR then they don't actually need > > AC_SUBST ... not sure why I left the JAVA_LDFLAGS one around. Removed all. > On 2012-01-24 02:46:34, Benjamin Hindman wrote: > > src/Makefile.am, lines 403-404 > > <https://reviews.apache.org/r/3580/diff/3/?file=70254#file70254line403> > > > > Move these two lines to line 408 (after the > > libmesos_no_third_party_la_LIBADD += libjava.la). Done. > On 2012-01-24 02:46:34, Benjamin Hindman wrote: > > src/Makefile.am, line 493 > > <https://reviews.apache.org/r/3580/diff/3/?file=70254#file70254line493> > > > > Wrap this line (under 80 characters) please. Done. - Charles ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3580/#review4546 ----------------------------------------------------------- On 2012-01-24 08:33:36, Charles Reiss wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3580/ > ----------------------------------------------------------- > > (Updated 2012-01-24 08:33:36) > > > Review request for mesos. > > > Summary > ------- > > On Linux, I've needed to build with -Wl,-rpath to nicely link against -ljvm. > We should do something like this automatically. > > This patch (on Linux only) uses -Wl,-ljvm instead of -ljvm to link -ljvm so > libtool doesn't add it to dependency_libs [*]. Besides hopefully being > unnecessary, it fails to link things linked against that libtool library > because libtool doesn't add the rpath for the java libraries (even if I > specify it with -R). This also requires that the flags be added on > libmesos.la and uses the assumption that libmesos.la is linked dynamically. > > [*] Things added to dependency_libs are automatically linked into any > executables linking against the library; this would be fine (and would make > sense if our header files might contain inline functions that call JNI_* > calls) if the -R settings were properly passed through, but libtool does not > appear to do this. > > > This addresses bug MESOS-127. > https://issues.apache.org/jira/browse/MESOS-127 > > > Diffs > ----- > > configure.ac bda6769 > src/Makefile.am b8fc58e > > Diff: https://reviews.apache.org/r/3580/diff > > > Testing > ------- > > > Thanks, > > Charles > >
