fix(python): only bundle the DiskANN plugin where it is supported#510
Open
Cuiyus wants to merge 2 commits into
Open
fix(python): only bundle the DiskANN plugin where it is supported#510Cuiyus wants to merge 2 commits into
Cuiyus wants to merge 2 commits into
Conversation
Cuiyus
force-pushed
the
fix/wheel-diskann-only-when-supported
branch
from
3d4f931 to
f15e692
Compare
The wheel install rule for the DiskANN plugin was gated on `if(TARGET core_knn_diskann)`. That target always exists: on platforms where DiskAnn is unsupported (macOS, ARM64, ...) it is built from an empty stub (src/core/algorithm/CMakeLists.txt) with zero exported symbols, and the runtime load path is compiled out via `#if DISKANN_SUPPORTED`. So the condition was always true and a useless ~16 KB stub `.dylib` was shipped in every non-Linux-x86_64 wheel. Gate the install on `DISKANN_SUPPORTED` instead, so the plugin is packaged only where it is real (currently Linux x86_64 with libaio). Verified on macOS arm64: `libcore_knn_diskann.dylib` no longer appears in the wheel; `import zvec` and other index types are unaffected.
Cuiyus
force-pushed
the
fix/wheel-diskann-only-when-supported
branch
from
f15e692 to
a6588ee
Compare
`wheel.packages = ["python/zvec"]` copies the directory into the wheel verbatim, so a stray `_zvec*.so` / `*.dylib` / `*.pyd` left in the source tree gets bundled — e.g. a stale binary with the wrong macOS deployment target, which then fails `delocate` during wheel repair. Ignore these build artifacts so they can never be committed or bundled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Non-Linux-x86_64 wheels (e.g. macOS arm64) ship a useless
libcore_knn_diskann.dylib— a ~16 KB shared library with zero exported symbols.DiskAnn is only buildable on Linux x86_64 (it needs libaio). On every other platform
src/core/algorithm/CMakeLists.txtstill defines thecore_knn_diskanntarget, but builds it from an empty stub (diskann_stub.cc). The runtime load path that woulddlopenit is also compiled out via#if DISKANN_SUPPORTED(src/core/interface/index_factory.cc), so the stub is never loaded.The wheel install rule gated on
if(TARGET core_knn_diskann), which is always true (the stub target exists everywhere), so the dead stub got packaged into every wheel.Fix
Gate the install on
DISKANN_SUPPORTEDinstead of the target's existence:The plugin is now packaged only where it is real (Linux x86_64 with libaio). On Linux x86_64 the behaviour is unchanged.
Verification (macOS arm64)
libcore_knn_diskann.dylibno longer present in the wheel (nm -gUon the old stub confirmed 0 exported symbols)import zvecworks; HNSW/IVF/Flat/Vamana index types unaffected