Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lemon-pants-chew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@platforma-open/milaboratories.runenv-python-3.12.10-rapids': minor
---

NVIDIA env vars
6 changes: 5 additions & 1 deletion python-3.12.10-rapids/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
"registry": "platforma-open",
"python-version": "3.12.10",
"envVars": [
"RPY2_CFFI_MODE=ABI"
"RPY2_CFFI_MODE=ABI",
"NVIDIA_VISIBLE_DEVICES=all",
"NVIDIA_DRIVER_CAPABILITIES=compute,utility",
"LD_LIBRARY_PATH=/usr/local/nvidia/lib64:/usr/local/nvidia/lib",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 LD_LIBRARY_PATH silently overwrites any pre-existing value

Unlike PATH, which at least attempts to incorporate the existing value via ${PATH}, LD_LIBRARY_PATH is set to a fixed string. If the host container (or any previously applied runenv layer) already exports LD_LIBRARY_PATH entries (e.g. CUDA toolkit, cuDNN), those entries will be silently dropped, potentially causing runtime linker failures for libraries that depend on them. Using the same ${LD_LIBRARY_PATH} pattern keeps existing paths intact — assuming the runtime supports interpolation.

Suggested change
"LD_LIBRARY_PATH=/usr/local/nvidia/lib64:/usr/local/nvidia/lib",
"LD_LIBRARY_PATH=/usr/local/nvidia/lib64:/usr/local/nvidia/lib:${LD_LIBRARY_PATH}",
Prompt To Fix With AI
This is a comment left during a code review.
Path: python-3.12.10-rapids/package.json
Line: 28

Comment:
**`LD_LIBRARY_PATH` silently overwrites any pre-existing value**

Unlike `PATH`, which at least attempts to incorporate the existing value via `${PATH}`, `LD_LIBRARY_PATH` is set to a fixed string. If the host container (or any previously applied runenv layer) already exports `LD_LIBRARY_PATH` entries (e.g. CUDA toolkit, cuDNN), those entries will be silently dropped, potentially causing runtime linker failures for libraries that depend on them. Using the same `${LD_LIBRARY_PATH}` pattern keeps existing paths intact — assuming the runtime supports interpolation.

```suggestion
              "LD_LIBRARY_PATH=/usr/local/nvidia/lib64:/usr/local/nvidia/lib:${LD_LIBRARY_PATH}",
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

"PATH=/usr/local/nvidia/bin:${PATH}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using ${PATH} in a static JSON configuration file like package.json will likely not be expanded by the platform's runtime executor, resulting in a literal ${PATH} string in the environment variable. This will break standard executable lookups because the original path directories will be lost. If the platform runner does not support shell-style variable expansion for envVars, consider if this prepending is necessary or if the platform provides an alternative mechanism to append/prepend to the PATH variable.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 ${PATH} expansion not guaranteed in JSON envVars

Every other envVars entry in this repo is a plain KEY=VALUE literal with no variable references. If the Platforma runtime sets these strings directly (e.g. via setenv) rather than passing them through a shell, PATH will be set to the literal string /usr/local/nvidia/bin:${PATH} — with the unexpanded text ${PATH} as the suffix. That would break resolution of every binary (including python itself) that lives outside /usr/local/nvidia/bin, making the entire run environment non-functional. Please confirm that the runtime performs ${...} interpolation before applying envVars, or replace this entry with a fully resolved static path.

Prompt To Fix With AI
This is a comment left during a code review.
Path: python-3.12.10-rapids/package.json
Line: 29

Comment:
**`${PATH}` expansion not guaranteed in JSON `envVars`**

Every other `envVars` entry in this repo is a plain `KEY=VALUE` literal with no variable references. If the Platforma runtime sets these strings directly (e.g. via `setenv`) rather than passing them through a shell, `PATH` will be set to the literal string `/usr/local/nvidia/bin:${PATH}` — with the unexpanded text `${PATH}` as the suffix. That would break resolution of every binary (including `python` itself) that lives outside `/usr/local/nvidia/bin`, making the entire run environment non-functional. Please confirm that the runtime performs `${...}` interpolation before applying `envVars`, or replace this entry with a fully resolved static path.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

Comment on lines +28 to +29

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 NVIDIA runtime vars applied to all platforms including macOS and Windows

NVIDIA_VISIBLE_DEVICES, NVIDIA_DRIVER_CAPABILITIES, and the /usr/local/nvidia/... paths are NVIDIA Container Runtime constructs that are only meaningful in Linux containers with GPU passthrough. The roots in this package include macosx-x64, macosx-aarch64, and windows-x64. On those platforms the paths won't exist and LD_LIBRARY_PATH will point to directories that are absent, potentially interfering with any loader that respects LD_LIBRARY_PATH on macOS (DYLD_LIBRARY_PATH is the macOS equivalent). If RAPIDS GPU features are explicitly unsupported on non-Linux platforms, consider conditionally applying these vars only for Linux targets (if the runtime supports platform-specific env vars), or document that the NVIDIA vars are no-ops on non-Linux platforms.

Prompt To Fix With AI
This is a comment left during a code review.
Path: python-3.12.10-rapids/package.json
Line: 28-29

Comment:
**NVIDIA runtime vars applied to all platforms including macOS and Windows**

`NVIDIA_VISIBLE_DEVICES`, `NVIDIA_DRIVER_CAPABILITIES`, and the `/usr/local/nvidia/...` paths are NVIDIA Container Runtime constructs that are only meaningful in Linux containers with GPU passthrough. The `roots` in this package include `macosx-x64`, `macosx-aarch64`, and `windows-x64`. On those platforms the paths won't exist and `LD_LIBRARY_PATH` will point to directories that are absent, potentially interfering with any loader that respects `LD_LIBRARY_PATH` on macOS (`DYLD_LIBRARY_PATH` is the macOS equivalent). If RAPIDS GPU features are explicitly unsupported on non-Linux platforms, consider conditionally applying these vars only for Linux targets (if the runtime supports platform-specific env vars), or document that the NVIDIA vars are no-ops on non-Linux platforms.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

],
"roots": {
"linux-x64": "./pydist/linux-x64",
Expand Down
Loading