You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2015/12/08 22:00:40 UTC

Re: Review Request 39780: Update OversubscriptionTest to not assume dynamic dlopen search.


> 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
> 
>