----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3580/#review4546 -----------------------------------------------------------
Just a few minor nits. Otherwise this is great! configure.ac <https://reviews.apache.org/r/3580/#comment10145> Can we change the comment to "linker flags for JNI, when building Python egg" please? configure.ac <https://reviews.apache.org/r/3580/#comment10146> 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. configure.ac <https://reviews.apache.org/r/3580/#comment10143> Mind fixing this indentation? configure.ac <https://reviews.apache.org/r/3580/#comment10142> 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. src/Makefile.am <https://reviews.apache.org/r/3580/#comment10147> Move these two lines to line 408 (after the libmesos_no_third_party_la_LIBADD += libjava.la). src/Makefile.am <https://reviews.apache.org/r/3580/#comment10148> Wrap this line (under 80 characters) please. - Benjamin On 2012-01-23 07:02:50, Charles Reiss wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3580/ > ----------------------------------------------------------- > > (Updated 2012-01-23 07:02:50) > > > 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 > >
