> On Sept. 28, 2017, 5:22 p.m., Benjamin Bannier wrote: > > configure.ac > > Lines 239 (patched) > > <https://reviews.apache.org/r/62661/diff/1/?file=1838943#file1838943line239> > > > > Let's call this `libtool-wrappers` to match what we use in the code (it > > also refers to multiple wrappers in the end).
Fixed. > On Sept. 28, 2017, 5:22 p.m., Benjamin Bannier wrote: > > configure.ac > > Lines 241-242 (patched) > > <https://reviews.apache.org/r/62661/diff/1/?file=1838943#file1838943line241> > > > > This helpstring needs to clearly call out that the resulting binaries > > *cannot be installed* properly (due to the hardcoded build lib dir in > > `RPATH`). This is more important that what exact flag we pass down to > > `libtool`, e.g., something along the lines of > > > > Directly create binaries instead of libtool wrapper scripts to > > prevent relinking when first executing binaries from the build directory. > > This flag leads to binaries with the build directory encoded in the > > executables' RPATH which makes them in general unfit for installation. USE > > WITH CARE. Ok. Fixed. > On Sept. 28, 2017, 5:22 p.m., Benjamin Bannier wrote: > > configure.ac > > Lines 667 (patched) > > <https://reviews.apache.org/r/62661/diff/1/?file=1838943#file1838943line667> > > > > The control flow in the makefile might be easier to follow if we define > > `DISABLE_LIBTOOL_WRAPPERS` here instead, > > > > AM_CONDITIONAL([DISABLE_LIBTOOL_WRAPPERS], [test > > x"enable_libtool_wrappers" != "xno"]) Fixed. > On Sept. 28, 2017, 5:22 p.m., Benjamin Bannier wrote: > > src/Makefile.am > > Lines 242-247 (patched) > > <https://reviews.apache.org/r/62661/diff/1/?file=1838944#file1838944line242> > > > > I'd suggest to not define this variable, but instead modify > > `AM_LDFLAGS`, > > > > AM_LDFLAGS += -no-install > > > > With that should we add another executable it would automatically pick > > up the flag without having to set its `LDFLAGS`. Fixed. > On Sept. 28, 2017, 5:22 p.m., Benjamin Bannier wrote: > > src/Makefile.am > > Lines 1587 (patched) > > <https://reviews.apache.org/r/62661/diff/1/?file=1838944#file1838944line1587> > > > > This isn't needed if we modify the global `AM_LDFLAGS`. > > > > Similar for all instances below. Fixed. - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62661/#review186569 ----------------------------------------------------------- On Sept. 28, 2017, 11:37 a.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62661/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2017, 11:37 a.m.) > > > Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Kapil > Arya. > > > Bugs: MESOS-7500 > https://issues.apache.org/jira/browse/MESOS-7500 > > > Repository: mesos > > > Description > ------- > > This flag is used to force libtool to generate executables instead of > wrapper scripts. A wrapper script might trigger relinking, which takes > quite a while on slow machines, thus causing failure of tests. > > > Diffs > ----- > > configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a > src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 > > > Diff: https://reviews.apache.org/r/62661/diff/2/ > > > Testing > ------- > > 1. sudo make check (fedora) > 2. Apache CI (centos 7, ubuntu 14.04) > > > Thanks, > > Andrei Budnik > >
