Add host prefix to NVCC_PREPEND_FLAGS when using conda-build

conda-forge / cuda-nvcc-feedstock

A conda-smithy repository for cuda-nvcc.

BSD 3-Clause "New" or "Revised" License

1 stars 11 forks source link

Add host prefix to NVCC_PREPEND_FLAGS when using conda-build #19

Closed carterbox closed 1 year ago

carterbox commented 1 year ago

Adds the host requirements prefix include-paths and linking-search-paths to the NVCC_PREPEND_FLAGS for conda-build (because those two prefixes are separate) by using an activation script only when conda-build is active.

Outside of the conda-build context, these paths are not needed because nvcc would share a prefix with all the cuda libs.

I didn't do anything for Windows because it's skipped.

Checklist

[x] Used a personal fork of the feedstock to propose changes
[x] Bumped the build number (if the version is unchanged)
[x] Reset the build number to 0 (if the version changed)
[x] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[ ] Ensured the license file is being packaged.

Closes #18 Closes https://github.com/conda-forge/libcublas-feedstock/issues/11 ~~Requires https://github.com/conda-forge/cuda-nvcc-impl-feedstock/pull/2~~

conda-forge-webservices[bot] commented 1 year 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.

carterbox commented 1 year ago

@conda-forge-admin, please rerender

jakirkham commented 1 year ago

Thanks Daniel! 🙏

Had a question above 🙂

isuruf commented 1 year ago

I don't think we should introduce new env variables into the mix. Better to use NVCC_PREPEND_FLAGS.

carterbox commented 1 year ago

I tested this on a local build of the magma-feedstock. It seems to work, but silently. i.e. The args don't appear in the build logs.

jakirkham commented 1 year ago

The args don't appear in the build logs.

Maybe we should considering adding set -x at the top and set +x at the end (since it is sourced)?

carterbox commented 1 year ago

This is the one, people. Let's get this patch out before the CUDA 12 migration is too far along. 😆

jakirkham commented 1 year ago

Thanks Daniel for the PR and everyone for reviewing! 🙏

Let's get this change out. We can follow up on anything else separately

robertmaynard commented 1 year ago

I don't think we should introduce new env variables into the mix. Better to use NVCC_PREPEND_FLAGS.

I wanted to use a different env variable and inject into nvcc.profile so that CMake would be able to use these include and link directories as they would be in the INCLUDES and LIBRARIES output of nvcc ( https://gitlab.kitware.com/cmake/cmake/-/issues/24915 ). By using NVCC_PREPEND_FLAGS those directories will not be considered by CMake as that is a user flag and not part of the compiler implicit information.