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

Change the hashes in the installation report to be a mapping #11312

Closed
1 task done
pradyunsg opened this issue Jul 27, 2022 · 23 comments · Fixed by #11679
Closed
1 task done

Change the hashes in the installation report to be a mapping #11312

pradyunsg opened this issue Jul 27, 2022 · 23 comments · Fixed by #11679
Labels
C: direct url Direct URL references (PEP 440, PEP 508, PEP 610) C: inspect The inspect command C: install report pip install --report type: feature request Request for a new feature

Comments

@pradyunsg
Copy link
Member

What's the problem this feature will solve?

Currently, pip install --report generates hash information that looks like:

        "archive_info": {
          "hash": "sha256=18f3e912f9ad1bdec27fb06b8198a2ccc32f201e24174cec1b3424dda605a310"
        }

This means that we can't show hash information with multiple algorithms easily.

Describe the solution you'd like

Change archive information into being a mapping of hashes, similar to PEP 691:

      "hashes": {"sha256": "...", "blake2b": "..."},

We can put this key in archive_info, and replace the existing hash key with it.

Alternative Solutions

Not really. The other option is doing something like adding & as a separator between hashes, but that sounds like a horrible thing to do given the alternatives we have and that we're allowed to make backwards incompatible changes right now.

Additional context

N/A

Code of Conduct

@pradyunsg pradyunsg added type: feature request Request for a new feature S: needs triage Issues/PRs that need to be triaged labels Jul 27, 2022
@pradyunsg
Copy link
Member Author

/cc @sbidoul

@uranusjr
Copy link
Member

For an alternative solution (not suggesting we should do it instead), we can make hashes a list of algo=hashval strings.

@pradyunsg
Copy link
Member Author

It's a mapping -- let's store it as one. I expect that users will compose one anyway, regardless of what format we spew it out in. ([s.split("=") for s in li] vs di.items(), di.get(algo) vs {alternative too difficult to code-golf})

@sbidoul
Copy link
Member

sbidoul commented Jul 27, 2022

Yeah, I've had that in mind too.

I think that should be done as an evolution of PEP 610.

I'm under the impression that PyPI only returns one hash type, though ?

@uranusjr
Copy link
Member

The JSON API returns multiple hashes.

@sbidoul sbidoul added resolution: needs standard Should be agreed as a standard before implementation and removed S: needs triage Issues/PRs that need to be triaged labels Jul 27, 2022
@sbidoul
Copy link
Member

sbidoul commented Jul 27, 2022

The JSON API returns multiple hashes.

It is foreseen in the PEP 691 API yes, but in practice pypi.org returns sha256 only.

Anyway I agree this should be done, I'm not sure about the urgency.

@sbidoul sbidoul added C: install report pip install --report C: inspect The inspect command C: direct url Direct URL references (PEP 440, PEP 508, PEP 610) labels Jul 27, 2022
@pradyunsg
Copy link
Member Author

Let's do this before we stabilise the format.

@pradyunsg pradyunsg removed the resolution: needs standard Should be agreed as a standard before implementation label Jul 27, 2022
@pradyunsg
Copy link
Member Author

I've removed the needs standard thing, because it's already technically possible, through existing standards, to have multiple hashes from multiple algorithms.

@sbidoul
Copy link
Member

sbidoul commented Jul 27, 2022

I've removed the needs standard thing, because it's already technically possible, through existing standards, to have multiple hashes from multiple algorithms.

Yes but no :) The standard used for the download_info field is PEP 610 and it is important to keep it so because

  • in some case, they are direct URLs,
  • and sometimes it comes from origin.json in the wheel cache for which we use the direct URL format too,
  • and because the closer we are to documented interoperability standards, the easier it will be to consume the format.

So let's do the standard update work first, please. It should be relatively easy: add the hashes field you propose and write up the SHOULD/MUST language to address backward compatibility concerns for consumers and producers.

@pradyunsg
Copy link
Member Author

pradyunsg commented Jul 27, 2022

I'm very confused. The pip installation report format is explicitly noted to be a pip-only non-interoperability standard format.

Why is a PEP/standard process involved in changing it?

@pradyunsg
Copy link
Member Author

I can understand that if we wanted to extend the download info key in the standard, to allow storing the metadata in that format there -- but we're not blocked on that. Like, literally, we can add a dict([s.split("=")]) in the pip-specific format, and close this issue. And, come around to dealing with multiple hashes later.

Sure, that's not the best way to do things, but my point is that we don't have to wait on a standardisation process and this is firmly a detail we can change today.

@pfmoore
Copy link
Member

pfmoore commented Jul 27, 2022

I agree. I think we should future-proof the pip-only format now, before people start relying on it, and then we're prepared should PEP 610 get extended later. And if PEP 610 never gets extended, we've lost nothing and we've gained a small amount of user friendliness (we do the .split("=") so you don't have to 🙂)

@sbidoul
Copy link
Member

sbidoul commented Jul 27, 2022

Of course we could do without changing PEP 610.

Yet let me add these arguments:

  • the installation report is not an interoperability standard but it composes interoperability standards to ease consumption (as well as production)
  • download_info is explicitly documented as being PEP 610 compliant
  • diverging from it will make the implementation and documentation much more complex
  • it will make it harder to consume for our users
  • the fact that PEP 610 supports a single hash only is clearly an oversight

I'll try to prepare an amendment PEP 610 and I'll see how well it fits.

@pradyunsg
Copy link
Member Author

I'll try to prepare an amendment PEP 610 and I'll see how well it fits.

A very gentle nudge on this. :)

@sbidoul
Copy link
Member

sbidoul commented Sep 30, 2022

Hi @pradyunsg, this is still on my radar, but I won't have time to tackle this before November/December.

@pradyunsg
Copy link
Member Author

That's perfectly reasonable. We should keep the warning about the format until we address this though.

@sbidoul
Copy link
Member

sbidoul commented Sep 30, 2022

I think this change will be backward compatible, though, as I plan to propose new a hashes field to the PEP 610 data structure, while keeping hash for backward compatibility.

@sbidoul
Copy link
Member

sbidoul commented Dec 30, 2022

@pradyunsg As promised, here is the draft PEP to amend the direct_url.json data structure.

@pradyunsg
Copy link
Member Author

Thank you for doing this @sbidoul! ^.^

@sbidoul
Copy link
Member

sbidoul commented Jan 22, 2023

@pradyunsg @pfmoore how can we progress with this? I think this is the only issue blocking us marking the installation report format as stable.

Quite understandably, this being a niche issue with little or no practical interest right now, my little draft PEP did not raise any comment.

There are a few options:

  • move forward with the draft PEP (I'll need your help for that)
  • keep the implementation as is (with one hash) and declare the install report format as stable, knowing a backward compatible path forward exists, and that we can dig out the draft PEP if/when practical needs arise
  • keep the install report format in experimental status
  • implement a ad-hoc deviation from the PEP 610 format in the installation report (which would complicate the implementation and documentation quite a bit, and I'm really not keen to do it)

@pfmoore
Copy link
Member

pfmoore commented Jan 22, 2023

I've posted on the draft PEP thread on Discourse, suggesting that in the absence of any objections, this change can be made as a text-only change to the spec. If no-one has any problem with that over the next week, let's just make the change.

Either way, I say we declare the report format as stable. So basically either your first or second option, with the first being my preference for "just get it done with" reasons 🙂

@sbidoul
Copy link
Member

sbidoul commented Jan 22, 2023

Perfect. Thank you, Paul.

@pradyunsg
Copy link
Member Author

+1 to all that Paul said!

inmantaci pushed a commit to inmanta/inmanta-core that referenced this issue Jan 31, 2023
Bumps [pip](https://github.com/pypa/pip) from 22.3.1 to 23.0.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/pypa/pip/blob/main/NEWS.rst">pip's changelog</a>.</em></p>
<blockquote>
<h1>23.0 (2023-01-30)</h1>
<h2>Features</h2>
<ul>
<li>Change the hashes in the installation report to be a mapping. Emit the
<code>archive_info.hashes</code> dictionary in <code>direct_url.json</code>. (<code>[#11312](pypa/pip#11312) &lt;https://github.com/pypa/pip/issues/11312&gt;</code>_)</li>
<li>Implement logic to read the <code>EXTERNALLY-MANAGED</code> file as specified in PEP 668.
This allows a downstream Python distributor to prevent users from using pip to
modify the externally managed environment. (<code>[#11381](pypa/pip#11381) &lt;https://github.com/pypa/pip/issues/11381&gt;</code>_)</li>
<li>Enable the use of <code>keyring</code> found on <code>PATH</code>. This allows <code>keyring</code>
installed using <code>pipx</code> to be used by <code>pip</code>. (<code>[#11589](pypa/pip#11589) &lt;https://github.com/pypa/pip/issues/11589&gt;</code>_)</li>
<li>The inspect and installation report formats are now declared stabled, and their version
has been bumped from <code>0</code> to <code>1</code>. (<code>[#11757](pypa/pip#11757) &lt;https://github.com/pypa/pip/issues/11757&gt;</code>_)</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Wheel cache behavior is restored to match previous versions, allowing the
cache to find existing entries. (<code>[#11527](pypa/pip#11527) &lt;https://github.com/pypa/pip/issues/11527&gt;</code>_)</li>
<li>Use the &quot;venv&quot; scheme if available to obtain prefixed lib paths. (<code>[#11598](pypa/pip#11598) &lt;https://github.com/pypa/pip/issues/11598&gt;</code>_)</li>
<li>Deprecated a historical ambiguity in how <code>egg</code> fragments in URL-style
requirements are formatted and handled. <code>egg</code> fragments that do not look
like PEP 508 names now produce a deprecation warning. (<code>[#11617](pypa/pip#11617) &lt;https://github.com/pypa/pip/issues/11617&gt;</code>_)</li>
<li>Fix scripts path in isolated build environment on Debian. (<code>[#11623](pypa/pip#11623) &lt;https://github.com/pypa/pip/issues/11623&gt;</code>_)</li>
<li>Make <code>pip show</code> show the editable location if package is editable (<code>[#11638](pypa/pip#11638) &lt;https://github.com/pypa/pip/issues/11638&gt;</code>_)</li>
<li>Stop checking that <code>wheel</code> is present when <code>build-system.requires</code>
is provided without <code>build-system.build-backend</code> as <code>setuptools</code>
(which we still check for) will inject it anyway. (<code>[#11673](pypa/pip#11673) &lt;https://github.com/pypa/pip/issues/11673&gt;</code>_)</li>
<li>Fix an issue when an already existing in-memory distribution would cause
exceptions in <code>pip install</code> (<code>[#11704](pypa/pip#11704) &lt;https://github.com/pypa/pip/issues/11704&gt;</code>_)</li>
</ul>
<h2>Vendored Libraries</h2>
<ul>
<li>Upgrade certifi to 2022.12.7</li>
<li>Upgrade chardet to 5.1.0</li>
<li>Upgrade colorama to 0.4.6</li>
<li>Upgrade distro to 1.8.0</li>
<li>Remove pep517 from vendored packages</li>
<li>Upgrade platformdirs to 2.6.2</li>
<li>Add pyproject-hooks 1.0.0</li>
<li>Upgrade requests to 2.28.2</li>
<li>Upgrade rich to 12.6.0</li>
<li>Upgrade urllib3 to 1.26.14</li>
</ul>
<h2>Improved Documentation</h2>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/368c7b4c557e673b05b0f8cffc967d3e333eee19"><code>368c7b4</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/aa94ccadb45d6ee44defea8a82bd5b647ccba799"><code>aa94cca</code></a> Update AUTHORS.txt</li>
<li><a href="https://github.com/pypa/pip/commit/60ce5c0943c303e48f0aed8bce650f725dcd222d"><code>60ce5c0</code></a> Fix the kind of news fragment</li>
<li><a href="https://github.com/pypa/pip/commit/e3e7bc34eb486622ebbb6412afc98ee57fcbff4a"><code>e3e7bc3</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11766">#11766</a> from uranusjr/upgrade-pre-commit-isort</li>
<li><a href="https://github.com/pypa/pip/commit/b653b129c56b29ad565886c1f423de89639d20f3"><code>b653b12</code></a> Bump pre-commit isort to 5.12.0</li>
<li><a href="https://github.com/pypa/pip/commit/a2a4feb588edc7233ae262d76b2c7291d6857a31"><code>a2a4feb</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11761">#11761</a> from sbidoul/direct-url-hashes-part-3-sbi</li>
<li><a href="https://github.com/pypa/pip/commit/ec7eb6f179866151f148c7695fc773e66b8c3adc"><code>ec7eb6f</code></a> Add version history to inspect and install report docs</li>
<li><a href="https://github.com/pypa/pip/commit/169511e68eb64efff5705305f72b0c53d7bff580"><code>169511e</code></a> Update direct URL hashes examples</li>
<li><a href="https://github.com/pypa/pip/commit/efedf09c4967dcbe3105e3746aaca7bfb55d605f"><code>efedf09</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11759">#11759</a> from pradyunsg/fix-keyring-auth</li>
<li><a href="https://github.com/pypa/pip/commit/60a45984404460192067f3990e0258deeeafa636"><code>60a4598</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11758">#11758</a> from pradyunsg/vendoring-update</li>
<li>Additional commits viewable in <a href="https://github.com/pypa/pip/compare/22.3.1...23.0">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pip&package-manager=pip&previous-version=22.3.1&new-version=23.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

</details>
mergify bot pushed a commit to aws/jsii that referenced this issue Feb 8, 2023
…s/@jsii/python-runtime (#3950)

Updates the requirements on [pip](https://github.com/pypa/pip) to permit the latest version.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/pypa/pip/blob/main/NEWS.rst">pip's changelog</a>.</em></p>
<blockquote>
<h1>23.0 (2023-01-30)</h1>
<h2>Features</h2>
<ul>
<li>Change the hashes in the installation report to be a mapping. Emit the
<code>archive_info.hashes</code> dictionary in <code>direct_url.json</code>. (<code>[#11312](pypa/pip#11312) &lt;https://github.com/pypa/pip/issues/11312&gt;</code>_)</li>
<li>Implement logic to read the <code>EXTERNALLY-MANAGED</code> file as specified in PEP 668.
This allows a downstream Python distributor to prevent users from using pip to
modify the externally managed environment. (<code>[#11381](pypa/pip#11381) &lt;https://github.com/pypa/pip/issues/11381&gt;</code>_)</li>
<li>Enable the use of <code>keyring</code> found on <code>PATH</code>. This allows <code>keyring</code>
installed using <code>pipx</code> to be used by <code>pip</code>. (<code>[#11589](pypa/pip#11589) &lt;https://github.com/pypa/pip/issues/11589&gt;</code>_)</li>
<li>The inspect and installation report formats are now declared stabled, and their version
has been bumped from <code>0</code> to <code>1</code>. (<code>[#11757](pypa/pip#11757) &lt;https://github.com/pypa/pip/issues/11757&gt;</code>_)</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Wheel cache behavior is restored to match previous versions, allowing the
cache to find existing entries. (<code>[#11527](pypa/pip#11527) &lt;https://github.com/pypa/pip/issues/11527&gt;</code>_)</li>
<li>Use the &quot;venv&quot; scheme if available to obtain prefixed lib paths. (<code>[#11598](pypa/pip#11598) &lt;https://github.com/pypa/pip/issues/11598&gt;</code>_)</li>
<li>Deprecated a historical ambiguity in how <code>egg</code> fragments in URL-style
requirements are formatted and handled. <code>egg</code> fragments that do not look
like PEP 508 names now produce a deprecation warning. (<code>[#11617](pypa/pip#11617) &lt;https://github.com/pypa/pip/issues/11617&gt;</code>_)</li>
<li>Fix scripts path in isolated build environment on Debian. (<code>[#11623](pypa/pip#11623) &lt;https://github.com/pypa/pip/issues/11623&gt;</code>_)</li>
<li>Make <code>pip show</code> show the editable location if package is editable (<code>[#11638](pypa/pip#11638) &lt;https://github.com/pypa/pip/issues/11638&gt;</code>_)</li>
<li>Stop checking that <code>wheel</code> is present when <code>build-system.requires</code>
is provided without <code>build-system.build-backend</code> as <code>setuptools</code>
(which we still check for) will inject it anyway. (<code>[#11673](pypa/pip#11673) &lt;https://github.com/pypa/pip/issues/11673&gt;</code>_)</li>
<li>Fix an issue when an already existing in-memory distribution would cause
exceptions in <code>pip install</code> (<code>[#11704](pypa/pip#11704) &lt;https://github.com/pypa/pip/issues/11704&gt;</code>_)</li>
</ul>
<h2>Vendored Libraries</h2>
<ul>
<li>Upgrade certifi to 2022.12.7</li>
<li>Upgrade chardet to 5.1.0</li>
<li>Upgrade colorama to 0.4.6</li>
<li>Upgrade distro to 1.8.0</li>
<li>Remove pep517 from vendored packages</li>
<li>Upgrade platformdirs to 2.6.2</li>
<li>Add pyproject-hooks 1.0.0</li>
<li>Upgrade requests to 2.28.2</li>
<li>Upgrade rich to 12.6.0</li>
<li>Upgrade urllib3 to 1.26.14</li>
</ul>
<h2>Improved Documentation</h2>

</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/368c7b4c557e673b05b0f8cffc967d3e333eee19"><code>368c7b4</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/aa94ccadb45d6ee44defea8a82bd5b647ccba799"><code>aa94cca</code></a> Update AUTHORS.txt</li>
<li><a href="https://github.com/pypa/pip/commit/60ce5c0943c303e48f0aed8bce650f725dcd222d"><code>60ce5c0</code></a> Fix the kind of news fragment</li>
<li><a href="https://github.com/pypa/pip/commit/e3e7bc34eb486622ebbb6412afc98ee57fcbff4a"><code>e3e7bc3</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11766">#11766</a> from uranusjr/upgrade-pre-commit-isort</li>
<li><a href="https://github.com/pypa/pip/commit/b653b129c56b29ad565886c1f423de89639d20f3"><code>b653b12</code></a> Bump pre-commit isort to 5.12.0</li>
<li><a href="https://github.com/pypa/pip/commit/a2a4feb588edc7233ae262d76b2c7291d6857a31"><code>a2a4feb</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11761">#11761</a> from sbidoul/direct-url-hashes-part-3-sbi</li>
<li><a href="https://github.com/pypa/pip/commit/ec7eb6f179866151f148c7695fc773e66b8c3adc"><code>ec7eb6f</code></a> Add version history to inspect and install report docs</li>
<li><a href="https://github.com/pypa/pip/commit/169511e68eb64efff5705305f72b0c53d7bff580"><code>169511e</code></a> Update direct URL hashes examples</li>
<li><a href="https://github.com/pypa/pip/commit/efedf09c4967dcbe3105e3746aaca7bfb55d605f"><code>efedf09</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11759">#11759</a> from pradyunsg/fix-keyring-auth</li>
<li><a href="https://github.com/pypa/pip/commit/60a45984404460192067f3990e0258deeeafa636"><code>60a4598</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11758">#11758</a> from pradyunsg/vendoring-update</li>
<li>Additional commits viewable in <a href="https://github.com/pypa/pip/compare/22.3...23.0">compare view</a></li>
</ul>
</details>
<br />


Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: direct url Direct URL references (PEP 440, PEP 508, PEP 610) C: inspect The inspect command C: install report pip install --report type: feature request Request for a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants