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

setuptools ==69.3.0 builds my-package CLI as my_package (69.2.0 would build my-package CLI as my-package #4300

Open
kmcquade opened this issue Apr 12, 2024 · 8 comments

Comments

@kmcquade
Copy link

setuptools version

setuptools==69.3.0

Python version

Python 3.11

OS

Linux, Mac

Additional environment information

No response

Description

We have a CLI that is built with setuptools. We didn't have our version pinned. Right after setuptools==69.3.0 was released to PyPi, our automated jobs started failing that use this package.

Let's call the package my-package.

We install the CLI with:

setup-env:
	pip3 install pipenv
	pipenv install
build: setup-env clean
	python3 -m pip install setuptools==69.2.0 wheel==0.43.0
	python3 -m setup -q sdist bdist_wheel

I noticed that setuptools came out with a release less than an hour ago (at the time of this writing) so I tried pinning the setuptools version to the prior version (69.2.0). It worked.

Changing make build to this fixed the error:

build: setup-env clean
	python3 -m pip install setuptools==69.2.0 wheel==0.43.0
	python3 -m setup -q sdist bdist_wheel

The setup.py script is like this:

"""Setup script"""
import setuptools
import os

setuptools.setup(
    name="my-package",
    include_package_data=True,
    version="420.69",
    author="Kinnaird McQuade",
    author_email="me@me.net",
    description="My package",
    url="https://github.com/my-org/my-package",
    packages=setuptools.find_packages(exclude=["test*"]),
    install_requires=[
        "requests"
    ],
    classifiers=[
        "Programming Language :: Python :: 3",
        "Operating System :: OS Independent",
    ],
    entry_points={"console_scripts": "my-package=my_package.bin.cli:main"},
    zip_safe=True,
    python_requires=">=3.11",
)

Before the downgrade, we were getting messages like this in GitHub Actions when we would try to install the package

python3 -m pip install -q ./dist/my-package*.tar.gz
WARNING: Requirement './dist/my-package*.tar.gz' looks like a filename, but the file does not exist
ERROR: Could not install packages due to an OSError: [Errno 2] No such file or directory: '/home/runner/work/e2e-tests/e2e-tests/my-package/dist/my-package*.tar.gz'

And I saw that if you checked out the contents of the dist/ folder, the file that was there was titled my_package-420.69.tar.gz, not my-package-420.69.tar.gz as it was before 69.3.0.

Hope this helps. Our current workaround of pinning the version works, but wanted to flag as soon as possible. Thanks.

Expected behavior

See the description

How to Reproduce

See the description

Output

See the description

@kmcquade kmcquade added bug Needs Triage Issues that need to be evaluated for severity and status. labels Apr 12, 2024
@kmcquade kmcquade changed the title [BUG] setuptools ==69.3.0 builds my-package as my_package [BUG] setuptools ==69.3.0 builds my-package CLI as my_package (69.2.0 would build my-package CLI as my-package Apr 12, 2024
@kmcquade
Copy link
Author

I think it was due to this?

https://github.com/pypa/setuptools/pull/4286/files#diff-b9c5224191f52b3ea80acfdc52fce5ea9e840a6a9237d479918bb7e974642f0bR266

It's unfortunate that this affects any CLI tool that wants to be my-package versus my_package and builds with setup.py.

from setuptools import setup

setup(
    # ...,
    entry_points={
        'console_scripts': [
            'hello-world = timmins:hello_world',
        ]
    }
)

@mikealfare
Copy link

Yup, this broke our CI as well. I would argue this is probably not a patch release.

@abravalheri
Copy link
Contributor

abravalheri commented Apr 12, 2024

Please note that the specific name of the built file is subject to change according to the changes Packaging PEPs (that is what happened in this case). Depending on the exact name, without considering that it might be cannonicalised to a different but corresponding form, will likely introduce points of failure in your CI.

So this is not a bug, but an intentional change that was required to align to the latest standardisation efforts.

@kmcquade, @mikealfare, could you please try to use patterns/regexes in your CI scripts to make them robust to name normalisation?

@abravalheri abravalheri changed the title [BUG] setuptools ==69.3.0 builds my-package CLI as my_package (69.2.0 would build my-package CLI as my-package setuptools ==69.3.0 builds my-package CLI as my_package (69.2.0 would build my-package CLI as my-package Apr 12, 2024
@abravalheri abravalheri added out-of-scope and removed bug Needs Triage Issues that need to be evaluated for severity and status. labels Apr 12, 2024
@mikealfare
Copy link

Yeah, we're making those updates currently.

@jaraco
Copy link
Member

jaraco commented Apr 12, 2024

It's unfortunate that this affects any CLI tool that wants to be my-package versus my_package and builds with setup.py.

I only expect the name of the package as reflected by the package metadata to be affected. It should not affect the package's true name nor its import name nor the names of any console scripts. It's unfortunate that applications were depending on this naming convention, but since it was merely an implementation detail and had no backing tests or specifications, the transition to something that is specified and standardized means you can now start to rely on the naming.

@mikealfare
Copy link

I only expect the name of the package as reflected by the package metadata to be affected.

The issue was that the names of the artifacts also changed. So if you want to do anything with them locally (e.g. verify them, log the generated files that fit a regex, etc.) prior to publishing them, then you rely on the names to some extent.

the transition to something that is specified and standardized means you can now start to rely on the naming.

I agree that we should move towards standardization. However, here are the changes we wound up making as a result of this change:

  • broaden our regex (effectively don't rely on the name)
  • hard pin setuptools to the minor in our CI to avoid being broken in the future, and rely on things like dependabot changes to push patch updates

copybara-service bot pushed a commit to grpc/grpc that referenced this issue Apr 15, 2024
`setuptools` made a decision to change the artifact name it builds (from `grpcio-health-checking` to `grpcio_health_checking`) in their latest release (pypa/setuptools#4300).
As a result, we need broaden our regex so that our tests can pickup the correct files.

### Note
* Using `[_-]*` instead of `[_-]?` to match one character since `bash` uses a different flavor of regular expressions called basic regular expressions (BREs) which do not support the optional quantifier `?`.

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes #36352

COPYBARA_INTEGRATE_REVIEW=#36352 from XuanWang-Amos:fix_distribution_test 8dfcc4e
PiperOrigin-RevId: 625083784
XuanWang-Amos added a commit to XuanWang-Amos/grpc that referenced this issue Apr 15, 2024
`setuptools` made a decision to change the artifact name it builds (from `grpcio-health-checking` to `grpcio_health_checking`) in their latest release (pypa/setuptools#4300).
As a result, we need broaden our regex so that our tests can pickup the correct files.

### Note
* Using `[_-]*` instead of `[_-]?` to match one character since `bash` uses a different flavor of regular expressions called basic regular expressions (BREs) which do not support the optional quantifier `?`.

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes grpc#36352

COPYBARA_INTEGRATE_REVIEW=grpc#36352 from XuanWang-Amos:fix_distribution_test 8dfcc4e
PiperOrigin-RevId: 625083784
XuanWang-Amos added a commit to grpc/grpc that referenced this issue Apr 15, 2024
Backport of #36352 to v1.63.x.
---
`setuptools` made a decision to change the artifact name it builds (from
`grpcio-health-checking` to `grpcio_health_checking`) in their latest
release (pypa/setuptools#4300).
As a result, we need broaden our regex so that our tests can pickup the
correct files.

### Note
* Using `[_-]*` instead of `[_-]?` to match one character since `bash`
uses a different flavor of regular expressions called basic regular
expressions (BREs) which do not support the optional quantifier `?`.

<!--

If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the
appropriate
lang label.

-->
@piehld
Copy link

piehld commented Apr 16, 2024

Is this update is only intended to canonicalize sdist files? Or is it also supposed to apply to wheel files?

The binary-distribution-format specification seems to suggest a similar naming standardization (i.e., no periods or hyphens). However, for our packages, while sdist files are now being canonically-named, bdist_wheel is still producing the original file names. E.g.:

/Users/runner/work/1/s/dist/rcsb_utils_config-0.39.tar.gz
/Users/runner/work/1/s/dist/rcsb.utils.config-0.39-py2.py3-none-any.whl

@abravalheri
Copy link
Contributor

abravalheri commented Apr 16, 2024

The binary-distribution-format specification seems to suggest a similar naming standardization (i.e., no periods or hyphens). However, for our packages, while sdist files are now being canonically-named, bdist_wheel is still producing the original file names. E.g.:

@piehld, likely to be related to the update in the standard:

February 2021: The rules on escaping in wheel filenames were revised, to bring them into line with what popular tools actually do.

This update is not implemented yet, only PEP 625 is in the process of being adopted by setuptools.
In the future it is likely to change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants