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

Publish: Warn about unnormalized filenames #8203

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

konstin
Copy link
Member

@konstin konstin commented Oct 15, 2024

The spec requires normalizing the name and the version in distribution filenames (https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode). Missing normalization caused #8030, so we warn about it. Ideally, we would error, but this would break setuptools (pypa/setuptools#3777)

@konstin konstin added enhancement New feature or improvement to existing functionality preview Experimental behavior labels Oct 15, 2024
@charliermarsh
Copy link
Member

I'm torn on this because as far as I can tell there isn't any remedy available to the user?

@konstin
Copy link
Member Author

konstin commented Oct 15, 2024

I believe it's the only responsible course of action: It would be irresponsible to publish invalid wheels without comment, but given the setuptools bug, we can't make it an error yet.

The spec requires normalizing the name and the version in distribution filenames (https://packaging.python.org/en/latest/specifications/binary-distribution-format/#escaping-and-unicode). Missing normalization caused #8030, this PR adds a warning for it.
@konstin konstin force-pushed the konsti/warn-unnormalized-wheel-filenames branch from 3613c09 to 14dcee9 Compare October 15, 2024 12:24
@charliermarsh
Copy link
Member

So what's the exact course of events here. The user has a package with a dot in the name, setuptools creates a wheel with an invalid name (includes the dot), PyPI accepts it, and then what?

@Avasam
Copy link

Avasam commented Oct 15, 2024

What about incorrect capitalization? #8030 (comment)

Should that also be tested for for completeness ?

@Avasam
Copy link

Avasam commented Oct 15, 2024

I'm torn on this because as far as I can tell there isn't any remedy available to the user?

When I first hit the issue, my publish command failed and I was able to manually fix the folder name in the whl file to be normalized.

@konstin
Copy link
Member Author

konstin commented Oct 15, 2024

Currently, PyPI accepts the legacy form with unnormalized wheel filenames. It seems to just split wheel filenames on dashes (https://github.com/pypi/warehouse/blob/50a58f3081e693a3772c0283050a275e350004bf/warehouse/forklift/legacy.py#L1133-L1155). This is a gap in PyPI's wheel checking. PyPI regularly tightens their checks to follow the standards more closely, e.g. pypi/warehouse#15795 fixed the check for the version in source dist filenames. We're a bit ahead of the curve, but I'd rather emit warnings now than to quietly publish spec-breaking wheels and/or wait until PyPI starts rejecting such packages. I'd hope that setuptools fixes this on their end, which should unblock PyPI from adding such a check.

Should that also be tested for for completeness ?

Good point, I've added a dedicated test case.

My publish command failed and I was able to manually fix the folder name in the whl file to be normalized.

I've added a workaround in #8204 that should fix publishing without manual edits. This PR is basically adding a mechanism of complaining that what #8204 does is a hack around missing standards compliance in the build backend.

@charliermarsh
Copy link
Member

We're a bit ahead of the curve, but I'd rather emit warnings now than to quietly publish spec-breaking wheels and/or wait until PyPI starts rejecting such packages. I'd hope that setuptools fixes this on their end, which should unblock PyPI from adding such a check.

But when PyPI accepts such a wheel, what's the impact on consumers of the package?

@konstin
Copy link
Member Author

konstin commented Oct 15, 2024

Currently, there is no impact that I'd know of, since all resolver and installers I'm aware of accept the legacy style, too (there are packages that were publish before these normlization rules were written, and the spec says tools need to take care of this legacy style). I had hoped that following the spec would be sufficient grounds to add a warning.

pnasrat pushed a commit to pnasrat/uv that referenced this pull request Oct 21, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [windows-sys](https://togithub.com/microsoft/windows-rs) |
workspace.dependencies | minor | `0.52.0` -> `0.59.0` |

---

### Release Notes

<details>
<summary>microsoft/windows-rs (windows-sys)</summary>

###
[`v0.59.0`](https://togithub.com/microsoft/windows-rs/releases/tag/0.59.0)

[Compare
Source](https://togithub.com/microsoft/windows-rs/compare/0.52.0...0.59.0)

This release includes an update to the
[windows-sys](https://crates.io/crates/windows-sys) crate only. The
`windows-sys` crate is updated very infrequently and only when there is
an explicit need to do so. The 0.59.0 release includes a rollup of API
fixes, updates, and additions since the
[0.52.0](https://togithub.com/microsoft/windows-rs/releases/tag/0.52.0)
release nine months ago. Notably:

- This update introduces support for Arm64EC
([#&astral-sh#8203;2957](https://togithub.com/microsoft/windows-rs/issues/2957))
- Updated bindings for the latest APIs
https://github.com/microsoft/windows-rs/tree/0.59.0/crates/libs/bindgen/default
- Derive standard traits
([#&astral-sh#8203;3041](https://togithub.com/microsoft/windows-rs/issues/3041))
-   Updates to code generation to handle newer Rust warnings and lints
- Overall smaller crate and more efficient code gen to reduce build time
- Support for feature search
https://microsoft.github.io/windows-rs/features/#/0.59.0
-   MSRV is updated to 1.60

**Full Changelog**:
microsoft/windows-rs@0.52.0...0.59.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 4am on Monday" (UTC),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/astral-sh/uv).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM4LjU2LjAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbImludGVybmFsIl19-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement to existing functionality preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants