Skip to content

Fix sdist build and add CI job for testing it.#2622

Closed
Rot127 wants to merge 2 commits intocapstone-engine:nextfrom
Rot127:cmake-cpack-build-fix
Closed

Fix sdist build and add CI job for testing it.#2622
Rot127 wants to merge 2 commits intocapstone-engine:nextfrom
Rot127:cmake-cpack-build-fix

Conversation

@Rot127
Copy link
Copy Markdown
Collaborator

@Rot127 Rot127 commented Feb 6, 2025

Your checklist for this pull request

  • I've documented or updated the documentation of every API function and struct this PR changes.
  • I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

Addresses #2621 on the v6 branch.
The sdist build was not tested before. Hence we didn't detect issues with it.

This fixes the build (missing files in sdist archive) and adds a new test for it as job.
It also adds a CAPSTONE_BUILD_FUZZER flag. Because the sdist doesn't include any testing tools. So the fuzzer source should be excluded.

Test plan

Added

Closing issues

...

@github-actions github-actions Bot added Python Bindings Documentation Github-files Github related files labels Feb 6, 2025
@Rot127 Rot127 force-pushed the cmake-cpack-build-fix branch 2 times, most recently from 8a88a38 to bb1ef07 Compare February 6, 2025 11:18
@Rot127 Rot127 force-pushed the cmake-cpack-build-fix branch from bb1ef07 to c774ad7 Compare February 6, 2025 11:45
@Rot127 Rot127 force-pushed the cmake-cpack-build-fix branch from a279539 to ec3aa8a Compare February 6, 2025 12:20
@@ -264,6 +272,14 @@ jobs:
name: sdist-archive
path: bindings/python/dist/*.tar.gz
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It should be enough to add the following line:

python3 -m pip install dist/*.tar.gz

instead of the section added below

- name: '🛠️ Set up QEMU'
if: runner.os == 'Linux' && matrix.arch != 'x86_64'
uses: docker/setup-qemu-action@v3
with:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMHO it's better doing this change in a separate PR

@Rot127
Copy link
Copy Markdown
Collaborator Author

Rot127 commented Feb 12, 2025

@Antelox As discussed given to you,

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

Labels

Documentation Github-files Github related files Python Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants