Use pip package of nanovdb-editor#580
Conversation
Signed-off-by: Petra Hapalova <phapalova@nvidia.com>
There was a problem hiding this comment.
Pull request overview
This PR shifts fVDB’s NanoVDB Editor integration to use CPM-pinned headers at build time while relying on the nanovdb-editor PyPI package for runtime binaries by default, with an opt-in path to build/install the editor from a local source checkout.
Changes:
- Make
nanovdb-editora required Python dependency (no longer an optional extra). - Refactor the CMake NanoVDB Editor fetch/install flow to pin headers via CPM and only build from source when
CPM_nanovdb_editor_SOURCEis set. - Update
build.shand docs to remove the oldeditor_skip/editor_forcemodifiers and document the new local-source override mechanism.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/tests/CMakeLists.txt |
Removes the NANOVDB_EDITOR_SKIP gating block and leaves the viewer test commented out. |
src/cmake/get_nanovdb_editor.cmake |
Pins editor headers via CPM; detects installed pip package for runtime binaries; builds/install from source only when CPM source override is set. |
README.md |
Documents the new default behavior and local-source override flag. |
pyproject.toml |
Promotes nanovdb-editor from optional extra to required dependency. |
docs/installation.rst |
Updates the source installation command (but still contains outdated viewer-extra guidance elsewhere). |
build.sh |
Removes editor_* modifiers; installs nanovdb-editor via pip on install builds unless a local editor source override is used. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Petra Hapalova <phapalova@nvidia.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
build.sh:363
- PASS_THROUGH_ARGS is built using
printf %qand then later expanded unquoted in thepip ... $PIP_ARGSinvocations.%qescaping does not survive bash word-splitting (e.g., paths with spaces will still be split), so-C cmake.define.CPM_nanovdb_editor_SOURCE=/path with spaceswill be parsed incorrectly despite the “handling potential spaces safely” comment. Consider switching to a bash array for pip arguments (accumulate tokens into an array and invoke pip with"${args[@]}") instead of a space-delimited string with%qescaping.
case "$1" in
-C)
shift
if [ "$#" -eq 0 ] || [[ "$1" == -* ]]; then
echo "Error: -C requires a value argument." >&2
exit 1
fi
record_nanovdb_editor_source_override "$1"
PASS_THROUGH_ARGS+=" -C $(printf "%q" "$1")"
is_config_arg_handled=true
;;
-C*)
record_nanovdb_editor_source_override "${1#-C}"
PASS_THROUGH_ARGS+=" $(printf "%q" "$1")"
is_config_arg_handled=true
;;
--config-settings=*)
record_nanovdb_editor_source_override "${1#--config-settings=}"
PASS_THROUGH_ARGS+=" $(printf "%q" "$1")"
is_config_arg_handled=true
;;
--cuda-arch-list=*)
CUDA_ARCH_LIST_ARG="${1#*=}"
is_config_arg_handled=true
;;
--cuda-arch-list)
shift
CUDA_ARCH_LIST_ARG="$1"
is_config_arg_handled=true
;;
*)
# Append other arguments, handling potential spaces safely
PASS_THROUGH_ARGS+=" $(printf "%q" "$1")"
;;
esac
fi
shift
done
NINJA_PATH=$(command -v ninja 2>/dev/null || command -v ninja-build 2>/dev/null)
if [ -n "$NINJA_PATH" ]; then
CONFIG_SETTINGS+=" --config-settings=cmake.define.CMAKE_MAKE_PROGRAM=$NINJA_PATH"
fi
# Construct PIP_ARGS with potential CMake args and other pass-through args
export PIP_ARGS="--no-build-isolation$CONFIG_SETTINGS$PASS_THROUGH_ARGS"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Petra Hapalova <phapalova@nvidia.com>
Signed-off-by: Petra Hapalova <phapalova@nvidia.com>
Prerequisite to pass tests in #580 --------- Signed-off-by: Petra Hapalova <phapalova@nvidia.com>
…deps Signed-off-by: Petra Hapalova <phapalova@nvidia.com>
cmake.define.CPM_nanovdb_editor_SOURCE