conda-forge / nvcc-feedstock

A conda-smithy repository for nvcc.
BSD 3-Clause "New" or "Revised" License
12 stars 23 forks source link

Symlink libcuda.so #84

Closed galipremsagar closed 2 years ago

galipremsagar commented 2 years ago

Checklist

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

jakirkham commented 2 years ago

@conda-forge-admin, please re-render

github-actions[bot] commented 2 years ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/nvcc-feedstock/actions/runs/2385824767.

jakirkham commented 2 years ago

Would enable azure/store_build_artifacts in conda-forge.yml to ensure binaries built here are available on Azure to try out. This will need another re-render.

Also would bump the build number

jakirkham commented 2 years ago

cc @robertmaynard

galipremsagar commented 2 years ago

@conda-forge-admin, please re-render

github-actions[bot] commented 2 years ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/nvcc-feedstock/actions/runs/2386263206.

jakirkham commented 2 years ago

@conda-forge-admin, please re-render

jakirkham commented 2 years ago

Ok once CI builds complete, would follow these instructions to try the packages

jakirkham commented 2 years ago

When extracting the zip file, a build_artifacts directory should show up. This can be treated as a channel. IOW one can do

conda install -c file:///<path to build_artifacts>/build_artifacts nvcc_linux-64
galipremsagar commented 2 years ago

When extracting the zip file, a build_artifacts directory should show up. This can be treated as a channel. IOW one can do

conda install -c file:///<path to build_artifacts>/build_artifacts nvcc_linux-64

I see, thanks. Didn't know this and was wondering why there is only build_artifacts.

jakirkham commented 2 years ago

Yeah it is archiving the whole build_artifacts directory, which is what is used for the whole build. So contains more than just the package. If there are ways to improve the docs to make this clearer, would be interested on your feedback on this 🙂 Though maybe we can save for a PR after

galipremsagar commented 2 years ago

@conda-forge-admin, please re-render

github-actions[bot] commented 2 years ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/nvcc-feedstock/actions/runs/2386864171.

jakirkham commented 2 years ago

JFYI this seems to be roughly working for us in terms of building. Though the stub library can't stay where it is (as noted in the thread above). We also find that issue when loading libraries locally (though removing the stub library at runtime fixes that). Going to try to dig into this a bit more, but it may not be until tomorrow

vyasr commented 2 years ago

Forgive my ignorance, but could someone please summarize exactly what use cases that we are trying to support?

robertmaynard commented 2 years ago

Forgive my ignorance, but could someone please summarize exactly what use cases that we are trying to support?

  • Can users make use of system host compilers, or is the assumption that they will be using conda-provided host compilers (gcc)?

I would state that the goal is to have this file behave as is for users using system host compilers, with the updates allowing conda-provided compilers to find the libcuda.so stub in the conda env. Currently the assumption for system host compilers is that they can find an existing system libcuda.so ( stub or real ).

  • Does the build process always use the libcuda.so stub, even if we're on a system that actually has GPU drivers enabled (i.e. if there is a functional libcuda.so available in addition to the stub)? Is that managed by the compiler's sysroot? Does the existence or nonexistence of a functional libcuda.so change what the build process might find?

CMake always searches for real libraries before stub libraries. Specifically CMake only searches inside stubs/ after first searching other directories. Since we are adding a stub file to a system search location this will only be used by CMake as the last search location.

This behavior is managed by how CMake performs find_library calls and the order of search locations, which ensures that compiler / system extracted locations are the last location it searches.

  • Is there any scenario, now or in the future where we would need to support an actual libcuda.so provided by conda? That is not the current state of the cudatoolkit conda-forge feedstock, but it may be the future, right?

As far as I am aware this is something we will need to support. If we always place the libcuda.so in a stubs directory we won't clobber a real libcuda.so and will be future proofed.

isuruf commented 2 years ago

My question is: Why would anyone want to use this package in a non-conda-build environment?

jakirkham commented 2 years ago

Because it works better with the Conda compilers, which are also being used in build environments. That in turn because it is useful to develop with the compilers used to build the packages later

isuruf commented 2 years ago

If that's the case, then CONDA_BUILD_SYSROOT is defined and this PR is unnecessary.

kkraus14 commented 2 years ago

If that's the case, then CONDA_BUILD_SYSROOT is defined and this PR is unnecessary.

That's not necessarily true. I install the gcc package but not the c-compiler package because I don't want the activation environment variable setting a bunch of compiler options automatically for me in my conda environment (as opposed to a conda build). CONDA_BUILD_SYSROOT is then not defined.

jakirkham commented 2 years ago

Unclear. We were having issues with this and CMake. Hence the exploration here

jakirkham commented 2 years ago

FWIW a few people have tested this locally and it seems to resolve the issue. After reviews are addressed, planning on getting this in. Though please let us know ASAP if there are issues with that

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

galipremsagar commented 2 years ago

@conda-forge-admin, please re-render

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

galipremsagar commented 2 years ago

@conda-forge-admin, please re-render

github-actions[bot] commented 2 years ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/nvcc-feedstock/actions/runs/2396819194.

galipremsagar commented 2 years ago

Completed testing this fix locally and verified it works. @jakirkham | @kkraus14 could either one of you review when you have time?

galipremsagar commented 2 years ago

@conda-forge-admin, please re-render

leofang commented 2 years ago

LGTM but I'll let John/Keith/Isuru merge 👍

galipremsagar commented 2 years ago

@conda-forge-admin, please re-render

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

conda-forge-linter commented 2 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

jakirkham commented 2 years ago

@kkraus14 and @isuruf, could you please take another look? It is worth noting the changes here are meant for use outside conda-forge. In particular this fixes some issues users ran into locally and also recently on RAPIDS CI. Am eager to get this in to fix those issues.

jakirkham commented 2 years ago

@conda-forge-admin, please re-render

jakirkham commented 2 years ago

Thanks all! 🙏

Looks like we resolved the remaining comments. So went ahead and merged. If it turns out we missed something, happy to follow up in a new issue/PR 🙂