-
-
Notifications
You must be signed in to change notification settings - Fork 258
Fix Python deprecation warnings during build #2427
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
base: main
Are you sure you want to change the base?
Fix Python deprecation warnings during build #2427
Conversation
…t#2306) - Remove deprecated codecs.open() import from setup.py (Python 3.14+) - Replace setup.py install with pip install to avoid SetuptoolsDeprecationWarning - Use pip's modern installation interface instead of deprecated distutils commands - Fixes warnings: 'Please avoid running setup.py directly' and 'codecs.open() is deprecated' 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Fixes CI build failures by ensuring pip module is available for python3 -m pip install command. The original PR performancecopilot#2427 changed from setup.py install to pip install to avoid deprecation warnings, but pip wasn't installed in most CI environments. This adds python3-pip to the package manifest for all supported platforms (dpkg, rpm, emerge, pkgin, pkg_add, F_pkg, S_pkg, slackpkg, pacman, brew). Verified package availability: - Ubuntu 18.04+: python3-pip (pip 9.0.1+) - Debian 11+: python3-pip (pip 20.3.4+) - CentOS Stream 8+: python3-pip - Fedora 42+: python3-pip (pip 24.3.1+) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
ok, this is working on the Mac because we include python3-pip as a dependency. To fix this we'd need to add that as a dependency to all operating systems. This probably needs discussion @natoscott . |
The previous commit added python3-pip to the manifest but didn't update the static package list files used by CI. The CI workflows use list-packages with -x pip3 which was intended to exclude Python packages that need pip install (tagged as "pip3"), but python3-pip is a system package that should be installed via the system package manager. This adds python3-pip to all affected platform package lists: - Ubuntu 18.04, 20.04, 22.04, 24.04 (x86_64 and i686) - Debian 11, 12, 13 (x86_64, i686, aarch64) - CentOS Stream 8, 9, 10 (x86_64) Without python3-pip installed, the build fails with: /usr/bin/python3: No module named pip
The previous fix added 'python3 -m pip install' but didn't include the --record flag that was present in the old PY3_INSTALL_OPTS. This flag creates the python3-pcp.list file which is required by debian/post-auto-install for packaging. Without this file, the debian build fails with: post-auto-install: Error: python3-pcp expects ../python3-pcp.list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tallpsmith ...
We need a bit more Build-Depends glue, but that can be added later.
More critically the build out of the fix-python-setup-warning branch of your git tree is not working for Debian. It is failing here ...
=== python ===
export nothing=''; /usr/bin/python3 -m pip install --no-build-isolation --root="${DIST_ROOT:-/}" --prefix=/usr .
Processing /home/kenj/src/pcp-psmith/build/deb/pcp-7.1.0/src/python
Preparing metadata (setup.py): started
Preparing metadata (setup.py): finished with status 'done'
Building wheels for collected packages: pcp
Building wheel for pcp (setup.py): started
Building wheel for pcp (setup.py): finished with status 'done'
Created wheel for pcp: filename=pcp-7.0-cp312-cp312-linux_x86_64.whl size=224158 sha256=1975f2bcfd74e786c6038e0a14067177847635e5c3e41f45932add0a1c9c57f5
Stored in directory: /tmp/pip-ephem-wheel-cache-0dlslsj_/wheels/3a/a4/60/fe3f1b7f899ff1730d6c91c3e1cd9436e2791f93c7f4b6f316
Successfully built pcp
Installing collected packages: pcp
Attempting uninstall: pcp
Found existing installation: pcp 7.0
Uninstalling pcp-7.0:
ERROR: Could not install packages due to an OSError: [Errno 13] Permission denied: 'pmi.py'
Consider using the `--user` option or check the permissions.
Now this does not look even close to right to me from about the "Attempting uninstall: pcp" line.
I'm afraid I have no knowledge of Python packaging, and plan to keep it that way until I fall off the perch.
|
@tallpsmith No --record for |
|
Yeah @kmcdonell this is looking hairier than Hairy Maclary right now as I dig in. I also have to solve the "how do I run the CI podman setup locally" so that I can easily iterate locally, however I ran into a container architecture mismatch with Apple Silicon that might be another rabbit hole. I may end up resorting to an Option Claude Code suggested earlier - simply suppress the warnings! :) |
Summary
Eliminates Python deprecation warnings that appear during the build process by modernizing the Python packaging approach.
Changes
setup.py installwithpip installin the build systemcodecs.open()import from setup.pypython3-pipto QA package manifest for all supported platforms (CI build dependency)Why These Changes Are Safe
codecs.open() removal:
The
codecs.open()import was added in 2016 when PCP supported Python 2, where the built-inopen()function lacked anencodingparameter. In Python 3 (the only version PCP now supports), the built-inopen()natively supports encoding and is the recommended approach. Both functions behave identically for UTF-8 files.pip install migration:
The
setup.py installcommand has been deprecated by setuptools in favor of standards-based tools like pip. The new approach usespip installwith equivalent flags to achieve the same installation behavior.python3-pip in manifest:
The pip module is now required for the build process but wasn't installed in most CI environments. Added python3-pip entries to
qa/admin/other-packages/manifestfor all package managers (dpkg, rpm, emerge, etc.) to ensure pip is available during builds. Verified availability on all supported platforms: Ubuntu 18.04+, Debian 11+, CentOS Stream 8+, Fedora 42+, and others.Testing
Fixes #2306