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

python bindings: Fully remove distutils and pkg_resources #2369

Open
wants to merge 7 commits into
base: next
Choose a base branch
from

Conversation

pyrox0
Copy link

@pyrox0 pyrox0 commented May 25, 2024

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

Distutils has been fully removed from the setup.py script. This was done because Python 3.12 removes this package, so this ensures that this will continue to function in the future. The code only minimally changes, as the actual APIs are implemented in other parts of the standard library, so this just replaces the distutils functions with appropriate functions from elsewhere.

I've also replaced the deprecated pkg_resources with importlib.resources, as in its own documentation, it's mentioned that pkg_resources is a deprecated API, and projects using it should migrate to importlib-based solutions, which I've done. The code changes slightly, but it's all based on the migration guide provided by importlib.
...

Test plan

N/A
...

Closing issues

...

@pyrox0 pyrox0 mentioned this pull request May 25, 2024
13 tasks
@XVilka
Copy link
Contributor

XVilka commented May 26, 2024

Related issues:

@XVilka
Copy link
Contributor

XVilka commented May 26, 2024

    + mkdir -p /tmp/cibuildwheel/built_wheel
    + python -m pip wheel /project/bindings/python --wheel-dir=/tmp/cibuildwheel/built_wheel --no-deps
Processing ./bindings/python
  Preparing metadata (setup.py): started
  ERROR: Command errored out with exit status 1:
   command: /opt/python/cp36-cp36m/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/project/bindings/python/setup.py'"'"'; __file__='"'"'/project/bindings/python/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-av1hzh_2
       cwd: /project/bindings/python/
  Complete output (5 lines):
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File "/project/bindings/python/setup.py", line 12, in <module>
      from setuptools.command.build import build
  ModuleNotFoundError: No module named 'setuptools.command.build'
  ----------------------------------------
  Preparing metadata (setup.py): finished with status 'error'
WARNING: Discarding file:///project/bindings/python. Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.
ERROR: Command errored out with exit status 1: python setup.py egg_info Check the logs for full command output.

@Rot127 Rot127 added the python bindings label May 26, 2024
@XVilka
Copy link
Contributor

XVilka commented Jun 10, 2024

@pyrox0 please rebase this PR on top of the latest next.

@pyrox0
Copy link
Author

pyrox0 commented Jun 11, 2024

@pyrox0 please rebase this PR on top of the latest next.

Rebased. I've also cherry-picked the commits from #2377, as they are important to this. Apologies for the slow response from my end.

@Rot127
Copy link
Collaborator

Rot127 commented Jun 12, 2024

@kabeor and @aquynh This seems to be the Python 3.12 with 3.6 conflict, which was already discussed in the community channel. Could you please give your opinion on it?

cd .. && python3 const_generator.py python
Generating bindings for python
Generating bindings for python
Check test_basic.py ...
Traceback (most recent call last):
  File "./test_basic.py", line 4, in <module>
    from capstone import *
  File "/home/runner/work/capstone/capstone/bindings/python/capstone/__init__.py", line 388, in <module>
    from importlib import resources
ImportError: cannot import name 'resources'
FAILED
make: *** [Makefile:51: check] Error 1
Error: Process completed with exit code 2.

@pyrox0 Could you check if this is also related to Python3.6 incompatibility?

[100%] Built target cstool
+ cd /src/capstonenext/bindings/python
+ sed -i -e s/#print/print/ capstone/__init__.py
+ export CFLAGS=
+ CFLAGS=
+ export AFL_NOOPT=1
+ AFL_NOOPT=1
+ python setup.py install
Traceback (most recent call last):
  File "setup.py", line 227, in <module>
    long_description=open('README.txt', encoding="utf8").read(),
TypeError: 'encoding' is an invalid keyword argument for this function

@aquynh
Copy link
Collaborator

aquynh commented Jun 13, 2024

I think for Python3 we can just support the modern versions, and ignore the EOLF ones.

@Rot127
Copy link
Collaborator

Rot127 commented Jun 15, 2024

@pyrox0 Then feel free and go ahead with removing Python 3.6.

@Rot127
Copy link
Collaborator

Rot127 commented Jun 17, 2024

Python 3.9 still complaints:

Traceback (most recent call last):
  File "/home/runner/work/capstone/capstone/bindings/python/setup.py", line 12, in <module>
    from setuptools.command.build import build
ModuleNotFoundError: No module named 'setuptools.command.build'
make: *** [Makefile:10: install] Error 1

twizmwazin and others added 4 commits June 17, 2024 12:25
Removes distutils from the setup.py script for the python bindings, as
it is removed in Python 3.12 onwards.
Since support is removed per maintainer request, this should be removed
from the test suite as well.
Needed to fix a setuptools error on python 3.9
@Rot127
Copy link
Collaborator

Rot127 commented Jun 19, 2024

Python 3.11

capstone.__pycache__.xcore.cpython-311: module references __file__
capstone.__pycache__.xcore_const.cpython-311: module references __file__
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/capstone-5.0.0.post1-py3.11.egg/capstone/__init__.py", line 433, in <module>
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/capstone-5.0.0.post1-py3.11.egg/capstone/__init__.py", line 404, in _load_lib
  File "<frozen posixpath>", line 76, in join
TypeError: expected str, bytes or os.PathLike object, not Path
Error: Process completed with exit code 1.

Python 3.9:

 capstone.__pycache__.x86.cpython-39: module references __file__
capstone.__pycache__.x86_const.cpython-39: module references __file__
capstone.__pycache__.xcore.cpython-39: module references __file__
capstone.__pycache__.xcore_const.cpython-39: module references __file__
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 664, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 627, in _load_backward_compatible
  File "<frozen zipimport>", line 259, in load_module
  File "/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/capstone-5.0.0.post1-py3.9.egg/capstone/__init__.py", line 433, in <module>
  File "/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/capstone-5.0.0.post1-py3.9.egg/capstone/__init__.py", line 404, in _load_lib
  File "/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/posixpath.py", line 76, in join
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not Path
Error: Process completed with exit code 1.

and CSFuzz:

2024-06-18T07:27:14.8753152Z [ 98%] Building C object CMakeFiles/cstool.dir/cstool/cstool_x86.c.o
2024-06-18T07:27:15.0051122Z [ 99%] Building C object CMakeFiles/cstool.dir/cstool/cstool_xcore.c.o
2024-06-18T07:27:15.0697505Z [ 99%] Building C object CMakeFiles/cstool.dir/cstool/getopt.c.o
2024-06-18T07:27:15.1075197Z [100%] Linking C executable cstool
2024-06-18T07:27:15.5716526Z [100%] Built target cstool
2024-06-18T07:27:15.5775941Z + cd /src/capstonenext/bindings/python
2024-06-18T07:27:15.5776653Z + sed -i -e s/#print/print/ capstone/__init__.py
2024-06-18T07:27:15.5797113Z + export CFLAGS=
2024-06-18T07:27:15.5798278Z + CFLAGS=
2024-06-18T07:27:15.5798642Z + export AFL_NOOPT=1
2024-06-18T07:27:15.5799033Z + AFL_NOOPT=1
2024-06-18T07:27:15.5799427Z + python setup.py install
2024-06-18T07:27:15.6716267Z Traceback (most recent call last):
2024-06-18T07:27:15.6717069Z   File "setup.py", line 12, in <module>
2024-06-18T07:27:15.6718169Z     from setuptools.command.build import build
2024-06-18T07:27:15.6718935Z ImportError: No module named build
2024-06-18T07:27:19.0363408Z 2024-06-18 07:27:19,035 - root - ERROR - Building fuzzers failed.
2024-06-18T07:27:19.0364526Z 2024-06-18 07:27:19,035 - root - ERROR - Error building fuzzers for (commit: 3cdfa49cdaa809a01738ed688682dba7093cb25e, pr_ref: refs/pull/2369/merge).
2024-06-18T07:27:19.2356558Z ##[group]Run actions/upload-artifact@v3
2024-06-18T07:27:19.2356919Z with:
2024-06-18T07:27:19.2357130Z   name: artifacts
2024-06-18T07:27:19.2357369Z   path: ./out/artifacts
2024-06-18T07:27:19.2357645Z   if-no-files-found: warn
2024-06-18T07:27:19.2357905Z ##[endgroup]
2024-06-18T07:27:19.4366103Z ##[warning]No files were found with the provided path: ./out/artifacts. No artifacts will be uploaded.
2024-06-18T07:27:19.4545161Z Cleaning up orphan processes

Sorry, I know these bug classes are super annoying. I really appreciate you take care of this part of the Python bindings!

Copy link
Contributor

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

See bindings/python/capstone/__init__.py:434 (_load_lib call):

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/capstone-5.0.0.post1-py3.11.egg/capstone/__init__.py", line 434, in <module>
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/capstone-5.0.0.post1-py3.11.egg/capstone/__init__.py", line 405, in _load_lib
  File "<frozen posixpath>", line 76, in join
TypeError: expected str, bytes or os.PathLike object, not Path

@Rot127
Copy link
Collaborator

Rot127 commented Jun 24, 2024

@pyrox0 @twizmwazin We just reverted #2391, because it broke the CI fuzz job and I wanted to have it work for the bigger PRs for now (Xtensa, LoongArch, AArch64).

#2378 was unfortunately incomplete, as it turned out. Python2 is used in test_corpus.py which in turn is used by the CI's fuzz job.

@twizmwazin Please update google/oss-fuzz#12028 to use test_corpus3.py.

Also, there are plenty of other scripts which have a shebang of /usr/bin/env python or even /usr/sbin/python or similar (they should be all /usr/bin/env python3).

Some of them are Python2 scripts, which can be deleted.
@pyrox0 If you could do this in PR as well, it would be nice. We can make this PR a simple "delete Python2" PR.

@Rot127
Copy link
Collaborator

Rot127 commented Jun 24, 2024

@pyrox0 Just saw that this PR is the only one which stops us from releasing v5.0.2.
Please ignore my request to add any unnecessary Python 2 removal things here.

But do you think you find time in the next week to finish the distutils issue, Python 3.6 drop and Python 3.12 support?

@Rot127 Rot127 added this to the v5.0.2 milestone Jun 24, 2024
@twizmwazin
Copy link
Contributor

OSS-Fuzz PR updated

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

Successfully merging this pull request may close these issues.

None yet

5 participants