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

Version 1.2 breaks pypa/build on Python 3.8 and 3.9 #206

Open
henryiii opened this issue Oct 6, 2024 · 20 comments
Open

Version 1.2 breaks pypa/build on Python 3.8 and 3.9 #206

henryiii opened this issue Oct 6, 2024 · 20 comments

Comments

@henryiii
Copy link
Contributor

henryiii commented Oct 6, 2024

1.2 has broken build's test suite when using backend-path on Python 3.8 and 3.9 (and check-sdist's setuptools test, though I just dropped the setuptools test there). See pypa/build#820, python/importlib_metadata#508, henryiii/check-sdist#55. I've finally narrowed it down to this package's 1.2 release.

Backend path is ".", and setuptools is making test_no_prepare.egg-info, which is then getting picked up as a nameless distribution, which causes a confusing error when trying to get the name and falling back on None, then not handling the None case.

Originally posted by @henryiii in #199 (comment)

Traceback (most recent call last):
  File "/build/.tox/py39/lib/python3.9/site-packages/importlib_metadata/compat/py39.py", line 19, in normalized_name
    return dist._normalized_name
AttributeError: 'PathDistribution' object has no attribute '_normalized_name'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/build/.tox/py39/lib/python3.9/site-packages/pyproject_hooks/_in_process/_in_process.py", line 389, in <module>
    main()
  File "/build/.tox/py39/lib/python3.9/site-packages/pyproject_hooks/_in_process/_in_process.py", line 373, in main
    json_out["return_val"] = hook(**hook_input["kwargs"])
  File "/build/.tox/py39/lib/python3.9/site-packages/pyproject_hooks/_in_process/_in_process.py", line 280, in build_wheel
    return _build_backend().build_wheel(
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/build_meta.py", line 421, in build_wheel
    return self._build_with_temp_dir(
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/build_meta.py", line 403, in _build_with_temp_dir
    self.run_setup()
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/build_meta.py", line 318, in run_setup
    exec(code, locals())
  File "<string>", line 1, in <module>
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/__init__.py", line 117, in setup
    return distutils.core.setup(**attrs)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/core.py", line 183, in setup
    return run_commands(dist)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/core.py", line 199, in run_commands
    dist.run_commands()
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/dist.py", line 954, in run_commands
    self.run_command(cmd)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/dist.py", line 950, in run_command
    super().run_command(command)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/dist.py", line 973, in run_command
    cmd_obj.run()
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/command/bdist_wheel.py", line 433, in run
    self.run_command("install")
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/cmd.py", line 316, in run_command
    self.distribution.run_command(command)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/dist.py", line 950, in run_command
    super().run_command(command)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/dist.py", line 973, in run_command
    cmd_obj.run()
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/command/install.py", line 91, in run
    return super().run()
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/command/install.py", line 706, in run
    self.run_command(cmd_name)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/cmd.py", line 316, in run_command
    self.distribution.run_command(command)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/dist.py", line 950, in run_command
    super().run_command(command)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/dist.py", line 973, in run_command
    cmd_obj.run()
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/command/install_egg_info.py", line 32, in run
    self.run_command('egg_info')
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/cmd.py", line 316, in run_command
    self.distribution.run_command(command)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/dist.py", line 950, in run_command
    super().run_command(command)
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/_distutils/dist.py", line 973, in run_command
    cmd_obj.run()
  File "/build/.tox/py39/lib/python3.9/site-packages/setuptools/command/egg_info.py", line 302, in run
    for ep in metadata.entry_points(group='egg_info.writers'):
  File "/build/.tox/py39/lib/python3.9/site-packages/importlib_metadata/__init__.py", line 1051, in entry_points
    return EntryPoints(eps).select(**params)
  File "/build/.tox/py39/lib/python3.9/site-packages/importlib_metadata/__init__.py", line 1048, in <genexpr>
    eps = itertools.chain.from_iterable(
  File "/build/.tox/py39/lib/python3.9/site-packages/importlib_metadata/_itertools.py", line 17, in unique_everseen
    k = key(element)
  File "/build/.tox/py39/lib/python3.9/site-packages/importlib_metadata/compat/py39.py", line 23, in normalized_name
    return Prepared.normalize(getattr(dist, "name", None) or dist.metadata['Name'])
  File "/build/.tox/py39/lib/python3.9/site-packages/importlib_metadata/__init__.py", line 889, in normalize
    return re.sub(r"[-_.]+", "-", name).lower().replace('-', '_')
  File "/usr/local/lib/python3.9/re.py", line 210, in sub
    return _compile(pattern, flags).sub(repl, string, count)
TypeError: expected string or bytes-like object
@layday
Copy link
Member

layday commented Oct 6, 2024

It would appear #199 returns a stdlib (importlib.metadata) distribution which setuptools then attempts to operate on using the backport (importlib_metadata) and the two are incompatible on Python 3.8 and .9.

@pradyunsg
Copy link
Member

/cc @jaraco @takluyver @abravalheri for awareness.

@pradyunsg
Copy link
Member

Responding to #199 (comment):

I'm sorely tempted to declare that pyproject-hooks now requires Python 3.10 or above, and you should just pin whichever version works for you on older Python versions. I'm normally pretty keen on backwards compatibility, so I'd welcome a better solution, but I don't have the time or energy to wade through importlib stuff to figure this out. And I don't know how we can trust that the next solution won't just lead to another of these bugs popping up somewhere.

I have two concerns with that:

  • pip won't be able to upgrade to a newer version of pyproject_hooks, because pip can't be dropping support for Python 3.9 just yet. So, the underlying pip fails to build project from backend-path when MetaPathFinder is present and installing from VCS url pip#11812 issue can't be resolved until at least an year from now. Plus, pip won't be able to pull in any bugfixes etc from this project outside of maintaining its own fork... which is unappealing to say the least. 😓
  • If we do this, we should yank 1.2.0 and release a 1.3.0 with the right python-requires, so that anyone using 3.8/3.9 isn't going to get a broken release by default.

@pfmoore
Copy link
Member

pfmoore commented Oct 6, 2024

My inclination is to say that this is either a bug in setuptools (which should spot when it gets a stdlib object and deal with it) or in the importlib_metadata backport (which should offer some sort of compatibility).

None of which helps that much, unless those projects can release fixes ASAP, but at the same time, I sympathise with @takluyver's point that if we can't rely on being able to safely pass distribution objects around, we're going to be at risk of yet more areas of incompatibility popping up.

@takluyver
Copy link
Member

If we do this, we should yank 1.2.0 and release a 1.3.0 with the right python-requires...

We might then need to yank 1.1.0 as well, since that was already reported as a regression (#192) and 1.2.0 was meant to fix it. And then I presume people would be upset about the problem that #165 was written to fix again. I totally agree that I don't want to do this.

A slightly less drastic version might be to change the conditional added in #199 - if sys.version_info >= (3, 8): - to (3, 10). This would mean no importlib[._]metadata integration on Python 3.8 or 3.9 (like v1.1), but keep the integration (like v1.2) for newer Pythons.

If setuptools is committed to using the backport on 3.8 & 3.9, we could decide that compatibility with setuptools is more important than anything else and insist on using the backport for those versions as well. But that presumably breaks other code that does the right thing and uses the stdlib version. And it's an extra headache for anyone who cares about bootstrapping, since importlib_metadata needs setuptools & setuptools_scm to build it. I really don't like that option.

@layday
Copy link
Member

layday commented Oct 6, 2024 via email

@henryiii
Copy link
Contributor Author

henryiii commented Oct 6, 2024

It broke check-sdist’s tests too, where it was just building an SDist for setuptools. I believe it breaks any package that has a local backend at “.” and uses setuptools. Including setuptools itself (https://github.com/pypa/setuptools/blob/66a8aee91de7ed8b0f819fb836e5543212e4b860/pyproject.toml#L4) and setuptools-scm. I’ve verified locally that you can’t build setuptools-scm anymore. Setuptools-scm uses uv to build SDists & wheels now, which is why their CI hasn't broken this last week.

You can find others at https://github.com/search?type=code&q=path%3Apyproject.toml+backend-path - look for the ones with setuptools and “.”.

@nanonyme
Copy link

nanonyme commented Oct 6, 2024

Oh. It fixed setuptools local backend at "." based on our actual usage of build to bootstrap setuptools at least on newer Python. Wait, is this only about sdist? Because we don't build any sdists through wheel in our deptree since the only point of building sdist is redistributing sources. We only build wheels.

@henryiii
Copy link
Contributor Author

henryiii commented Oct 6, 2024

This is only about Python 3.8 and 3.9, everything works fine after that.

It breaks both:

$ git clone https://github.com/cvxpy/cvxpy
$ cd cvxpy
$ pipx run --python 3.9 build --sdist
...
TypeError: expected string or bytes-like object
$ pipx run --python 3.9 build --wheel
...
TypeError: expected string or bytes-like object
$ pipx run --python 3.10 build --sdist
...
Successfully built cvxpy-1.6.0.tar.gz

If you avoid the new pyproject-hooks, it works:

$ uvx --python 3.9 --from build --with pyproject-hooks==1.1.0 pyproject-build --sdist
...
Successfully built cvxpy-1.6.0.tar.gz

You have to remove the *.dist-info dir after a successful build, however, otherwise it starts working.

@jaraco
Copy link
Member

jaraco commented Oct 6, 2024

It would appear #199 returns a stdlib (importlib.metadata) distribution which setuptools then attempts to operate on using the backport (importlib_metadata) and the two are incompatible on Python 3.8 and .9.

As a reminder, importlib.metadata prior to Python 3.10 was considered "provisional", so it's probably preferable to use importlib_metadata at least on those versions.

importlib_metadata has attempted to accommodate objects from those implementations, but it's complicated due to the two API surfaces that library manages (providers and consumers). In python/importlib_metadata#486, we're exploring ways to shore up some of those relationships, but that still won't fix the fact that importlib.metadata on Python 3.9 and earlier will never honor the newer interfaces and guarantees of later releases. Later versions of importlib_metadata will provide an error earlier when the Name key is missing, but only for Distributions constructed from importlib_metadata.

That said, an egg-info directory containing no metadata is in fact invalid, right? Shouldn't the solution be to address the root cause (prevent that invalid metadata from existing)? What is the origin of these empty metadata directories?

@jaraco
Copy link
Member

jaraco commented Oct 6, 2024

Oh, do we understand why this issue only affects Python 3.9 and earlier? If the underlying problem is a distribution with no Name, I'd expect that also to cause problems. I guess it doesn't because Distribution.name (or ._normalized_name) happens to return None for these invalid distributions and because the unique check when scanning entry points happens not to care if it gets None where it expects a str. That means that this issue will also affect Python 3.14, where importlib.metadata.Distribution.name will now raise a KeyError when no name is present in the metadata.

@abravalheri
Copy link
Contributor

abravalheri commented Oct 6, 2024

Isn't this the same problem we were discussing in #195 (comment)?

We are in the "complicated zone" when importlib_metadata exists in path.

(Frustratingly, I tested the patch against the setuptools test suite and it passed at that point in time https://github.com/abravalheri/setuptools/pull/14/files)

@abravalheri
Copy link
Contributor

abravalheri commented Oct 7, 2024

That said, an egg-info directory containing no metadata is in fact invalid, right? Shouldn't the solution be to address the root cause (prevent that invalid metadata from existing)? What is the origin of these empty metadata directories?

@jaraco, I suspect that this parish aspect is an issue in setuptools because it uses entry-points to generate the .egg-info directory itself. So setuptools create an empty .egg-info folder, and then it fails while populating it (race-condition-ish unravelled by the change).

@abravalheri
Copy link
Contributor

abravalheri commented Oct 15, 2024

Hi @henryiii, in pypa/setuptools#4680 I am trying to change the code path so that the conflict between importlib.metadata and importlib_metadata is never reached. Does that solve the problems you initially reported?

You probably can test the PR specific version of setuptools by installing https://github.com/abravalheri/setuptools/archive/refs/heads/issue-pyproject-hooks-206-take2.zip

@hswong3i
Copy link

I could confirm this affecting Debian 11 (i.e. Python 3.9) and Ubuntu 20.04 (i.e. Python 3.8).

My OBS packages are most likely fine with setuptools v59.6.0 (for Python <= 3.7) and v75.2.0 (for Python >= 3.8), but only Debian 11 and Ubuntu 20.04 coming with compile error, see https://build.opensuse.org/project/monitor/home:alvistack?defaults=0&failed=1&arch_x86_64=1&repo_Debian_11=1&repo_xUbuntu_20_04=1:
image

This happened since pyproject-hooks 1.2 get released; I am now going to pin pyproject-hooks 0.13.1 for those legacy OS, and let's see if it could help the condition :-(

@henryiii
Copy link
Contributor Author

Have you tried updating setuptools? This should have been fixed there.

@hswong3i
Copy link

Have you tried updating setuptools? This should have been fixed there.

Already having setuptools v75.2.0 with Debian 11 and Ubuntu 20.04, but still failing :-(

@jaraco
Copy link
Member

jaraco commented Oct 28, 2024

@jaraco, I suspect that this parish aspect is an issue in setuptools because it uses entry-points to generate the .egg-info directory itself. So setuptools create an empty .egg-info folder, and then it fails while populating it (race-condition-ish unravelled by the change).

In that case, maybe the issue could be addressed in Setuptools by keeping the egg-info directory off sys.path until it's populated.

@hswong3i
Copy link

hswong3i commented Oct 28, 2024

@jaraco, I suspect that this parish aspect is an issue in setuptools because it uses entry-points to generate the .egg-info directory itself. So setuptools create an empty .egg-info folder, and then it fails while populating it (race-condition-ish unravelled by the change).

In that case, maybe the issue could be addressed in Setuptools by keeping the egg-info directory off sys.path until it's populated.

@jaraco just some quick update that may help: with alvistack/adrienverge-yamllint@3de73c2 I had try to git add --force *.egg-info* so embed the local pre-build egg-info directory into the source for packaging, but still fail with Debian 11 and Ubuntu 20.04. It seems running install_scripts step couldn't be triggered.

In case of Ubuntu 22.04 (see https://build.opensuse.org/package/live_build_log/home:alvistack/adrienverge-yamllint-1.35.1/xUbuntu_22.04/x86_64):

[   20s] byte-compiling /usr/src/packages/BUILD/debian/tmp/usr/lib/python3.10/dist-packages/yamllint/parser.py to parser.cpython-310.pyc
[   20s] byte-compiling /usr/src/packages/BUILD/debian/tmp/usr/lib/python3.10/dist-packages/yamllint/cli.py to cli.cpython-310.pyc
[   20s] byte-compiling /usr/src/packages/BUILD/debian/tmp/usr/lib/python3.10/dist-packages/yamllint/config.py to config.cpython-310.pyc
[   20s] running install_egg_info
[   20s] Copying yamllint.egg-info to /usr/src/packages/BUILD/debian/tmp/usr/lib/python3.10/dist-packages/yamllint-1.35.1.egg-info
[   20s] Skipping SOURCES.txt
[   20s] running install_scripts
[   20s] Installing yamllint script to /usr/src/packages/BUILD/debian/tmp/usr/bin
[   20s] find debian/tmp/usr/lib/python*/*-packages -type f -name '*.pyc' -exec rm -rf {} \;
[   20s] fdupes -qnrps debian/tmp/usr/lib/python*/*-packages
[   20s] make[1]: Leaving directory '/usr/src/packages/BUILD'
[   20s]    dh_install -O--buildsystem=pybuild
[   20s]    dh_installdocs -O--buildsystem=pybuild
[   20s]    dh_installchangelogs -O--buildsystem=pybuild
[   20s]    dh_systemd_enable -O--buildsystem=pybuild
[   20s]    dh_python3 -O--buildsystem=pybuild
[   21s]    dh_installinit -O--buildsystem=pybuild
[   21s]    dh_systemd_start -O--buildsystem=pybuild
[   21s]    dh_lintian -O--buildsystem=pybuild
[   21s]    dh_perl -O--buildsystem=pybuild
[   21s]    dh_link -O--buildsystem=pybuild
[   21s]    dh_strip_nondeterminism -O--buildsystem=pybuild
[   21s]    dh_compress -O--buildsystem=pybuild
[   21s]    dh_fixperms -O--buildsystem=pybuild
[   21s]    dh_missing -O--buildsystem=pybuild
[   21s]    dh_installdeb -O--buildsystem=pybuild
[   21s]    dh_gencontrol -O--buildsystem=pybuild
[   21s] dpkg-gencontrol: warning: Depends field of package yamllint: substitution variable ${shlibs:Depends} used, but is not defined
[   21s]    dh_md5sums -O--buildsystem=pybuild
[   21s]    dh_builddeb -O--buildsystem=pybuild
[   21s] dpkg-deb: building package 'yamllint' in '../yamllint_1.35.1-1_all.deb'.
[   21s]  dpkg-genbuildinfo -O../python-yamllint_1.35.1-1_amd64.buildinfo
[   21s]  dpkg-genchanges -O../python-yamllint_1.35.1-1_amd64.changes
[   22s] dpkg-genchanges: info: including full source code in upload
[   22s]  dpkg-source --after-build .
[   22s] dpkg-buildpackage: info: full upload (original source is included)

In case of Debian 11 (see https://build.opensuse.org/package/live_build_log/home:alvistack/adrienverge-yamllint-1.35.1/Debian_11/x86_64):

[   34s] byte-compiling /usr/src/packages/BUILD/debian/tmp/usr/lib/python3.9/dist-packages/yamllint/rules/commas.py to commas.cpython-39.pyc
[   34s] byte-compiling /usr/src/packages/BUILD/debian/tmp/usr/lib/python3.9/dist-packages/yamllint/rules/anchors.py to anchors.cpython-39.pyc
[   34s] byte-compiling /usr/src/packages/BUILD/debian/tmp/usr/lib/python3.9/dist-packages/yamllint/rules/octal_values.py to octal_values.cpython-39.pyc
[   34s] running install_egg_info
[   34s] Writing /usr/src/packages/BUILD/debian/tmp/usr/lib/python3.9/dist-packages/yamllint-1.35.1.egg-info
[   34s] find debian/tmp/usr/lib/python*/*-packages -type f -name '*.pyc' -exec rm -rf {} \;
[   34s] fdupes -qnrps debian/tmp/usr/lib/python*/*-packages
[   34s] make[1]: Leaving directory '/usr/src/packages/BUILD'
[   34s]    dh_install -O--buildsystem=pybuild
[   34s] dh_install: warning: Cannot find (any matches for) "usr/bin/*" (tried in ., debian/tmp)
[   34s] 
[   34s] dh_install: warning: yamllint missing files: usr/bin/*
[   34s] dh_install: error: missing files, aborting
[   34s] make: *** [debian/rules:15: binary] Error 25
[   34s] dpkg-buildpackage: error: fakeroot debian/rules binary subprocess returned exit status 2

@abravalheri
Copy link
Contributor

In that case, maybe the issue could be addressed in Setuptools by keeping the egg-info directory off sys.path until it's populated.

@jaraco I have already implemented something in setuptools>=75.1.1 that tries to solve that. Initially I was tempted to use a temporary dir and then rename it at the end similar to what you are suggesting (see pypa/setuptools#4670), but I went for a smaller change (call the importlib.metadata APIs before creating the directory, see pypa/setuptools#4680). We can revisit the more holistic approach in pypa/setuptools#4670 if necessary.


@hswong3i could you maybe try to reduce the reproducer to its most basic form using only setuptools APIs (maybe also pypa/build) without the extra Ubuntu/Debian stack on top of it? Please feel free to open an issue in the setuptools repository with the minimum reproducer. Version 75.1.1 should have solved the error related to importlib-metadata initially reported in #206 (comment). The error traces in #206 (comment) don't look very similar to the original problem.

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

No branches or pull requests

9 participants