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

typing installed unconditionally on all Pythons when installed from wheel #133

Closed
reaperhulk opened this issue Aug 4, 2020 · 12 comments
Closed

Comments

@reaperhulk
Copy link

reaperhulk commented Aug 4, 2020

hyperlink defines conditional requirements on install_requires in its setup.py

install_requires=["idna>=2.5", 'typing ; python_version<"3.5"'],

However, older setuptool cannot interpret this when building wheels so typing is installed unconditionally even on Python 3.5+. This is compounded by a pip isue where installed packages are improperly prioritized (pypa/pip#8272). To fix this you should read below so you don't get the wrong fix 😄

@bluetech
Copy link

bluetech commented Aug 4, 2020

To fix this you'll need to add a requires section to your pyproject.toml

Not sure if this will fix it but to add my 2 cents:

When I download the wheel of version 20.0.0 from PyPI (link: https://files.pythonhosted.org/packages/90/d0/37eb861a66061ec462c6f31ad2d4a2ebff7057c9dd17467e89665c97a834/hyperlink-20.0.0-py2.py3-none-any.whl) and manually unzip it, the hyperlink-20.0.0.dist-info/METADATA file has this:

Requires-Dist: typing

Note that the environment marker is missing. But when I build the wheel locally using pip wheel --no-deps . in a local virtualenv created with virtualenv, the same line is

Requires-Dist: typing ; python_version < "3.5"

So maybe the problem is that the wheel was generated with an old toolchain that doesn't understand the environment marker? I am not an expert but presumably doing pip install --upgrade pip wheel setuptools before building the wheel would fix it.

This currently breaks the pytest CI due to the pip bug @reaperhulk mentioned (ref: pytest-dev/pytest#7616). We will work around it but a fix in hyperlink would be best.

@Julian
Copy link
Contributor

Julian commented Aug 4, 2020

(Breaks the jsonschema one too, and a couple other ones of mine which I tried to diagnose this morning -- @bluetech if you share whatever intermediate workaround you come up with would definitely appreciate it so I don't duplicate the work myself later)

@Julian

This comment has been minimized.

@mahmoud
Copy link
Member

mahmoud commented Aug 4, 2020

Old toolchain is definitely a possibility. I'll see about a rebuild + rerelease today.

@wsanchez
Copy link
Contributor

wsanchez commented Aug 4, 2020

@reaperhulk I'm not following what needs to change in pyproject.toml and why. Note that we already have a requires build-system.section… are you saying that typing needs to be put in there with the Python version qualifiers?

@wsanchez
Copy link
Contributor

wsanchez commented Aug 4, 2020

@twm agrees that new tooling fixes this.

@Julian
Copy link
Contributor

Julian commented Aug 4, 2020

If it helps now or after another release using the existing release mechanism hyperlink uses, feel free to steal https://github.com/Julian/jsonschema/blob/master/.github/workflows/packaging.yml to do releases straight out of GH Actions. Happy of course to send a PR with it too (now or after another release)

@twm
Copy link
Collaborator

twm commented Aug 4, 2020

Yeah I've hit the same issue (with typing, even). You need a non-ancient setuptools to use modern syntax like:

setup(
    # ...
    install_requires=[
        'typing; python_version < "3.5"',
    ],
)

I specify a newish minimum version in pyproject.toml to at least document this:

[build-system]
requires = [
    "setuptools>=41.2.0",
    "wheel",
]
build-backend = "setuptools.build_meta"

I'm not sure what the actual minimum is, but my notes indicate setuptools 20-ish is too old.

With older setuptools you had to use the old syntax:

setup(
    # ...
    extras_require={
        ':python_version < "3.5"': ['typing'],
    },
)

@reaperhulk
Copy link
Author

@wsanchez Yeah I'm sure I'm wrong about the fix here 😄

@mahmoud
Copy link
Member

mahmoud commented Aug 5, 2020

It's like a party in here! By all means keep it going, but v20.0.1 is released, and I checked the wheel METADATA (thanks @bluetech!) before uploading, so I'm pretty sure the party'll have to continue in a closed issue 🎉

PS Missin y'all! See you next release/PyCon, whichever comes first. :)

@Julian
Copy link
Contributor

Julian commented Aug 5, 2020

Woo! Release partayy

@wsanchez
Copy link
Contributor

wsanchez commented Aug 6, 2020

Thanks @mahmoud !

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

6 participants