rougier / freetype-py

Python binding for the freetype library
Other
298 stars 88 forks source link

Invalid setuptools_scm entry in setup.py #128

Closed AndyMender closed 3 years ago

AndyMender commented 3 years ago

I'm not sure whether this worked before, but according to the setuptools_scm README, setuptools_scm should be listed in the setup_requires field of the setuptools.setup call without the toml key. Running setup.py results in the following traceback:

[amender@localhost freetype-py-2.2.0]$ /usr/bin/python3 setup.py build '--executable=/usr/bin/python3 -s'
# Will use the system-provided FreeType.
Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/pkg_resources/__init__.py", line 2741, in requires
    deps.extend(dm[safe_extra(ext)])
KeyError: 'toml'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "setup.py", line 103, in <module>
    setup(
  File "/usr/lib/python3.8/site-packages/setuptools/__init__.py", line 144, in setup
    _install_setup_requires(attrs)
  File "/usr/lib/python3.8/site-packages/setuptools/__init__.py", line 139, in _install_setup_requires
    dist.fetch_build_eggs(dist.setup_requires)
  File "/usr/lib/python3.8/site-packages/setuptools/dist.py", line 717, in fetch_build_eggs
    resolved_dists = pkg_resources.working_set.resolve(
  File "/usr/lib/python3.8/site-packages/pkg_resources/__init__.py", line 794, in resolve
    new_requirements = dist.requires(req.extras)[::-1]
  File "/usr/lib/python3.8/site-packages/pkg_resources/__init__.py", line 2743, in requires
    raise UnknownExtra(
pkg_resources.UnknownExtra: setuptools-scm 3.3.3 has no such extra feature 'toml'

Below patch fixes the issue:

diff --git a/setup.py b/setup.py
index 7844173..46384eb 100644
--- a/setup.py
+++ b/setup.py
@@ -127,5 +127,5 @@ setup(
         'Topic :: Multimedia :: Graphics',
     ],
     keywords=['freetype', 'font'],
-    setup_requires=['setuptools_scm[toml]'],
+    setup_requires=['setuptools_scm'],
 )
anthrotype commented 3 years ago

aargh, it probably changed recently because it definitely used to work. That's why somewhere in the setuptools docs they recommend to never drop extras_require entries if later on an extra set of dependencies becomes required, but always keep it with an empty list value so that it still works as no-op without breaking all the clients that use the old extra..

AndyMender commented 3 years ago

The change was made here: https://github.com/rougier/freetype-py/commit/f09dd56b21ae1779161baef17af582d6b221f227#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7R130

anthrotype commented 3 years ago

wait -- I see toml is still among the setuptools_scm extras_require (this is current master): https://github.com/pypa/setuptools_scm/blob/303defef04aa1a60dc74a4e52fef0bcf05c3d6be/setup.cfg#L42-L43

so I don't know why it isn't working for you. Are you using the latest pip and setuptools?

anthrotype commented 3 years ago

anyway, i think setuptools_scm[toml] is only needed for the new PEP517 pyproject.toml setup, not for the old setup_requires approach. So like @AndyMender suggested, the fix is to replace it with plain setuptools_scm

AndyMender commented 3 years ago

wait -- I see toml is still among the setuptools_scm extras_require (this is current master): https://github.com/pypa/setuptools_scm/blob/303defef04aa1a60dc74a4e52fef0bcf05c3d6be/setup.cfg#L42-L43

so I don't know why it isn't working for you. Are you using the latest pip and setuptools?

I'm using the following:

pip 19.3.1 from /usr/lib/python3.8/site-packages/pip (python 3.8)
setuptools 41.6.0 from /usr/lib/python3.8/site-packages/setuptools (python 3.8)
setuptools-scm 3.3.3 from /usr/lib/python3.8/site-packages/setuptools_scm (python 3.8)

The problem might actually be setuptools_scm, however that's the latest available in Fedora 32 and that's also the version I'm using to test the RPM build process locally before pushing the updated package to Fedora.

HinTak commented 3 years ago

Fedora 33 ships 4.1.2 apparently https://koji.fedoraproject.org/koji/packageinfo?packageID=21210

AndyMender commented 3 years ago

Fedora 33 ships 4.1.2 apparently https://koji.fedoraproject.org/koji/packageinfo?packageID=21210

And it builds cleanly in Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=56205224 I should've checked that first or in mock, I guess. Apologies!

Since Fedora 32 is being phased out, I'll remove the patch (lines in the original message) I added to the SPEC file already and resubmit to Fedora 34/Rawhide and Fedora 33 only. Then, we can close this here ticket.

Thoughts? :)

HinTak commented 3 years ago

Fedora's setuptools_scm package for 4.1.2 has a +toml sub-package: https://koji.fedoraproject.org/koji/rpminfo?rpmID=22407617 . Maybe an alternative approach to the patch is to add a "BuildRequire"?

AndyMender commented 3 years ago

Fedora's setuptools_scm package for 4.1.2 has a +toml sub-package: https://koji.fedoraproject.org/koji/rpminfo?rpmID=22407617 . Maybe an alternative approach to the patch is to add a "BuildRequire"?

Adding a BuildRequires on python3-setuptools_scm was actually sufficient.

I will revert my patch in the SPEC file and push an update.

Thanks a lot for the super quick help! :+1: