-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[infra] Upgrade Python to 3.10.14 in base-builder & base-runner Images #12027
base: master
Are you sure you want to change the base?
Conversation
The changes introduced here upgrade Python from 3.8 to 3.10.14 inside the base-builder and base-runner images. ### base-builder changes: Prior to these changes, base-builder compiled Python 3.8 from source using sources downloaded from the official release servers at https://www.python.org/ftp/python/. This updates the compiled version to 3.10.14 (the latest 3.10 release) instead. ### base-runner changes: Prior to these changes, base-runner installed Python 3.8 from the default apt repository provided by the Ubuntu 20.04 image it's based on. These apt repositories do not have a version of Python 3.10 available by default. This updates the base-runner to instead use a multi-stage build to copy the same Python interpreter compiled by the base-builder image into the runner image, which ensures both Python versions remain in-sync while saving build time by re-using a pre-built version. ## Motivation - Code coverage does not work on Python projects that use Python 3.10+ syntax, and will not work until this or similar changes are landed (see google#11419) - Upgrading the base-image to use Ubuntu 22.04 (which provides more recent Python versions via apt) has been stated as being unlikely to happen any time soon (see google#3290) - Many OSS-Fuzz integrated Python projects no longer support Python 3.8 and have resorted to implementing ad-hoc workarounds to upgrade to newer Python versions, including installing Python from the Dead Snakes PPA. - This leads to fragmentation and hard to debug issues. Maintenance is easier when everyone is using the same version without issue. - With [Python 3.8 reaching end of life soon (in 2024-10)][python- versions-EOL], it is likely that more Python projects will begin dropping support for 3.8, further increasing the number of broken builds and ad-hoc workarounds. - Previous attempts at upgrading Python have stalled. ## Known & Expected Issues Several project Dockerfiles and build scripts contain hard coded references to python3.8 file system paths, and many more have implanted ad-hoc workarounds to upgrade to newer Python versions than 3.8 (typically 3.9.) Additional changes are required to each of these projects to ensure they successfully build after this upgrade to Python 3.10. ### Fuzz Introspector Caveat Fuzz Introspector currently uses Python 3.9. While an upgrade to 3.10 is not expected to introduce any new issues, it was not tested on these changes and may require additional work. ## Possible Areas of Improvement Using the base-builder image in a multi-stage build to copy the pre- compiled Python into base-runner is effective, but feels like a workaround that may be introducing tech debt. A cleaner approach would be to extract the Python compilation into a discrete base image similar to how `base-clang` works, and use that as the multi-stage builder in images that need it. --- Fixes: - google#11419 Supersedes: - google#9532 - google#11420 [python-versions-EOL]: https://devguide.python.org/versions/
/gcbrun trial_build.py all --sanitizer coverage address --fuzzing-engine libfuzzer |
1 similar comment
/gcbrun trial_build.py all --sanitizer coverage address --fuzzing-engine libfuzzer |
Thanks for the runs. I'll check the timeouts in about 24 hours from now. |
`MarkupSafe` is a transitive dependency through `code_coverage`'s Jinja2 requirement. The previously pinned version, `MarkupSafe==0.23`, is incompatible with Python 3.10 raising the following error: ``` ImportError: cannot import name 'Mapping' from 'collections' ``` Upgrading MarkupSafe to a compatible version requires `code_coverage`'s Jinja2 requirement to be bumped from Jinja2==2.10 to 2.10.3 The `sed` change introduced here is not ideal, but is required until the upstream requirement is bumped. At that point, the `sed` should become a no-op.
@jonathanmetzman I think e1a6e9f should fix the broken coverage builds. |
/gcbrun trial_build.py all --sanitizer coverage address --fuzzing-engine libfuzzer |
Thanks for grabbing that list @DonggeLiu! I've started looking over them and will report back. The few I've seen so far appear to be caused by ad-hock Python version updated in build scripts which is reassuring. FYI for anyone interested: Here's the list formatted as table: Failures Table
|
Updated the hook_pre_exec_eval function in command_injection.py to accept additional arguments (*args, **kwargs). This resolves a TypeError encountered in Python 3.10 where the function was called with more arguments than expected. The change ensures compatibility with Python 3.10 by aligning the function signature with the arguments passed by the add_hook mechanism. Also replaces the deprecated `importlib.find_loader` methoc call with the recommended ` importlib.util.find_spec` alternative. These changes were tested by running the "proof-of-exploit" examples, the pyscan tests in this project, and by running `check_build` on several projects (such as `black`) that enable Pyscan.
Atheris: Among many useful patches, the Python 3.10 compatability fixes in v2.2.2 are of particular note. See https://github.com/google/atheris/releases/tag/2.2.2 Pyinstaller: Dependency collection was improved significantly between Pyintstaller v5 and v6, in both the core library, and the more recent `pyinstaller-hooks-contrib` package it ships with. Pyinstaller versions 3.9.0 & 3.10.0 are particularly noteworthy. 3.9.0 includes updates for scipy, numpy 2.0.0, and Django to fix compatibility issues. 3.10.0 implements support for setuptools >= 71.0.0 and its new approach to vendoring its dependencies. See: https://setuptools.pypa.io/en/latest/history.html Setuptools: Many projects expect a more recent version of setuptools than was previously installed, including the pyscanner sanatizer from this repo: `infra/base-images/base-builder/sanitizers/pysecsan/`
Fixes `SetuptoolsDeprecationWarning` warnings during Pyscan installation. See: - https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html - pypa/setuptools#917
@jonathanmetzman @DonggeLiu Could you please start a trial build with the latest changes? Thanks in advance! The following information provides context for the changes and is intended for reference. Analysis of Recent FailuresAfter reviewing the recent run failures, here's what I found:
JVM/Java project faluresThe failures in JVM/Java projects appear to be network errors unrelated to this PR. It seems that
|
Ensures the CI actions use the same Python version as OSS-Fuzz images.
Versions with multiple digits after the forst "." in the version number must be quoted strings, otherwise the GH action runner does not read the whole version number and actions fail with an error similar to: > Error: The version '3.1' with architecture 'x64' was not found
Also upgraddes the Cloud SDK version to the latest availiable to attempt to avoid a python 3.10 compat issue:" module 'collections' has no attribute 'MutableMapping'" tracked here: https://issuetracker.google.com/issues/202172882 This also resolves an error in the GH actions prompting for upgrade: > The v0 series of google-github-actions/setup-gcloud is no longer > maintained. It will not receive updates, improvements, > or security patches.
The `>=` was unintentionally changed to `==` in commit: e6fc52c This reverts that change.
for consistentcy with pip commands in other files
The issue these attempted to solve appear to be related to GH Action caching and not the python version, meanwhile upgrading python in these actions introduces additional issues that would need to be addressed. - Revert "Bump Python Version from 3.8 to 3.10 in GitHub Actions" from commit 8b056dc. - Revert "Specify Python Version as Strings" from commit c4957f5. - Revert "Bump google-github-actions/setup-gcloud from v0 to v2" from commit 26a5c01.
This reverts commit de241d9.
Edit: Disregard the comment below. Rerunning it on a merge was successful.
|
@jonathanmetzman, could you trigger another trial build whenever you have a chance? Thanks! |
/gcbrun trial_build.py all --sanitizer coverage address --fuzzing-engine libfuzzer |
Pillow's oss-fuzz is currently failing, because Pillow main has [dropped support for Python 3.8](python-pillow/Pillow#8183). #12027 attempts to resolve this by upgrading the base builder to Python 3.10, but the [latest status update](#12027 (comment)) over there says that Pillow [fails in that branch with](https://oss-fuzz-gcb-logs.storage.googleapis.com/log-e19625fb-a984-47b1-be32-f92b4c76fc7f.txt) > Step 17: Step 6/11 : RUN ln -s /usr/local/bin/python3 /usr/local/bin/python && ln -s /bin/true /usr/local/bin/yum_install && ln -s /bin/true /usr/local/bin/yum && cd $SRC/Pillow && git submodule update --init wheels/multibuild && bash $SRC/build_depends.sh Step 17: ---> Running in 932abd1dc89a Step 17: �[91mln: failed to create symbolic link '/usr/local/bin/python': File exists So this pull request removes the python symlink in order to try and help. cc @hugovk --------- Co-authored-by: Andrew Murray <[email protected]>
That PR has now been merged into master. |
Thanks, @DonggeLiu. I've looked into the latest trial build failures from the build at 9db721e (GH Run: https://github.com/google/oss-fuzz/runs/29280838754) and outlined my findings below. Latest trial build failuresFix IdentifiedI've identified fixes for the failed projects in this section and committed them to a new branch on my fork for now. Project: configparser
Project: fwupd
Project: gitpython
Project: nbclassic
Project: pffft
Project: proto-plus-python
Project: pybind11
Project: pyzmq
Project: django
Failure Related but Require Further InvestigationProject: six
Project: ijson
Project: pycrypto
As noted in the PR description above, Pycrypto is deprecated and this is unlikely to be fixed upstream. Likely Unrelated?The project below failed with the following message:
I was not able to reproduce this locally. Project: lua
Project: libxml2
Project: tinyusb
Project: tarantool
Unrelated FailuresProject: bloaty
|
I'm not sure what these failed "Project tests / build" CI checks are about, but it seems to happen intermittently on this PR and the logs make me think it has something to do with a stale Docker image cache? |
@DonggeLiu or @jonathanmetzman, could either of you comment on what next steps are necessary to get this landed? Given the latest trial build is showing the changes proposed in this PR will only break ~9 project builds (7 of which I've prepared a patch for) and the significantly larger number projects that would benefit from the Python version bump, I'm eager to bring this over the finish line. Is there anything more I can do for the time being? |
@DonggeLiu @jonathanmetzman Just bumping this up in case it got missed. Could you share any thoughts on the feasibility of this Python upgrade approach and whether it’s worth keeping the branch updated? |
Fyi any project that depends on zephyr is likely broken until this lands see zephyrproject-rtos/zephyr@9d1b361 |
Note
I was looking for somewhere to get feedback from maintainers about this approach to the Python 3.10 upgrade before attempting it, but the discussion surrounding a Python upgrade has been rather fragmented across many issues, PRs, and comment chains.
For that reason, I felt it would be easier to propose with a working example and dedicated PR.
Fixes:
Supersedes:
Changes
The changes introduced here upgrade Python from 3.8 to 3.10.14 inside the base-builder and base-runner images.
Base Image Changes
Known Impact on Projects
3.9 Workarounds That Can Be Removed
Anticipated Build Failures
Preexisting Failures
Fix is Prepared
Fix Requires Upstream Changes
archinfo
dependency requires >=3.10. Fails after the 3.10 upgrade because the upstream build script needspython3.9
replaced withpython3
.Requires More Investigation
TypeError: Parser.non_math() takes 2 positional arguments but 4 were given" in "File "fuzz_plt.py", line 43, in TestOneInput
.export LDFLAGS="-fuse-ld=lld"
is set, the error becomes: "ld.lld: error: undefined symbol: __asan_report_store4
".build.sh
is the issue.SystemError: PY_SSIZE_T_CLEAN macro must be defined for '#' formats
". Seems like the issue described here. Pycrypto is deprecated and this is unlikely to be fixed upstream.Possible Future Improvements
Using the base-builder image in a multi-stage build to copy the pre- compiled Python into base-runner is effective, but feels like a workaround that may be introducing tech debt. A cleaner approach would be to extract the Python compilation into a discrete base image similar to how
base-clang
works, and use that as the multi-stage builder in images that need it.Fuzz Introspector Caveat
Fuzz Introspector currently uses Python 3.9. While an upgrade to 3.10 is not expected to introduce any new issues, it was not tested on these changes and may require additional work.
Motivation
numpy
,scipy
,pandas
, etc.)