Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First step of dynamically updating the version from cmake #2610

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

AndrewQuijano
Copy link
Contributor

@AndrewQuijano AndrewQuijano commented Jan 19, 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
Update Capstone such that Python binding versioning can be obtained from the CMake file, this would save time to just pass the version into CMake rather than manually update the version via commit.

Essentially, can we automate this step done for each release?
5526125
...

Test plan

1- Confirm that creating the Debian Package works as intended
2- Confirm that version.h and pkgconfig.mk file are correctly filled out. Not sure what exactly to check with the Python bindings though

...

Closing issues

closes #2609
...

@github-actions github-actions bot added the CS-core-files auto-sync label Jan 19, 2025
@AndrewQuijano
Copy link
Contributor Author

AndrewQuijano commented Jan 19, 2025

Showing how the current versioning works, but I will need help about the bindings/python/capstone/__init__.py @Rot127 I think the easiest thing is, since setup.py is reading from pkgconfig.mk file, remove the code duplication somehow?

image

Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

I am all for centralizing the version info!

But I think we can't get rid of some hard-coded version numbers.
Not all commits have a tag assigned. If you build on next without having a tag, it fails to set the macros, right?

The version number macros need to be set. Because people use them to distinguish between pre-v5, v5, v6 etc. So they can build their project with different Capstone versions.

CMakeLists.txt Outdated Show resolved Hide resolved
include/capstone/capstone.h Outdated Show resolved Hide resolved
include/capstone/version.h Outdated Show resolved Hide resolved
@AndrewQuijano AndrewQuijano force-pushed the version branch 7 times, most recently from ae122aa to c2fcace Compare January 21, 2025 15:11
@AndrewQuijano
Copy link
Contributor Author

@Rot127 I commented out the cmake to configure pkgconfig.mk, this will only dynamically the version information for the capstone.h file. Figure I can keep the template in the meantime for your use when you work on getting the bindings working.

@Rot127
Copy link
Collaborator

Rot127 commented Jan 22, 2025

Thanks! I will review this one after Lunar New year. (So hopefully after the Alpha3 patch release).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CS-core-files auto-sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamically Update Python Version
2 participants