> On Nov. 30, 2015, 8:51 a.m., Benjamin Bannier wrote: > > src/tests/oversubscription_tests.cpp, line 117 > > <https://reviews.apache.org/r/39780/diff/3/?file=1133385#file1133385line117> > > > > This isn't really too nice, and e.g. cmake does not put build libraries > > into these paths (i.e. what you write will only work with automake). > > > > Like I already pointed out on #40553, you should really lift the level > > of abstraction here, and use a function to get this path. > > James Peach wrote: > The intent of the original code was clearly to use this path. When > running in the context of the automake wrapper scripts, automake constructs > LD_LIBRARY_PATH so that we don't really need to do anything here. What does > cmake do? Can you point me to any documentation of how it handles library > search paths when executing artifacts from the build directory? > > Benjamin Bannier wrote: > You are of course right that this wasn't yours. > > My point is that in #40553 you introduce abstractions like `findModule` > that should be perfectly good to capture what was written here. > > James Peach wrote: > Do you recommend rebasing this onto the subsequent patches? Or adding a > new ```getX``` helper here and rebasing the later patches on to it? Or > something else that will work for both automake and cmake?
If we want and abstraction like ```findModule``` then IMHO #40553 or a separate change is the right way to do that. Let's keep this change within the current scope. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39780/#review108299 ----------------------------------------------------------- On Nov. 26, 2015, 2:03 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39780/ > ----------------------------------------------------------- > > (Updated Nov. 26, 2015, 2:03 p.m.) > > > Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and > Till Toenshoff. > > > Bugs: MESOS-3725 > https://issues.apache.org/jira/browse/MESOS-3725 > > > Repository: mesos > > > Description > ------- > > Update OversubscriptionTest to not assume dynamic dlopen search. > > > Diffs > ----- > > src/tests/oversubscription_tests.cpp > 0d0bf7e0b9a4028ed116e00b56d59b670510c5ce > > Diff: https://reviews.apache.org/r/39780/diff/ > > > Testing > ------- > > make check > > > Thanks, > > James Peach > >
