Conversation
Key takeaways: 1. Vendoring is no longer presented as the primary recommendation. Rather, it is given as something installers are allowed to do, much like reimplementing. 2. UI recommendations were removed, and even more emphasis has been put on leaving the exact trust mechanism "implementation-specific". 3. I've volunteered that we start a dedicated team to maintain the allowlist of providers, but kept it in the "rationale" section, so we don't get into governance area too deeply. I think it's an acceptable compromise that satisfies maintainer's concerns that they don't want to be responsible for maintaining their own list. 4. Put even more emphasis that opt-in is likely to cause security issues (it was already there). Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Co-authored-by: konsti <konstin@mailbox.org>
Co-authored-by: konsti <konstin@mailbox.org>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Given that people have been complaining that it's both... Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
|
I suggest adding a |
Should that list just the actual specification changes, or also rationale clarifications? |
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
peps/pep-0817.rst
Outdated
| Publishing variant wheels on an index | ||
| ''''''''''''''''''''''''''''''''''''' | ||
|
|
||
| An index with full support for variant wheels should ideally generate |
There was a problem hiding this comment.
My preference is generating the file locally, and uploading it similar to the other artifacts. This avoids modifying the {name}-{version}-variants.json on the index, allowing to hash it on the client.
I don't really see the case of adding additional builds to a release, the best practice is releasing from CI and having CI build all the wheels.
There was a problem hiding this comment.
I think index-level support will lower the barrier for adoption. Besides, at least some CI workflows upload individual wheels separately. Generating the file locally would require people to recombine them first.
Modification here is merely a matter of a single upload session. Perhaps I could make this less ambiguous if I suggest that it can either be updated on-the-fly as wheels are uploaded or after the upload session is closed.
There was a problem hiding this comment.
This would be solved by PEP 694, but that's coming along slowly. The part I'm worried about is that updating the file on the index means that we can't put a hash of the variants.json in the lockfile (which we use for reproducibility and security, as something that's a widely understood by developers), and that without the file being fixed, we'd have to do cache revalidation for it.
There are solutions such as checking whether the version of the file you have has an entry for the wheel your looking at, but those seem complex in comparison to twine or uv publish generating the file.
We can also punt on making a decision here, or put it in open questions, this doesn't seem like a fail or pass question for the overall design.
There was a problem hiding this comment.
I don't think that's really that much of a problem: in general, I dare say the upload window should be relatively narrow. If people really lock onto the partial variants file (which is a problem in itself), they just need to update the hashes, right?
I mean, it's the same kind of problem as if you locked into the package before *-variants.json were uploaded, and therefore ended up using the regular wheel.
There was a problem hiding this comment.
An open question and a brief mention in that open question of PEP 694 seems reasonable to me.
This partial locking will happen enough for it to matter. We saw the same kind of issue when uploading sdist before wheels. Very quick calculation: current download rate for numpy is >300 wheels per second. So a few minutes delay is still a lot of potential users.
There was a problem hiding this comment.
For now, I'm going to rephrase this section to make the risk much clearer, and look at other sections where we mention these files.
rgommers
left a comment
There was a problem hiding this comment.
I reviewed the section "Installing a package from remote index" just now. The logic all looks good to me, and matches the diagram I posted. I just made a few textual suggestions for clarity. And also I agree with @konstin's open comments on that section.
Co-authored-by: konsti <konstin@mailbox.org>
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
peps/pep-0817.rst
Outdated
| Publishing variant wheels on an index | ||
| ''''''''''''''''''''''''''''''''''''' | ||
|
|
||
| An index with full support for variant wheels should ideally generate |
There was a problem hiding this comment.
This would be solved by PEP 694, but that's coming along slowly. The part I'm worried about is that updating the file on the index means that we can't put a hash of the variants.json in the lockfile (which we use for reproducibility and security, as something that's a widely understood by developers), and that without the file being fixed, we'd have to do cache revalidation for it.
There are solutions such as checking whether the version of the file you have has an entry for the wheel your looking at, but those seem complex in comparison to twine or uv publish generating the file.
We can also punt on making a decision here, or put it in open questions, this doesn't seem like a fail or pass question for the overall design.
rgommers
left a comment
There was a problem hiding this comment.
Overall this looks great, I only have a few localized suggestions. All the added content works well.
Not yet changed in this PR yet, but let's fix this: in the Out of Scope section, it says "or include variants in a pylock.toml file," which led to some confusion. @konstin had suggested an alternative already.
| Change history | ||
| ============== | ||
|
|
||
| - TBD |
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
Signed-off-by: Michał Górny <mgorny@quansight.com>
|
Updated. |
| 1. Query the remote index for the package in question. | ||
| 2. Initially select a package version meeting the version constraints, | ||
| as usual (this does not take variant metadata into account). | ||
| 3. Filter available wheels based on Platform Compatibility Tags. |
There was a problem hiding this comment.
| 3. Filter available wheels based on Platform Compatibility Tags. | |
| 3. **[Unchanged]** Filter available wheels (for resolved version) based on Platform Compatibility Tags. |
There was a problem hiding this comment.
PEPs use bold text sparingly, if at all - We need to follow the PEP style of writing!
| as usual (this does not take variant metadata into account). | ||
| 3. Filter available wheels based on Platform Compatibility Tags. | ||
| 4. Determine if any of the remaining wheels are variant wheels. | ||
| If not, proceed as with non-variant wheels. |
There was a problem hiding this comment.
There should be a If not, proceed as with non-variant wheels and JUMP TO STEP XYZ
There was a problem hiding this comment.
That's an exit branch, the procedure for non-variant wheels is already well-known to the reader, it's well-documented in the existing specs and implemented by a number of tools.
| 3. Filter available wheels based on Platform Compatibility Tags. | ||
| 4. Determine if any of the remaining wheels are variant wheels. | ||
| If not, proceed as with non-variant wheels. | ||
| 5. If any wheels feature variant labels, download the index-level |
There was a problem hiding this comment.
| 5. If any wheels feature variant labels, download the index-level | |
| 5. **[New Variant Step]** If any wheels feature variant labels, download the index-level |
| When asked to install a version of a package from an index, the proposed | ||
| behavior would be to: | ||
|
|
||
| 1. Query the remote index for the package in question. |
There was a problem hiding this comment.
| 1. Query the remote index for the package in question. | |
| 1. **[Unchanged]** Query the remote index for the package in question. |
I think it's nice to provide - here no modification, etc.
| behavior would be to: | ||
|
|
||
| 1. Query the remote index for the package in question. | ||
| 2. Initially select a package version meeting the version constraints, |
There was a problem hiding this comment.
| 2. Initially select a package version meeting the version constraints, | |
| 2. **[Unchanged]** Resolve the package version meeting the version constraints, |
I would replace by smthg along the line of "resolve the package exactly as normal - no change expected".
- use the word:
resolve - state very loudly & clearly => variant has no impact on package version resolution
| 9. If multiple wheels for a given version share the same variant label, | ||
| order them by Platform compatibility tags and build number, and | ||
| select the best wheel. | ||
|
|
There was a problem hiding this comment.
- [Unchanged] Proceed with installation of the best matching and selected wheel (variant or not).
| 1. If no variant label is present in the filename, proceed as with | ||
| non-variant wheels. | ||
| 2. Verify the wheel compatibility via Platform compatibility tags. | ||
| 3. Read variant metadata from ``*.dist-info/variant.json`` inside the |
There was a problem hiding this comment.
Important !
I think it's a bad idea honestly - I'm with you that "this should be ideal". However ... People will massively object.
If someone elects to manually install ABC => just give it to them as long as the platform tags are valid. If they make a mistake it's on them.
It's breaking a core assumption of the installer that if I'm doing pip install package.whl now I have to install a provider first ... And this will be VERY difficult to win ... Let's just drop this...
You ask A => You get A. If you mess up, that's on you
There was a problem hiding this comment.
This is a tool decision, this overview list show the behavior of how tools act today already and is the default that at least uv will implement (we will reject non-matching variants by default).
$ pip install numpy-2.4.2-pp311-pypy311_pp73-win_amd64.whl
ERROR: numpy-2.4.2-pp311-pypy311_pp73-win_amd64.whl is not a supported wheel on this platform.
$ uv pip install numpy-2.4.2-pp311-pypy311_pp73-win_amd64.whl
× No solution found when resolving dependencies:
╰─▶ Because only numpy==2.4.2 is available and numpy==2.4.2 has no wheels with a matching Python version tag
(e.g., `cp314`), we can conclude that all versions of numpy cannot be used.
And because you require numpy, we can conclude that your requirements are unsatisfiable.
| Publishing variant wheels on an index | ||
| ''''''''''''''''''''''''''''''''''''' | ||
|
|
||
| Variant wheels are uploaded to an index just like regular wheels. |
There was a problem hiding this comment.
| Variant wheels are uploaded to an index just like regular wheels. | |
| **Variant wheels are uploaded to an index just like regular wheels.** | |
Added line break is intentional
|
|
||
| 1. For the first variant wheel for a given package version, copy the | ||
| data from its ``*.dist-info/variant.json`` file. | ||
| 2. For subsequent wheels, merge the data from their |
There was a problem hiding this comment.
Add that variantlib exposes all the tooling to do this step in one API call.
Per another bikeshed, I've tried to roughly reword stuff to address people's concerns without actually weakening any of the points we've made.
(to be retargeted at an appropriate branch when we have one)
Note there's also more work added on top, see commit messages for a quick summary.