Add PEP-625 complaint changes #12233

vkuznet · 2025-01-20T13:34:43Z

Status

In development

Description

Propagate PEP-625 complaint changes to WMCore repository. It includes changing all - with _ in files related to WMCore build workflows.

Is it backward compatible (if not, which system it affects?)

MAYBE

Related PRs

External dependencies / deployment changes

dmwm-bot · 2025-01-20T13:50:18Z

Jenkins results:

Python3 Unit tests: failed
- 1 new failures
- 2 changes in unstable tests
Python3 Pylint check: succeeded
- 1 comments to review
Pycodestyle check: succeeded
- 28 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/286/artifact/artifacts/PullRequestReport.html

dmwm-bot · 2025-01-20T14:29:55Z

Jenkins results:

Python3 Unit tests: succeeded
- 1 changes in unstable tests
Python3 Pylint check: succeeded
- 1 comments to review
Pycodestyle check: succeeded
- 28 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/287/artifact/artifacts/PullRequestReport.html

dmwm-bot · 2025-01-20T14:34:28Z

Jenkins results:

Python3 Unit tests: succeeded
- 3 changes in unstable tests
Python3 Pylint check: succeeded
- 1 comments to review
Pycodestyle check: succeeded
- 28 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/288/artifact/artifacts/PullRequestReport.html

dmwm-bot · 2025-01-20T14:50:53Z

Jenkins results:

Python3 Unit tests: succeeded
- 1 tests no longer failing
- 2 changes in unstable tests
Python3 Pylint check: succeeded
- 1 comments to review
Pycodestyle check: succeeded
- 28 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/289/artifact/artifacts/PullRequestReport.html

dmwm-bot · 2025-01-20T16:00:54Z

Jenkins results:

Python3 Unit tests: succeeded
- 1 tests no longer failing
- 3 changes in unstable tests
Python3 Pylint check: succeeded
- 1 comments to review
Pycodestyle check: succeeded
- 28 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/290/artifact/artifacts/PullRequestReport.html

vkuznet · 2025-01-20T16:45:44Z

After many iterations with GithubActions I concluded that even though we can modify our packages with underscores they will not be uploaded to PyPI properly, instead the underscores will be converted back to dashes. Here are details:

I modified .github/workflows/pypi_build_and_images.yaml file and added additional parameters to pypa/gh-action-pypi-publish section of workflow

      - name: Upload package distribution to PyPi
        uses: pypa/gh-action-pypi-publish@release/v1
        with:
          password: ${{ secrets.TEST_PYPI_API_TOKEN }}
          repository-url: https://test.pypi.org/legacy/

These changes instructed to GitHubAction to use test.pypy.org with my token for uploading pypi packages.

When I received GitHubAction build, e.g. https://github.com/vkuznet/WMCore/actions/runs/12871949872/job/35886361302, I observed that it fails with the following message:

...
Warning: A new Trusted Publisher for the currently running publishing workflow can be created by accessing the following link(s) while logged-in as an owner of the package(s):
- https://test.pypi.org/manage/project/reqmgr2ms-output/settings/publishing/?provider=github&owner=vkuznet&repository=WMCore&workflow_filename=pypi_build_and_images.yaml

You may observed that it used reqmgr2ms-output test pypi project name instead of desired reqmgr2ms_output.

Upon further investigation into gh-action-pypi-publish action, I found that it relies on print-pkg-names.py script which reads dist directory files and produce names of packages used to compose pypi upload link, see https://github.com/pypa/gh-action-pypi-publish/blob/unstable/v1/twine-upload.sh#L47

# here content of my local dist area
ls dist
reqmgr2-2.1.6rc3.tar.gz           reqmgr2ms_output-2.3.8rc13.tar.gz

# and here is output of print-pkg-names.py to parse these names
python /Users/vk/tmp/gh-action-pypi-publish/print-pkg-names.py $PWD
reqmgr2
reqmgr2ms-output

As you may observe above the reqmgr2ms_output-2.3.8rc13.tar.gz produces reqmgr2ms-output package name the twine tool will use.

By inspecting print-pkg-names.py I found that the following line is responsible for aforementioned behavior:
https://github.com/pypa/gh-action-pypi-publish/blob/unstable/v1/print-pkg-names.py#L20

And, it can be reproduced easily using utils package from Python distribution:

python
Python 3.11.11 (main, Dec  7 2024, 12:01:32) [Clang 16.0.0 (clang-1600.0.26.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from packaging import utils
>>> utils.parse_sdist_filename("reqmgr2ms_output-2.3.8rc13.tar.gz")
('reqmgr2ms-output', <Version('2.3.8rc13')>)

Therefore, even though we modify our files, like requirements.txt, etc., the gh-action-pypi-publish action will in fact upload tarball under incorrect package name. In example above the GitHub action will properly construct tar ball name as reqmgr2ms_output-2.3.8rc13'.tar.gz but it will attempt to upload it under https://test.pypi.org/manage/project/reqmgr2ms-output URL which is obviously wrong and does not comply with required PEP-625. As such I conclude the following:

we either need to update python where utils package itself will be compliant with PEP-625 requirements
we may use different GH action (even though I don't know if it exists) or modify or file complain to gh-action-pypi-publish repository

Moreover, I found that this GH action upload packages into Trusted repositories, see various sections in pypi docs here, are unique, i.e. if userA create repository RepoA such repo name can't be used by another user. This creates additional troubles of publishing packages under specific repositories if these repositories are already allocated by someone else in PyPi. For instance in Test PyPi repo we already have wmagent repo owned by Todor and cms-oc-dmwm user. And, because of that I (under my TestPyPi user name) can't upload packages over there.

vkuznet · 2025-01-20T18:16:05Z

Oops, closed by wrong mouse click. Re-opening.

dmwm-bot · 2025-01-20T18:27:30Z

Jenkins results:

Python3 Unit tests: succeeded
- 1 tests no longer failing
- 3 changes in unstable tests
Python3 Pylint check: succeeded
- 1 comments to review
Pycodestyle check: succeeded
- 28 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/WMCore-PR-Report/292/artifact/artifacts/PullRequestReport.html

vkuznet · 2025-01-20T18:32:49Z

Digging further I found that Python PEPs are inconsistent with each other:

the utils package which I referred in my previous comment defines package name normalization via the following function .../site-packages/packaging/utils.py, see PEP-503 In particular, the PEP 503 defines names as:

This PEP references the concept of a “normalized” project name. As per PEP-426 the only valid characters in a name are the ASCII alphabet, ASCII numbers, ., -, and _. The name should be lowercased with all runs of the characters ., -, or _ replaced with a single - character. This can be implemented in Python with the re module:

import re

def normalize(name):
    return re.sub(r"[-_.]+", "-", name).lower()

as you can see from PEP-503

In other words the package name will be normalized to have dashes -, i.e. it will replace underscores with dashes.

But according to PEP-625

The name of an sdist should be {distribution}-{version}.tar.gz.

distribution is the name of the distribution as defined in PEP 345, and normalised as described in the wheel spec e.g. 'pip', 'flit_core'.
version is the version of the distribution as defined in PEP-440, e.g. 20.2, and normalised according to the rules in that PEP.

And, it is suggested by Python Packaging User guide as following:

In distribution names, any run of -_. characters (HYPHEN-MINUS, LOW LINE and FULL STOP) should be replaced with _ (LOW LINE), and uppercase characters should be replaced with corresponding lowercase ones. This is equivalent to regular name normalization followed by replacing - with _. Tools consuming wheels must be prepared to accept . (FULL STOP) and uppercase letters, however, as these were allowed by an earlier version of this specification.

Summary

The PEP-504 and PEP-426 suggests to use dashes, while PEP-625 does the opposite and suggests to use underscores. The current python code base implements PEP-503.

todor-ivanov · 2025-01-21T07:13:12Z

hi @vkuznet just some info about:

For instance in Test PyPi repo we already have wmagent repo owned by Todor and cms-oc-dmwm user

We've abandoned the Test Pypi repository, because of missing or inconsistent (between test && prod repositories) packages versions for some of those wmcore depends on. Even though, I was keeping for awhile the option in all the virtual env scripts that allowed ne to chose from which Pypi index to deploy, but it is all in the history now.

Add PEP-625 complaint changes

8c5b505

vkuznet self-assigned this Jan 20, 2025

vkuznet added 2 commits January 20, 2025 09:12

Add verbosity for build action

ee4b910

fix dist

518cde7

Add upload action to test-pypi

eaa8ace

vkuznet added 2 commits January 20, 2025 10:14

perform cleanup

6d5ce65

Remove password token

d89ee60

put back password to upload packages

57f2adf

vkuznet closed this Jan 20, 2025

vkuznet mentioned this pull request Jan 20, 2025

Make PyPi packages compliant with PEP 625 #12181

Open

vkuznet reopened this Jan 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PEP-625 complaint changes #12233

Add PEP-625 complaint changes #12233

vkuznet commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

vkuznet commented Jan 20, 2025

vkuznet commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

vkuznet commented Jan 20, 2025

todor-ivanov commented Jan 21, 2025

Add PEP-625 complaint changes #12233

Are you sure you want to change the base?

Add PEP-625 complaint changes #12233

Conversation

vkuznet commented Jan 20, 2025

Status

Description

Is it backward compatible (if not, which system it affects?)

Related PRs

External dependencies / deployment changes

dmwm-bot commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

vkuznet commented Jan 20, 2025

vkuznet commented Jan 20, 2025

dmwm-bot commented Jan 20, 2025

vkuznet commented Jan 20, 2025

Summary

todor-ivanov commented Jan 21, 2025