Page MenuHomePhabricator

Add the nvidia-cuda-toolkit Debian package path to search path

Authored by sylvestre.ledru on Nov 25 2017, 1:25 AM.

Diff Detail

rC Clang

Event Timeline

Hahnfeld edited edge metadata.

The change looks good in general and is small enough and last in the candidates list to not break working setups.

What concerns me is that we have to do the same for every distribution (I know that Arch Linux for example has CUDA installed in /opt/cuda, see In addition many HPC systems use the modules system to install multiple versions in parallel. As such we should make the installation detection smarter:

  1. We could have an environment variable that systems can set to the appropriate directory.
  2. We could try to automatically deduce the install path: If we find ptxas in the PATH and it is in a directory bin, its parent is a good candidate to start searching.

I think 2. would work for Arch Linux which adds /opt/cuda/bin to PATH IIRC. I'm not sure about Debian though, @sylvestre.ledru? If that doesn't work, we have to add the candidate statically anyway...

Debian packages don't update the PATH and we are aiming at providing packages working out of the box.

In general, yeah, we have to do that for every distros... :/

In that case I don't see a way to be "clever", so we have to add the static candidate. I'll let @tra or @jlebar have a final look.

tra requested changes to this revision.Nov 27 2017, 3:21 PM

I'm reluctant to add a distribution-specific search path unconditionally.

We do have Distro.IsDebian()

Adding distribution-specific path for particular distribution would be fine with me.

This revision now requires changes to proceed.Nov 27 2017, 3:21 PM
sylvestre.ledru edited edge metadata.
tra added inline comments.Nov 28 2017, 1:37 PM

No need for a named temporary. if (Distro(D.getVFS).IsDebian()) should do.

Also, please add a comment describing why Debian needs a special case here and include reference to the bug report which has the details.

On a related note please add context to your patch as described here:

tra accepted this revision.Nov 28 2017, 3:24 PM
This revision is now accepted and ready to land.Nov 28 2017, 3:24 PM
jlebar removed a reviewer: jlebar.Nov 28 2017, 5:10 PM
jlebar added a subscriber: jlebar.

I defer to tra on this.

This revision was automatically updated to reflect the committed changes.