NEW: Add cf-nvidia-tools #28794

carterbox · 2025-01-08T20:51:19Z

Checklist

github-actions · 2025-01-08T20:52:40Z

Hi! This is the staged-recipes linter and your PR looks excellent! 🚀

conda-forge-admin · 2025-01-08T20:52:57Z

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

I wanted to let you know that I linted all conda-recipes in your PR (recipes/cf-nvidia-tools/recipe.yaml) and found some lint.

Here's what I've got...

For recipes/cf-nvidia-tools/recipe.yaml:

❌ noarch packages can't have skips with selectors. If the selectors are necessary, please remove noarch: generic.

_{This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12794723558. Examine the logs at this URL for more detail.}

carterbox · 2025-01-08T21:06:42Z

@conda-forge/cuda, please review

jakirkham

Thanks Daniel! 🙏

Really appreciate you putting the time into creating this testing resource. This will be very helpful when testing packages and catching any missed changes.

Had a few questions below

recipes/cf-nvidia-tools/recipe.yaml

jakirkham · 2025-01-14T20:14:17Z

recipes/cf-nvidia-tools/recipe.yaml

+tests:
+  - script:
+    - export c_stdlib_version="2.17.0"
+    - check-glibc ${CONDA_PREFIX}/lib/*.so.*


Suggested change

- check-glibc ${CONDA_PREFIX}/lib/*.so.*

- check-glibc "${CONDA_PREFIX}/lib/*.so.*"

Also would it make sense to have a default search argument or would we rather be explicit like in this test

Lastly should we add a library test dependency to ensure there is one library we can check?

I don't want to provide a default argument because in production, we don't want to test all of the binaries in PREFIX, just the NVIDIA redists. There is no glob that I can think of that satisfies this condition.

No double quotes because we want the terminal to expand the glob expression not pass a literal glob expression.

jakirkham · 2025-01-14T20:17:11Z

recipes/cf-nvidia-tools/recipe.yaml

+  description: >
+    This package contains CLI tools for validating and linting NVIDIA's conda recipes on
+    conda-forge. For a description of the tools see the README in the package feedstock. The
+    tools are hosted directly in the feedstock; there is no external source code repository
+    for these tools at this time.


Do we want this to be one long string or do want to have it wrapped a certain way?

FWIW find this website to be a useful resource when playing with YAML string formatting: https://yaml-multiline.info

I want one long string. anaconda.org handles the wrapping of long lines.

recipes/cf-nvidia-tools/recipe.yaml

carterbox · 2025-01-15T17:22:39Z

@conda-forge/staged-recipes, please review

github-actions · 2025-01-15T17:22:52Z

To help direct your pull request to the best reviewers, please mention a topic-specifc team if your recipe matches any of the following: conda-forge/help-c-cpp, conda-forge/help-cdts, conda-forge/help-go, conda-forge/help-java, conda-forge/help-julia, conda-forge/help-nodejs, conda-forge/help-perl, conda-forge/help-python, conda-forge/help-python-c, conda-forge/help-r, conda-forge/help-ruby,or conda-forge/help-rust. Thanks!

billysuh7 · 2025-01-15T17:24:03Z

recipes/cf-nvidia-tools/bin/check-glibc

+      )
+      if [[ "${first}" == "${second}" ]] ; then
+        echo "true"
+        return 0


Is the return value of 1 or 0 ever used ? I found that when this file was used in cudnn feedstock, and when it returned 1 (incompatible), it would exit build.sh right away due to "set -e" in build.sh, without printing the message "The binary is not compatible with the recipe's glibc pinning". This does not happen here in this feedstock as it does not have "set -e" though.

This is a good point! You're saying that return causes the entire script to abort instead of just moving up one level. I will refactor to use the strings only.

I've stripped unused parts of the API and added a test to ensure that the script exits 1 when appropriate.

billysuh7 · 2025-01-15T17:27:54Z

recipes/cf-nvidia-tools/bin/check-glibc

+for file in "$@"; do
+    if [[ -f "$file" && ! -L "$file" ]]; then  # Ensure it's a file and not a link
+      BINARY_GLIBC_VERSION="$(glibc-detect req "$file")"
+      echo "binary glibc ${BINARY_GLIBC_VERSION} <= recipe glibc ${RECIPE_GLIBC_VERSION} <= system glibc ${SYSTEM_GLIBC_VERSION} $file"


Maybe I am missing something - is the 2nd half of the check i.e. "recipe glibc ${RECIPE_GLIBC_VERSION} <= system glibc ${SYSTEM_GLIBC_VERSION}" being run ? The next line, glibc-check compatible "$BINARY_GLIBC_VERSION" "$RECIPE_GLIBC_VERSION", compares binary glibc and recipe glibc only I think.

I think you are correct. We don't check if the system glibc version is >= the recipe version. This was important for debugging when developing this tool, but it doesn't actually affect the correctness of the recipe.

It is perfectly valid to build a package that requires glibc 5 on a system that only has glibc 2. It's just that you won't be able to run or install the binaries you have built because the package will require __glibc >= 5.

In summary, printing the system glibc version is printed for informational purposes and may help with debugging if tests which try to load the libraries fail during the test phase.

I could add a check and echo a warning message if system glibc is < recipe glibc, but definitely not raise an error.

Thanks for clarifying! Yeah I think it's better to give a warning if system glibc is < recipe glibc, or at least simply print SYSTEM_GLIBC_VERSION value on a separate line as dry information - right now it may give impression to the user that system glibc is being checked against recipe glibc version

billysuh7 · 2025-01-15T19:23:05Z

recipes/cf-nvidia-tools/bin/check-glibc

+    ## 1. detecting the glibc version
+    "detect" )
+      glibc-detect "${@}"
+    ;;
+    ## 2. checking a pair of glibc versions for compatibility
+    "check" )
+      glibc-check "${@}"
+    ;;


I don't believe these two modes are used. Are they for future extension ?

billysuh7 · 2025-01-15T19:28:43Z

recipes/cf-nvidia-tools/bin/check-glibc

+for file in "$@"; do
+    if [[ -f "$file" && ! -L "$file" ]]; then  # Ensure it's a file and not a link
+      BINARY_GLIBC_VERSION="$(glibc-detect req "$file")"
+      echo "binary glibc ${BINARY_GLIBC_VERSION} <= recipe glibc ${RECIPE_GLIBC_VERSION} <= system glibc ${SYSTEM_GLIBC_VERSION} $file"


Thanks for clarifying! Yeah I think it's better to give a warning if system glibc is < recipe glibc, or at least simply print SYSTEM_GLIBC_VERSION value on a separate line as dry information - right now it may give impression to the user that system glibc is being checked against recipe glibc version

NEW: Add cf-nvidia-tools

5536609

carterbox mentioned this pull request Jan 8, 2025

Create a package of NVIDIA specific test scripts to check our redists conda-forge/cuda-feedstock#60

Open

1 task

carterbox added 3 commits January 8, 2025 14:59

DOC: Update README text

54b8f67

DOC: Add homepage to metadata

8996032

BLD: Skip non-linux platforms

a882ad9

jakirkham reviewed Jan 14, 2025

View reviewed changes

carterbox added 6 commits January 14, 2025 16:36

TST: Add check of help message

f71de70

BLD: Run shellcheck on script

0767534

TST: Add a package to test environment for testing

05a8238

STY: Address lints from shellcheck

4963ad9

STY: Add double quotes around variables

8319ada

DOC: Use admonitions in docs to highlight common mistakes

5bf2a1d

carterbox requested a review from jakirkham January 14, 2025 23:05

github-actions bot added review-requested staged-recipes labels Jan 15, 2025

billysuh7 reviewed Jan 15, 2025

View reviewed changes

carterbox added 2 commits January 15, 2025 12:29

REF: Remove unused parts of API

9e4cbb9

TST: Add tests for negative result

cefc3e7

carterbox requested a review from billysuh7 January 15, 2025 18:34

billysuh7 reviewed Jan 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NEW: Add cf-nvidia-tools #28794

NEW: Add cf-nvidia-tools #28794

carterbox commented Jan 8, 2025 •

edited

Loading

github-actions bot commented Jan 8, 2025

conda-forge-admin commented Jan 8, 2025 •

edited

Loading

carterbox commented Jan 8, 2025

jakirkham left a comment

jakirkham Jan 14, 2025

carterbox Jan 14, 2025

carterbox Jan 14, 2025

jakirkham Jan 14, 2025

carterbox Jan 14, 2025

carterbox commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

billysuh7 Jan 15, 2025

carterbox Jan 15, 2025

carterbox Jan 15, 2025

billysuh7 Jan 15, 2025

carterbox Jan 15, 2025

billysuh7 Jan 15, 2025

billysuh7 Jan 15, 2025

billysuh7 Jan 15, 2025

	- check-glibc ${CONDA_PREFIX}/lib/.so.
	- check-glibc "${CONDA_PREFIX}/lib/.so."

NEW: Add cf-nvidia-tools #28794

Are you sure you want to change the base?

NEW: Add cf-nvidia-tools #28794

Conversation

carterbox commented Jan 8, 2025 • edited Loading

github-actions bot commented Jan 8, 2025

conda-forge-admin commented Jan 8, 2025 • edited Loading

carterbox commented Jan 8, 2025

jakirkham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carterbox commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carterbox commented Jan 8, 2025 •

edited

Loading

conda-forge-admin commented Jan 8, 2025 •

edited

Loading