capstone-engine / capstone

Capstone disassembly/disassembler framework for ARM, ARM64 (ARMv8), Alpha, BPF, Ethereum VM, HPPA, LoongArch, M68K, M680X, Mips, MOS65XX, PPC, RISC-V(rv32G/rv64G), SH, Sparc, SystemZ, TMS320C64X, TriCore, Webassembly, XCore and X86.
http://www.capstone-engine.org
7.18k stars 1.52k forks source link

Remove python2 leftovers #2378

Closed twizmwazin closed 3 weeks ago

twizmwazin commented 1 month ago

Your checklist for this pull request

Detailed description

Removes the python2 bits I could find.

Removes the python2 bits. No functional changes on python3. Extra changes avoided.

Test plan

Run tests on supported python3 versions.

Closing issues

2342

Rot127 commented 1 month ago
[ 99%] Building C object CMakeFiles/cstool.dir/cstool/getopt.c.o
[100%] Linking C executable cstool
[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 219, in <module>
    long_description=open('README.txt', encoding="utf8").read(),
TypeError: 'encoding' is an invalid keyword argument for this function
2024-06-02 08:12:42,865 - root - ERROR - Building fuzzers failed.
2024-06-02 08:12:42,865 - root - ERROR - Error building fuzzers for (commit: 90f34212fd79e050a3b60e6b7f5dbc525e525fa4, pr_ref: refs/pull/2378/merge).
twizmwazin commented 1 month ago

Looks like the OSS Fuzz stuff is still using python 2. I can make a pull request to the OSS fuzz to switch them to using a newer python 3, but this will need to be merged first. Is it ok if we disable the fuzz pipeline temporarily here, make a pull request to OSS Fuzz, then re-enable the fuzz pipeline again once the changes are merged?

Edit: Actually nevermind I don’t think the disabling is necessary, I’ll make a PR over there today.

Rot127 commented 1 month ago

I would be surprised if the OSS Fuzz is still using Python 2. Maybe CS just uses an outdated branch or something?

twizmwazin commented 4 weeks ago

Take a look here: https://github.com/google/oss-fuzz/tree/master/projects/capstone

In Dockerfile, it is using the generic base, not the python base, and installs python2. The branches are current, v5 and next. In build.sh, it uses python setup.py install, which will be python2.

twizmwazin commented 4 weeks ago

Opened the PR on OSS-Fuzz: https://github.com/google/oss-fuzz/pull/12028

Looks like I'll need an existing contributor to sign off on the change.

kabeor commented 4 weeks ago

@aquynh, How do you think about this? Remove python 2 supporting.

XVilka commented 4 weeks ago

@aquynh, How do you think about this? Remove python 2 supporting.

Python 2 support was already removed few months ago and discussed before. These are only remnants/leftovers.

XVilka commented 3 weeks ago

It seems that OSSFuzz PR fails because of the 5.0.1 use in the repo, so it should either should be switched to use next, or this one should be force-merged ignoring the red CIFuzz worker for now.

kabeor commented 3 weeks ago

Still waitting for rerun in https://github.com/google/oss-fuzz/pull/12028 .

kabeor commented 1 week ago

Considering that the PR still needs more modifications to adapt to oss-fuzz, we must revert to let the project continue to progress. Welcome to continue to contribute.

XVilka commented 1 week ago

@kabeor @Rot127 I suppose this can be unreverted again?

Rot127 commented 6 days ago

@twizmwazin If you have a little time, it would be awesome if you could make another one. If you have one or two minutes more, please also do a grep bin/python[^3] over all scripts, update the shebang and remove the Python2 ones.