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

Remove the setuptools-provided distutils hack, if using distutils #11298

Merged
merged 2 commits into from
Jul 26, 2022

Conversation

pradyunsg
Copy link
Member

This ensures that pip's imported copy of distutils comes from the
standard library, if/when the hack needs to be used.

Closes #11294
Closes #8761

@pradyunsg
Copy link
Member Author

To test this manually, you can pip install https://github.com/pradyunsg/pip/archive/refs/heads/remove-distutils-shim.zip

@jaraco jaraco self-requested a review July 24, 2022 15:28
@jaraco
Copy link
Member

jaraco commented Jul 24, 2022

In #8761 (comment), I tested the change and it's having the desired effect. Thanks!

@Jorricks
Copy link

Jorricks commented Jul 24, 2022

As mentioned here, this fixes the issue I am currently fixing with Flit.

@uranusjr
Copy link
Member

I wonder if we should instead back-port the distutils removal PRs to 22.2. We are already committed to doing it, and it feels somewhat unnecessary to maintain a different fix to the same issue in 22.2.

@sbidoul
Copy link
Member

sbidoul commented Jul 25, 2022

I wonder if we should instead back-port the distutils removal PRs to 22.2. We are already committed to doing it, and it feels somewhat unnecessary to maintain a different fix to the same issue in 22.2.

@uranusjr I think all of them are in 22.2, unless I missed one that is not merged yet ?

@uranusjr
Copy link
Member

uranusjr commented Jul 25, 2022

Ah right, the problem here is that some parts of pip have to continue using distutils (location-related stuff) and we still need a fix for those. So this PR is still neccesary.

@pradyunsg
Copy link
Member Author

the problem here is that some parts of pip have to continue using distutils (location-related stuff)

Huh?

22.2 has all the distutils removal PRs we created, and distutils won't get imported on 3.10 and above. However, we need to use distuils on 3.7-3.9, because we know that many redistributors haven't configured sysconfig correctly.

This PR is necessary since it's placing the shim-removal logic in the "correct" location within pip, which knows when it's going to import distutils (limited to a single module -- in _locations/_distutils.py) unlike setuptools which needs to do the same thing in a .pth file instead. The setuptools approach doesn't work with the removal of create-a-zipfile approach to making pip importable in an isolated environment, only because it tries to remove the shim as a side-effect of a lookup for a pip import -- a lookup that never reaches the setuptools import hook because pip's redirector handles it before it does.

@sbidoul
Copy link
Member

sbidoul commented Jul 26, 2022

@pradyunsg do you feel like adding a news entry that is understandable by mere humans? :) I personally wouldn't know how to formulate that 🤯

This ensures that pip's imported copy of distutils comes from the
standard library, if/when the hack needs to be used.
@pradyunsg
Copy link
Member Author

I have made an attempt, at communicating this in the NEWS file. Honestly, I'm not quite sure how we'd test this behaviour within our test suite.

@sbidoul How strongly do you feel about adding a test to the test suite for #11294? :)

Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbidoul How strongly do you feel about adding a test to the test suite for #11294? :)

I can live without a test for that :) especially since I would not know either where to start to add one. I'm still perplexed that our current test suite does not reveal it.

@pradyunsg pradyunsg merged commit fcb0c84 into pypa:main Jul 26, 2022
@pradyunsg pradyunsg deleted the remove-distutils-shim branch July 26, 2022 23:14
JonathanWillitts added a commit to effect-trial/effect-form-validators that referenced this pull request Jul 27, 2022
…dencies did not run successfully."

when pip installing in Python 3.9 environment.

See:
- pypa/pip#11294 for discussion on issue being introduced in pip 22.2
- pypa/pip#11298 for fix, due to be included in pip 22.2.1 release
@bdrosen96
Copy link
Contributor

This seems to cause issues because it can raise AttributeError if used with a distutils version without remove_shim

14:16:36      __import__("_distutils_hack").remove_shim()
14:16:36  AttributeError: module '_distutils_hack' has no attribute 'remove_shim'

@pradyunsg
Copy link
Member Author

What version of setuptools do you have?

@bdrosen96
Copy link
Contributor

setuptools==36.0.1

@bdrosen96
Copy link
Contributor

This errors occurs when running:

python3 -m pip install -U pip setuptool

@pradyunsg
Copy link
Member Author

That attribute has been in setuptools for over two years now: https://github.com/pypa/setuptools/blame/main/_distutils_hack/__init__.py#L218

It seems weird to have an environment with such an old version of setuptools and the latest pip release that was released mere hours ago.

@pradyunsg
Copy link
Member Author

Could you please file an issue with the details, and possibly a link to a publicly viewable failure log?

@jaraco
Copy link
Member

jaraco commented Jul 27, 2022

I was tempted to wrap the workaround in a failsafe, but I presumed it was unlikely that any user would have this combination (it's common and recommended for users to always upgrade to the latest Setuptools). Perhaps it deserves a failsafe (try/except), or perhaps it should be unsupported. Sorry for the hassle.

inmantaci pushed a commit to inmanta/inmanta-core that referenced this pull request Jul 28, 2022
Bumps [pip](https://github.com/pypa/pip) from 22.2 to 22.2.1.
<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>22.2.1 (2022-07-27)</h1>
<h2>Bug Fixes</h2>
<ul>
<li>Send the pip upgrade prompt to stderr. (<code>[#11282](pypa/pip#11282) &lt;https://github.com/pypa/pip/issues/11282&gt;</code>_)</li>
<li>Ensure that things work correctly in environments where setuptools-injected
<code>distutils</code> is available by default. This is done by cooperating with
setuptools' injection logic to ensure that pip uses the <code>distutils</code> from the
Python standard library instead. (<code>[#11298](pypa/pip#11298) &lt;https://github.com/pypa/pip/issues/11298&gt;</code>_)</li>
<li>Clarify that <code>pip cache</code>'s wheels-related output is about locally built wheels only. (<code>[#11300](pypa/pip#11300) &lt;https://github.com/pypa/pip/issues/11300&gt;</code>_)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/61bdbe0d66ad472372f67c7bce05629027bdfc2b"><code>61bdbe0</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/fcb0c84116dc40e0de2f0f79af9cbbb68350e2b9"><code>fcb0c84</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11298">#11298</a> from pradyunsg/remove-distutils-shim</li>
<li><a href="https://github.com/pypa/pip/commit/a14f1412ce269cd0869f56b589e8e3209b7ac215"><code>a14f141</code></a> 📰</li>
<li><a href="https://github.com/pypa/pip/commit/b728bdad2a4fe29f2eedb8f37bdeefc1f60e756d"><code>b728bda</code></a> Remove the setuptools-provided distutils hack, if using distutils</li>
<li><a href="https://github.com/pypa/pip/commit/0231a1d9b69e4e1db9b158d112451a934355c869"><code>0231a1d</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11303">#11303</a> from vanschelven/clarify-pip-cache-output</li>
<li><a href="https://github.com/pypa/pip/commit/80c3b9615fd3137208b72649071a08f85660b3ca"><code>80c3b96</code></a> Textual: &quot;locally built&quot; rather than &quot;built&quot;</li>
<li><a href="https://github.com/pypa/pip/commit/d57c5dd1eea3caf099190f34ca0c21e088f7b28e"><code>d57c5dd</code></a> Fixed the tests</li>
<li><a href="https://github.com/pypa/pip/commit/f2c49cdbad84323956df48a7d9c81b86faf263b4"><code>f2c49cd</code></a> Add news article</li>
<li><a href="https://github.com/pypa/pip/commit/906b87727b2a79277f2bf81e34e44afece06bdbd"><code>906b877</code></a> Clarify pip cache output</li>
<li><a href="https://github.com/pypa/pip/commit/c4606b3572529625762f0586dda134302cf6122c"><code>c4606b3</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11283">#11283</a> from pfmoore/classifier_311</li>
<li>Additional commits viewable in <a href="https://github.com/pypa/pip/compare/22.2...22.2.1">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.2&new-version=22.2.1)](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>
inmantaci pushed a commit to inmanta/inmanta-core that referenced this pull request Jul 28, 2022
Bumps [pip](https://github.com/pypa/pip) from 22.2 to 22.2.1.
<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>22.2.1 (2022-07-27)</h1>
<h2>Bug Fixes</h2>
<ul>
<li>Send the pip upgrade prompt to stderr. (<code>[#11282](pypa/pip#11282) &lt;https://github.com/pypa/pip/issues/11282&gt;</code>_)</li>
<li>Ensure that things work correctly in environments where setuptools-injected
<code>distutils</code> is available by default. This is done by cooperating with
setuptools' injection logic to ensure that pip uses the <code>distutils</code> from the
Python standard library instead. (<code>[#11298](pypa/pip#11298) &lt;https://github.com/pypa/pip/issues/11298&gt;</code>_)</li>
<li>Clarify that <code>pip cache</code>'s wheels-related output is about locally built wheels only. (<code>[#11300](pypa/pip#11300) &lt;https://github.com/pypa/pip/issues/11300&gt;</code>_)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/61bdbe0d66ad472372f67c7bce05629027bdfc2b"><code>61bdbe0</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/fcb0c84116dc40e0de2f0f79af9cbbb68350e2b9"><code>fcb0c84</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11298">#11298</a> from pradyunsg/remove-distutils-shim</li>
<li><a href="https://github.com/pypa/pip/commit/a14f1412ce269cd0869f56b589e8e3209b7ac215"><code>a14f141</code></a> 📰</li>
<li><a href="https://github.com/pypa/pip/commit/b728bdad2a4fe29f2eedb8f37bdeefc1f60e756d"><code>b728bda</code></a> Remove the setuptools-provided distutils hack, if using distutils</li>
<li><a href="https://github.com/pypa/pip/commit/0231a1d9b69e4e1db9b158d112451a934355c869"><code>0231a1d</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11303">#11303</a> from vanschelven/clarify-pip-cache-output</li>
<li><a href="https://github.com/pypa/pip/commit/80c3b9615fd3137208b72649071a08f85660b3ca"><code>80c3b96</code></a> Textual: &quot;locally built&quot; rather than &quot;built&quot;</li>
<li><a href="https://github.com/pypa/pip/commit/d57c5dd1eea3caf099190f34ca0c21e088f7b28e"><code>d57c5dd</code></a> Fixed the tests</li>
<li><a href="https://github.com/pypa/pip/commit/f2c49cdbad84323956df48a7d9c81b86faf263b4"><code>f2c49cd</code></a> Add news article</li>
<li><a href="https://github.com/pypa/pip/commit/906b87727b2a79277f2bf81e34e44afece06bdbd"><code>906b877</code></a> Clarify pip cache output</li>
<li><a href="https://github.com/pypa/pip/commit/c4606b3572529625762f0586dda134302cf6122c"><code>c4606b3</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11283">#11283</a> from pfmoore/classifier_311</li>
<li>Additional commits viewable in <a href="https://github.com/pypa/pip/compare/22.2...22.2.1">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.2&new-version=22.2.1)](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>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants