Skip to content

fix(packaging): sdist 排除运行时二进制#447

Merged
qin-ctx merged 1 commit intomainfrom
remove_binary
Mar 5, 2026
Merged

fix(packaging): sdist 排除运行时二进制#447
qin-ctx merged 1 commit intomainfrom
remove_binary

Conversation

@zhoujh01
Copy link
Collaborator

@zhoujh01 zhoujh01 commented Mar 5, 2026

  • MANIFEST.in 中 prune openviking/bin、openviking/lib,并排除 .so/.dylib/.dll/.exe
  • sdist 构建前在 CI 定点清理可能残留的本地二进制

Description

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

- MANIFEST.in 中 prune openviking/bin、openviking/lib,并排除 *.so/*.dylib/*.dll/*.exe
- sdist 构建前在 CI 定点清理可能残留的本地二进制
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Coverage Check

Verify that the added prune and recursive-exclude rules in MANIFEST.in fully prevent inclusion of the intended runtime binaries in the sdist.

# sdist should be source-only: never ship runtime binaries from working tree
prune openviking/bin
prune openviking/lib
recursive-exclude openviking *.so *.dylib *.dll *.exe
CI Cleanup Consistency

Ensure the CI cleanup steps match the MANIFEST.in exclusions and cover all potential locations of local runtime binaries that might leak into the sdist.

# For sdist, ensure local runtime binaries are not packaged even if present
rm -rf openviking/bin openviking/lib third_party/agfs/bin || true
rm -f openviking/storage/vectordb/*.so openviking/storage/vectordb/*.dylib openviking/storage/vectordb/*.dll openviking/storage/vectordb/*.exe || true

@github-actions
Copy link

github-actions bot commented Mar 5, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use global-exclude for binaries

Use global-exclude instead of recursive-exclude openviking to cover binary
extensions across all directories, ensuring no runtime binaries are accidentally
included in the sdist from any location.

MANIFEST.in [18]

-recursive-exclude openviking *.so *.dylib *.dll *.exe
+global-exclude *.so *.dylib *.dll *.exe
Suggestion importance[1-10]: 7

__

Why: Replaces directory-specific recursive-exclude with global-exclude to cover binaries in all directories, reducing accidental inclusion of runtime binaries in sdist.

Medium
Add prune for third_party binaries

Add prune third_party/agfs/bin to align the sdist exclusion logic with the CI
cleanup step. This ensures the directory is excluded from sdist even when built
locally without the CI workflow.

MANIFEST.in [15-18]

 # sdist should be source-only: never ship runtime binaries from working tree
 prune openviking/bin
 prune openviking/lib
+prune third_party/agfs/bin
 recursive-exclude openviking *.so *.dylib *.dll *.exe
Suggestion importance[1-10]: 6

__

Why: Aligns sdist exclusion logic with CI cleanup by adding prune third_party/agfs/bin, ensuring consistent source-only sdist builds locally and in CI.

Low

@qin-ctx qin-ctx merged commit 1f1df6e into main Mar 5, 2026
22 of 23 checks passed
@qin-ctx qin-ctx deleted the remove_binary branch March 5, 2026 12:20
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Mar 5, 2026
@qin-ctx
Copy link
Collaborator

qin-ctx commented Mar 5, 2026

/describe

@github-actions
Copy link

github-actions bot commented Mar 5, 2026

Preparing PR description...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants