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

fix python bindings shared library directory issue #301

Merged
merged 1 commit into from Oct 24, 2017
Merged

fix python bindings shared library directory issue #301

merged 1 commit into from Oct 24, 2017

Conversation

hongxuchen
Copy link
Contributor

using data_files dict prefixed with SITE_PACKAGES = os.path.join(get_python_lib(), "keystone") is not portable. Regular setup configuration does not work, e.g., specifying package_data etc, since that only affects sdist.

The fix is a hack: customize install_lib to explicitly copy the file into keystone install dir.

This should fix #235 and #297 .

using `data_files` dict prefixed with
`SITE_PACKAGES = os.path.join(get_python_lib(), "keystone")` is not
portable, see #235 and #297 for details. Regular setup configuration does
not work, e.g., specifying `package_data` etc, since that only affects
`sdist`.

The fix is a hack: customize `install_lib` to explicitly copy the file
into keystone install dir.
@hongxuchen
Copy link
Contributor Author

Also a bit os.chdir(cur_dir) to ensure after cmake build it will return to root dir even there are exceptions (merely used for local setup try).

@aquynh
Copy link
Member

aquynh commented Jun 6, 2017

cool, did you look at the setup code of Unicorn for reference?

@hongxuchen
Copy link
Contributor Author

@aquynh I didn't. I was just trying keystone and met the issue on my machine and didn't read your comment #297 carefully. Unicorn seems different as it also needs to include headers, etc?

@aquynh
Copy link
Member

aquynh commented Jun 13, 2017

the difference is not big. or you can look at the setup of Capstone, which works & proven.

by porting the setup.py of Capstone or Unicorn over, we can fix other minor issues. please let me know if you can work on this, thanks.

@hongxuchen
Copy link
Contributor Author

Okay, I will take a look at that and hopefully there is a general and consistent solution.

@hongxuchen
Copy link
Contributor Author

Hi @aquynh , I modified both capstone and unicorn's setup files, and I think it is now consistent. It supports:

  • sdist
  • bdist (with wheel if possible)

lib and include are put inside the installed root directories respectively.

keystone's setup should be modified to fit for the requirement.

Please check whether you're comfortable with these changes.

setups.zip

@williballenthin
Copy link

@aquynh whats the status of this PR? will it be merged soon, or are you awaiting additional fixes?

@aquynh aquynh merged commit 5bcff11 into keystone-engine:master Oct 24, 2017
@aquynh
Copy link
Member

aquynh commented Oct 24, 2017

merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python bindings - fail to load the dynamic library.
3 participants