> On March 8, 2016, 7:33 p.m., Ben Mahler wrote:
> > configure.ac, lines 957-976
> > <https://reviews.apache.org/r/44361/diff/1/?file=1280267#file1280267line957>
> >
> >     Could we flatten this to make it a bit easier to read?

There are alot of examples in configure.ac that still do it this way (e.g.:

```
AC_CHECK_HEADERS([event2/event.h],
                 [AC_CHECK_LIB([event],
                 ...
```

However, I agree that splitting them up is clearer and should still be correct 
since we always error if we can't find the headers.

Updated.


> On March 8, 2016, 7:33 p.m., Ben Mahler wrote:
> > configure.ac, lines 215-216
> > <https://reviews.apache.org/r/44361/diff/1/?file=1280267#file1280267line215>
> >
> >     Could you do a sweep and capitalize NVML, Nvidia, and GPU in the 
> > comments? The convention is to do acronyms in all caps, but we 
> > unfortunately do this for classes as well, like URL, which we should change 
> > at some point since it leads to confusing name boundaries (e.g. 
> > HTTPConnection would be better named as HttpConnection, this doesn't exist 
> > but it's just an example. Another example is http:: vs JSON::, ideally 
> > namespace names are only lower case).

Done.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44361/#review122582
-----------------------------------------------------------


On March 14, 2016, 1:12 a.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44361/
> -----------------------------------------------------------
> 
> (Updated March 14, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4861
>     https://issues.apache.org/jira/browse/MESOS-4861
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is the initial commit to begin adding native support for GPUs in
> Mesos. This initial version will only include support for Nvidia GPUs
> that can be managed by the Nvidia Management Library (nvml).
> 
> The configure flags added in this commit can be used to enable Nvidia
> GPU support, as well as specify the installation directories of the
> nvml header and library files if not already installed in standard
> include/library paths on the system.
> 
> In a subsequent commit, we will use these configure flags to
> conditionally build support for Nvidia GPUs into Mesos.
> 
> 
> Diffs
> -----
> 
>   configure.ac 8e4f03593df4a8ba13f00292963e351acc3f71c1 
> 
> Diff: https://reviews.apache.org/r/44361/diff/
> 
> 
> Testing
> -------
> 
> I ran `bootstrap` to generate configure.
> 
> I then ran:
> 
> ```
> mkdir build; cd build
> ../configure --enable-nvidia-gpu-support
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<path_to_headers>
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<bogus_path>
> ../configure --enable-nvidia-gpu-support --with-nvml-lib=<path_to_lib>
> ../configure --enable-nvidia-gpu-support --with-nvml-lib=<bogus_path>
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<bogus_path> 
> --with-nvml-lib=<path_to_lib>
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include=<path_to_headers> --with-nvml-lib=<bogus_path>
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<bogus_path> 
> --with-nvml-lib=<bogus_path>
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include=<path_to_headers> --with-nvml-lib=<path_to_lib>
> ```
> 
> And verified the proper errors / successes in each case (only the last one is 
> a success).
> 
> The exact command I ran in the success case for my configuration was:
> ```
> ../configure --enable-nvidia-gpu-support 
> --with-nvml-include=/opt/nvidia-gdk/usr/include 
> --with-nvml-lib=/opt/nvidia-gdk/usr/src/gdk/nvml/lib
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Reply via email to