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

Updates for PEP 566 #231

Closed
di opened this issue Mar 15, 2018 · 12 comments
Closed

Updates for PEP 566 #231

di opened this issue Mar 15, 2018 · 12 comments
Milestone

Comments

@di
Copy link
Member

di commented Mar 15, 2018

I didn't think wheel needed any additional updates to support PEP 566 , but over at pypa/setuptools#1286, @brainwane pointed out that wheel creates a DESCRIPTION.rst file.

It seems like if the user is providing Description-Content-Type, wheel should output the correct extension, (e.g. DESCRIPTION.md) instead.

However, I'm not really sure what this file is used for (especially because the Description is included as the message body in METADATA as well) so I'm hesitant to propose to change it in case some downstream tool is expecting DESCRIPTION.rst to exist.

Why does wheel create this file? Is it reasonable to change the extension if we know it to be incorrect?

@brainwane
Copy link
Contributor

A wheel I just built (project page, file) had "Metadata-Version: 2.0" at the top of METADATA; in this case, it should have been version 2.1.

I was using setuptools 38.6.0, twine 1.11.0rc1, and pkginfo 1.4.2.

Relevant contents of METADATA (via wheel unpack), which included a Description-Content-Type and Provides-Extra lines:

Metadata-Version: 2.0
Name: Forms990-analysis
Version: 1.0.8
Description-Content-Type: text/markdown
Provides-Extra: dev
Requires-Dist: arrow
Provides-Extra: dev
Requires-Dist: requests; extra == 'dev'
Requires-Dist: six; extra == 'dev'
Requires-Dist: bpython; extra == 'dev'

Contents of WHEEL:

Wheel-Version: 1.0
Generator: bdist_wheel (0.30.0)
Root-Is-Purelib: true
Tag: py3-none-any

@brainwane
Copy link
Contributor

Probably because METADATA is used by twine, when I use twine to upload a wheel that includes PEP 566 metadata to PyPI, Warehouse does not render Markdown long_descriptions properly. The release history of this sample project may be helpful in demonstrating and investigating this.

@di
Copy link
Member Author

di commented Mar 19, 2018

I'm going to propose the following changes:

  • Detect Metadata 2.1 fields and set Metadata-Version to 2.1 if present, otherwise leave it at 2.0
  • stop writing a DESCRIPTION.rst entirely, because it's redundant, seems old (added > 5 years ago) and I can't find anything that is using it.

@agronholm Any thoughts on this before I make the PR?

@agronholm
Copy link
Contributor

Hm, seems that I do not get any email notifications for new issues. I only got the notification now that you directly mentioned me. Anyway, I have a refactoring in progress for this, but seeing as how I'm not the original author, some help would be welcome. I'm also tackling one of my pet peeves here, namely the presence of metadata.json and the code that generates it. Wheel even generates extra requirements from the [metadata] section of setup.cfg which has not been documented anywhere as far as I'm aware. I'm throwing all that overboard too.

@agronholm
Copy link
Contributor

Also, as 2.0 is not listed among valid metadata versions, I would prefer setting it to 2.1 unconditionally.

@di
Copy link
Member Author

di commented Mar 19, 2018

@agronholm Great to hear. Setting 2.1 unconditionally sounds good to me.

Would you be open to me making these changes sooner, rather than waiting for your refactor to be done? Currently (as @brainwane mentioned) since twine prefers uploading wheels first, and since Warehouse only sets release metadata from the first distribution uploaded, using Description-Content-Type is broken if the user builds both a wheel and an sdist for a release.

@agronholm
Copy link
Contributor

How about this – if this is needed really soon and you're ready to write the code, I will welcome your PR and add my own changes before the next release.

Are you on #pypa-dev on Freenode btw?

@di
Copy link
Member Author

di commented Mar 19, 2018

@agronholm sounds good. Yes, I'm di_codes.

@testworksau
Copy link

testworksau commented Apr 20, 2018

@di @agronholm

Just a quick note - removing these files has broken Artifactory. I'm not sure whether other tools rely on this data too.

https://www.jfrog.com/jira/browse/RTFACT-16360

We've had to pin our wheel version to 0.30 for now.

@di
Copy link
Member Author

di commented Apr 20, 2018

@testworksau Yes, I think JFrog is already aware that Artifactory does not support Metadata 2.1. As far as I know, they're working on it, but pinning the wheel version back for now when building for Artifactory should be totally fine.

@agronholm
Copy link
Contributor

They should never have relied on that data as it was not based on any accepted PEP.

@edo248
Copy link

edo248 commented Apr 25, 2018

@agronholm Their actual problem is the version 2.1 (as with metadata.json, this version was never standard). Here's ticket: https://www.jfrog.com/jira/browse/RTFACT-16189

Not sure whether they could have prevented it, but seem like we lived with non-standard version 2.0 for >5 years so them relying on that was normal. 2.0 was a de-facto standard (although in deferred status).

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

5 participants