[PEP - Wheel Variant] Staging before push to upstream

[DEKHTIARJonathan]

No description provided.

[charliermarsh]

Nit: Do you mind using [email protected] (if not [email protected])?

[mgorny]

I'm fine with either, I think I've copied that from your commit messages. Just tell me which one you prefer (or "suggest" a change).

[charliermarsh]

Because that "one dependency" is, presumedly, the Python package containing the provider itself?

[mgorny]

Yes. It's basically "empty requires make no sense because then you don't have anything to call".

[rgommers]

Very nice and detailed review @emmatyping-nv. One remark on it:

It'd be good to differentiate installer from resolver and be consistent about that throughout the text.

This doesn't look like a good idea to me. A resolver isn't a standalone tool/package/thing from a packaging perspective, it's a component inside an installer. The relevant tools are "build backend", "build frontend", "installer". The term "resolver" doesn't seem used at all right now except for in a single section under Rejected Ideas, which seems fine.

[mgorny]

I've already replied to specific comments, but I think these two points are worth discussing top-level:

It'd be good to differentiate installer from resolver and be consistent about that throughout the text.

I think the PEP as it stands right now roughly splits tools into "installers" and "everything else". The boundary between the two is a bit blurry, and I don't think we're really following it consistently.

The primary idea behind "installers" is that these are packages that need to be able to determine whether variant wheels are supported, and may be executed with elevated privileges. Therefore, from the security PoV there is a risk that variant wheels would require installing and running arbitrary packages with these privileges. The "everything else" is basically tools where we don't worry about that — e.g. tools that only need to process wheels without determining whether they are supported, or build tools that install and run arbitrary packages anyway.

I suppose "installers" is a bit of a misnomer, since there are technically other tools that need to determine whether wheels can be installed. However, I don't think "resolver" is a good replacement. I mean, saying that "resolver should vendor a plugin" is weird — the installer does that, resolver is merely one of its components. Furthermore, the same applies to cases where resolver isn't really used, e.g. when installing a wheel from the local filesystem.

I'm open to improving things, but I don't really have a better term than "installer", and I'm not sure if using a strictly more correct term won't make things harder to comprehend.

In the overview of the specification, there is a lot of discussion of mitigations and security measures that should be taken, but no discussion of what the security risks are and what the threat model is. People will want to see an acknowledgement of the risks, probably more than the current Security implications section.

Indeed. Probably the "security considerations" section needs to be rewritten to make the problem clearer. I wonder if it would make sense to move it prior to specification; describe the problem there, then the measures taken in the specification part.

[emmatyping-nv]

A resolver isn't a standalone tool/package/thing from a packaging perspective, it's a component inside an installer. The relevant tools are "build backend", "build frontend", "installer".

Since resolvers are part of other tools like lockfile generators (e.g. pip-tools), I view resolvers as a component shared between installers and lockfile generators. Resolvers are used elsewhere too (e.g. tools to check licenses of various dependencies, etc.).

I think the PEP as it stands right now roughly splits tools into "installers" and "everything else". The boundary between the two is a bit blurry, and I don't think we're really following it consistently.

Yeah I think my main point is that the PEP makes specifications about package resolution, but tools other than installers do that.

However, I don't think "resolver" is a good replacement. I mean, saying that "resolver should vendor a plugin" is weird — the installer does that, resolver is merely one of its components.

One thing we could do is define "resolvers" as "tools which has a package resolution step". Or even use "resolver tools" (but maybe that is verbose).

Indeed. Probably the "security considerations" section needs to be rewritten to make the problem clearer. I wonder if it would make sense to move it prior to specification; describe the problem there, then the measures taken in the specification part.

I agree this seems like an improvement! Generally people don't move the sections around, but I think that should be fine since it is important to the flow of the PEP. We could also have a security section in the rationale I suppose.

[mgorny]

I'll prepare an update for the security-related stuff tomorrow.

[konstin]

Adding some notes, filed PRs for some others.

[konstin]

The abstract is already long, I'd cut the paragraph below

[konstin]

nit: Drop the "many more", three examples is good

[konstin]

The problem here is really that it's impossible to express any or different x86_64 versions, so you have to pick an oldest version you support and ship only that.

[konstin]

Suggested change

	As illustrated by
	`archspec <https://github.com/archspec/archspec>`__’s authors and
	maintainers, the ability to optimize a package for a specific
	architecture can lead to significant performance improvement (and often
	also reduce power requirements, etc.).
	As illustrated by
	`archspec <https://github.com/archspec/archspec>`__, the ability to
	optimize a package for a specific architecture can lead to a
	significant performance improvement.

[konstin]

What's the GROMACS benchmark, and what do the numbers refer to? The figure description should contain a concise description of what is being measured, just enough for a Python programmer who hasn't heard GROMACS before to understand the impact.

[konstin]

I'd drop these three paragraphs, this information should emerge from the description of the workarounds, and should otherwise already be understood by the PEP reader.

[konstin]

I would replace this with it being error prone. At least I haven't heard complaints about this being too much effort to document, but i've seen many cases in which those custom workarounds broke, from the pytorch index missing hashes over nvidia packages downloading the wrong artifact on the tooling side to so many complaints about installing GPU packages on the user side.

[konstin]

We haven't introduces SMs at this point

[konstin]

Suggested change

	// TODO: replace this with appendix URL for the PEP
	// The schema URL will be replaced with the final URL on packaging.python.org

[konstin]

We treat the variants json file like an archive file (not like a .metadata file), it's a regular top level enty, including having a hash. I'll file a PR.