-
-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Windows] Freeze PKG_VERSION and add fallback lookup in default installation path #52
Conversation
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 ( |
Previous changes in new PR conda-forge#52
@conda-forge/nvcc I think this is ready for review. If you agree, I'd like to add me as a maintainer for the Windows bits too. |
Sounds good to me, please add yourself as a maintainer on the recipe! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One typo, but approach LGTM
…k into fix-version-check
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 ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug. |
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 ( |
Thanks! |
Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)Part 1
This fixes a subtle bug I just found for the activation scripts on Windows. Since we are not creating the activation scripts from the install script, but by copying them from files, environment variables are not rendered to the final file. Instead the take the value set in the environment when they are executed. For our tests that didn't matter because they got rendered correctly, but when used in other recipes, they might take the wrong value.
For example, the CUDA version check uses
PKG_VERSION
, which will be set for other packages during build time. Example here. That7.5.0
is OpenMM's version, not CUDA!To fix this, I am rendering
PKG_VERSION
by hand, usingsed
😬This is not producing errors because of the fallback mechanisms for finding
CUDA_HOME
, but that doesn't mean it should not be fixed. Especially if this package is used outside the context of conda-forge.Part 2
For some reason, in some feedstocks
conda-forge-ci-setup
mechanisms do not reach the recipe (e.g.CUDA_PATH
is not set andPATH
does not containnvcc.exe
). See #53 for more details.As a workaround I am adding a fallback lookup in the default installation path, where the activation script might find nvcc.exe.