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

Do not call method that was removed in Python 3.9 in six #1

Conversation

radarhere
Copy link

An attempt to help google#12027

You linked to a log with a failure for six - https://oss-fuzz-gcb-logs.storage.googleapis.com/log-4180e85e-4b31-4442-849a-2188343052bc.txt

AttributeError: 'HTMLParser' object has no attribute 'unescape'

The failure makes sense as a result of the Python version upgrade, since in Python 3.9, 'unescape' was removed from HTMLParser - https://docs.python.org/3/whatsnew/3.9.html#porting-to-python-3-9

The unescape() method in the html.parser.HTMLParser class has been removed (it was deprecated since Python 3.4).

However, if you look at the six source code, unescape isn't even used. HTMLParser is sure, but that's it. So I think the problematic code can be removed from fuzz_six.py - the method is gone in Python >= 3.9, and Python 3.8 is EOL, so there's no need to continue testing it.

DaveLak pushed a commit that referenced this pull request Sep 26, 2024
- OSS-Fuzz builds are failing after pigweed was rebased in
project-chip/connectedhomeip#35644
- One of the failures is related to pigweed becoming incompatible with
python <3.9. Such as using subscript notation in the type hints.

- Fix: Base images in OSS-Fuzz use python 3.8, This PR aims to force the
usage of python3.10 instead
### Example Error
```
Step #1: Traceback (most recent call last):
Step #1:   File "../../third_party/pigweed/repo/pw_build/py/pw_build/python_runner.py", line 38, in <module>
Step #1:     import gn_resolver  # type: ignore
Step #1:   File "/src/connectedhomeip/third_party/pigweed/repo/pw_build/py/pw_build/gn_resolver.py", line 319, in <module>
Step #1:     _Actions = Iterator[tuple[_ArgAction, str]]
Step #1: TypeError: 'type' object is not subscriptable
Step #1: [137/1234] ln -f ../../third_party/pigweed/repo/pw_thread/pw_thread_protos/thread_snapshot_service.proto
```
DaveLak pushed a commit that referenced this pull request Oct 1, 2024
Fix build error
https://oss-fuzz-build-logs.storage.googleapis.com/log-17235690-03af-4f88-a2c8-1f7203f4695c.txt

```Step #1: Step 3/4 : COPY build.sh *.dict $SRC
Step #1: When using COPY with more than one source file, the destination must be a directory and end with a /
Finished Step #1
```

@AdamKorcz
@DaveLak DaveLak changed the base branch from upgrade-python-to-3-10-14 to infra/upgrade-python-to-3-10-14/patch-failing-projects October 3, 2024 08:14
@DaveLak
Copy link
Owner

DaveLak commented Oct 3, 2024

Hey @radarhere, thanks for taking the time to help with this! I wasn't expecting a PR on this repo so I must have missed the notification.

However, if you look at the six source code, unescape isn't even used. HTMLParser is sure, but that's it. So I think the problematic code can be removed from fuzz_six.py - the method is gone in Python >= 3.9, and Python 3.8 is EOL, so there's no need to continue testing it.

You're right. I saw that as well in my previous attempt to fix this build, but you've made me realize that I was too caught up on preserving the unescape call and without stopping to think through whether it's even valuable to test anymore.

I agree with your assessment that it's not.

I'll merge this into the separate branch that has the other project specific patches so that we can keep this branch scoped to the infrastructure changes.

I'm still not sure if or how the upstream maintainers would want to handle the project patches but I'm hopeful separating the infra and project changes will make it easier to get the changes reviewed and merged at all.

@DaveLak DaveLak merged commit 627ab3d into DaveLak:infra/upgrade-python-to-3-10-14/patch-failing-projects Oct 3, 2024
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.

5 participants