> On March 8, 2016, 7:15 p.m., Ben Mahler wrote: > > Looks good, main thing is just to add some context to the installer script > > so that others understand why it exists. > > Ben Mahler wrote: > Could you update the testing done so that others can tell how you tested > this?
Done. > On March 8, 2016, 7:15 p.m., Ben Mahler wrote: > > support/install-nvidia-gdk.sh, lines 1-2 > > <https://reviews.apache.org/r/44360/diff/2/?file=1280297#file1280297line1> > > > > Could we pull down some of the content from the description into a > > block comment here? A few things that would be great to have: > > > > -What is the GDK? > > -Why do we include this installer script? > > -How to uninstall. > > -That apparently nvidia will stop publishing GDKs? We should check with > > Rob on that. > > > > Would be helpful also to mention why we tie this to a particular > > version. I added everything you mentioned here in a comment except for mentioning why we tie this to a particular version. Instead, I mention that the script downloads the GDK for Nvidia's latest drivers (which should always be backwards compatible). So long as we keep the script up to date, we shouldn't have to worry about versioning. Also, given what I said above, I think we can hold off mentioning anything about Nvidia discontinuing the GDK in favor of just updating this script to properly install the equivalent stub libraries when the time comes. > On March 8, 2016, 7:15 p.m., Ben Mahler wrote: > > support/install-nvidia-gdk.sh, line 21 > > <https://reviews.apache.org/r/44360/diff/2/?file=1280297#file1280297line21> > > > > Should this include the ldcache flag as well? Done. > On March 8, 2016, 7:15 p.m., Ben Mahler wrote: > > support/install-nvidia-gdk.sh, line 89 > > <https://reviews.apache.org/r/44360/diff/2/?file=1280297#file1280297line89> > > > > Can you check for errors on wget and make sure that it exits nicely? > > (e.g. test it out with a bad URL). Done. > On March 8, 2016, 7:15 p.m., Ben Mahler wrote: > > support/install-nvidia-gdk.sh, line 95 > > <https://reviews.apache.org/r/44360/diff/2/?file=1280297#file1280297line95> > > > > I probably wouldn't know what "invalid" means in this context. :) Updated. > On March 8, 2016, 7:15 p.m., Ben Mahler wrote: > > support/install-nvidia-gdk.sh, lines 99-100 > > <https://reviews.apache.org/r/44360/diff/2/?file=1280297#file1280297line99> > > > > Maybe a little more context that nvidia's installer crashes when it > > encounters this file during re-installation? Done. > On March 8, 2016, 7:15 p.m., Ben Mahler wrote: > > support/install-nvidia-gdk.sh, lines 102-104 > > <https://reviews.apache.org/r/44360/diff/2/?file=1280297#file1280297line102> > > > > Maybe a small note about why we use `--silent` here? Done. > On March 8, 2016, 7:15 p.m., Ben Mahler wrote: > > support/install-nvidia-gdk.sh, line 108 > > <https://reviews.apache.org/r/44360/diff/2/?file=1280297#file1280297line108> > > > > Could we move this up to the top so that it's clearer that this is a > > side effect file after installation? Done. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44360/#review122572 ----------------------------------------------------------- On March 14, 2016, 12:25 a.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44360/ > ----------------------------------------------------------- > > (Updated March 14, 2016, 12:25 a.m.) > > > Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya. > > > Bugs: MESOS-4860 > https://issues.apache.org/jira/browse/MESOS-4860 > > > Repository: mesos > > > Description > ------- > > This script can be used to install the Nvidia GPU Deployment Kit (GDK) > on a mesos development machine. The purpose of the GDK is to provide > stubs for compiling code against Nvidia's latest drivers. It includes > all the necessary header files (nvml.h) and library files > (libnvidia-ml.so) necessary to build mesos with Nvidia GPU support. > However, it does not actually contain any driver code, and it can't be > used to interact with any GPUs installed on a system. For that, you > will have to find the latest released drivers themselves. > > We provide this script only for convenience, so that developers > working on non-GPU equipped systems can build code with Nvidia GPU > support. It is a simple wrapper around the official GDK installer > script provided by Nvidia, which can be downloaded here: > > https://developer.nvidia.com/gpu-deployment-kit > > The script itself takes a few parameters. Run it as > 'support/install-nvidia-gdk.sh --help' to see its usage semantics. > > > Diffs > ----- > > support/install-nvidia-gdk.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/44360/diff/ > > > Testing > ------- > > I ran the script on my Mac to verify that it fails out cleanly. > > I ran it on a linux box to exercise each of the different flags that can be > passed to it. > ``` > install-nvidia-gdk.sh --install-dir=<root_directory> > install-nvidia-gdk.sh --install-dir=<non_root_directory> > install-nvidia-gdk.sh --install-dir=<root_directory> --update-ldcache > install-nvidia-gdk.sh --install-dir=<non_root_directory> --update-ldcache > ``` > > With each combination, I verified that the proper error messages were > reported, or that the installation was successful. > > As per one of the comments below, I also temporarily changed the download URL > for the installer to make sure that the `wget` call errored out cleanly. > > > Thanks, > > Kevin Klues > >
