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

Add a UI to set config settings for PEP 517 backends #11059

Merged
merged 10 commits into from
Apr 29, 2022

Conversation

pfmoore
Copy link
Member

@pfmoore pfmoore commented Apr 21, 2022

Still to do:

  • Documentation
  • Tests

But it seems to basically work, so reviews are welcome 🙂

Copy link

@abravalheri abravalheri left a comment

Choose a reason for hiding this comment

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

Thank you very much for having a look on this @pfmoore ❤️

docs/html/reference/build-system/pyproject-toml.md Outdated Show resolved Hide resolved
src/pip/_internal/cli/req_command.py Show resolved Hide resolved
src/pip/_internal/cli/req_command.py Show resolved Hide resolved
src/pip/_internal/cli/req_command.py Show resolved Hide resolved
@pfmoore
Copy link
Member Author

pfmoore commented Apr 22, 2022

Docs added. And on reflection I'm not entirely sure if this needs further tests - I think what I've added probably covers the main cases. Suggestions for extra tests would be welcome, though.

So I'm going to move this out of "Draft" mode. Further reviews would be appreciated before I merge this.

@pfmoore pfmoore marked this pull request as ready for review April 22, 2022 15:21
@pfmoore pfmoore changed the title [WIP] Add a UI to set config settings for PEP 517 backends Add a UI to set config settings for PEP 517 backends Apr 22, 2022
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

Haven't looked at this throughly, but a few quick comments based on a quick skim. This looks pretty great overall! :)

A minor nit: call it unit/test_pyproject_config_settings.py, instead of PEP 517?

src/pip/_internal/cli/cmdoptions.py Show resolved Hide resolved
'''


def test_backend_sees_config(script: PipTestEnvironment) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a test that it gets called on editable hooks?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably... It would likely be just a small variant on this one, so I'll add it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, this picked up an error in my implementation (which took me all morning to find 🙁)! Fix (and test) incoming.

New feature suggestion: Have pip error at startup with the message "The InstallRequirement class is an abomination and I refuse to work with it" 🙂

@pfmoore
Copy link
Member Author

pfmoore commented Apr 28, 2022

I think I've addressed the various review comments, so unless anyone has any additional points they want to raise, I'll merge this in a day or two. Please comment if you want me to wait (even if it's just "I need some time to think about this, please give me a while longer").

@pradyunsg pradyunsg added this to the 22.1 milestone Apr 28, 2022
@pfmoore
Copy link
Member Author

pfmoore commented Apr 29, 2022

OK, going to merge this now so @pradyunsg isn't waiting around for me 🙂

@pfmoore pfmoore merged commit cdeb8f9 into pypa:main Apr 29, 2022
@pfmoore pfmoore deleted the config_settings branch April 29, 2022 11:58
@mgorny
Copy link
Contributor

mgorny commented May 7, 2022

I'm sorry for being late to the party but we'd also need a way to pass a list as the value. The only use of config_settings I know of is --global-option in setuptools, and passing more than one option requires using a list rather than str.

@pfmoore
Copy link
Member Author

pfmoore commented May 7, 2022

From PEP 517, "Build frontends SHOULD provide some mechanism for users to specify arbitrary string-key/string-value pairs to be placed in this dictionary." You could parse the string value passed for global_option in setuptools (split on whitespace or commas, parse arbitrary JSON). I considered having a way to accepting arbitrary JSON for config_settings, but decided it was over-complex for the user (correctly quoting arbitrary JSON in the shell is messy, and depends heavily on the shell...) as well as going beyond what PEP 517 suggested.

Alternatively, a PR to add an option for this would be considered. The hard part (IMO) is designing a reasonable UI.

As a final note, pip's legacy --global-option flag is never passed to setuptools in the case of pyproject-based builds, so I can guarantee you that no pip user is currently using the ability to pass a list of values via config_settings 🙂

inmantaci pushed a commit to inmanta/inmanta-core that referenced this pull request May 12, 2022
Bumps [pip](https://github.com/pypa/pip) from 22.0.4 to 22.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.1 (2022-05-11)</h1>
<h2>Process</h2>
<ul>
<li>Enable the <code>importlib.metadata</code> metadata implementation by default on
Python 3.11 (or later). The environment variable <code>_PIP_USE_IMPORTLIB_METADATA</code>
can still be used to enable the implementation on 3.10 and earlier, or disable
it on 3.11 (by setting it to <code>0</code> or <code>false</code>).</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Revert <code>[#9243](pypa/pip#9243) &lt;https://github.com/pypa/pip/issues/9243&gt;</code>_ since it introduced a regression in certain edge cases. (<code>[#10962](pypa/pip#10962) &lt;https://github.com/pypa/pip/issues/10962&gt;</code>_)</li>
<li>Fix missing <code>REQUESTED</code> metadata when using URL constraints. (<code>[#11079](pypa/pip#11079) &lt;https://github.com/pypa/pip/issues/11079&gt;</code>_)</li>
<li><code>pip config</code> now normalizes names by converting underscores into dashes. (<code>[#9330](pypa/pip#9330) &lt;https://github.com/pypa/pip/issues/9330&gt;</code>_)</li>
</ul>
<h1>22.1b1 (2022-04-30)</h1>
<h2>Process</h2>
<ul>
<li>Start migration of distribution metadata implementation from <code>pkg_resources</code>
to <code>importlib.metadata</code>. The new implementation is currently not exposed in
any user-facing way, but included in the code base for easier development.</li>
</ul>
<h2>Deprecations and Removals</h2>
<ul>
<li>Drop <code>--use-deprecated=out-of-tree-build</code>, according to deprecation message. (<code>[#11001](pypa/pip#11001) &lt;https://github.com/pypa/pip/issues/11001&gt;</code>_)</li>
</ul>
<h2>Features</h2>
<ul>
<li>Add option to install and uninstall commands to opt-out from running-as-root warning. (<code>[#10556](pypa/pip#10556) &lt;https://github.com/pypa/pip/issues/10556&gt;</code>_)</li>
<li>Include Project-URLs in <code>pip show</code> output. (<code>[#10799](pypa/pip#10799) &lt;https://github.com/pypa/pip/issues/10799&gt;</code>_)</li>
<li>Improve error message when <code>pip config edit</code> is provided an editor that
doesn't exist. (<code>[#10812](pypa/pip#10812) &lt;https://github.com/pypa/pip/issues/10812&gt;</code>_)</li>
<li>Add a user interface for supplying config settings to build backends. (<code>[#11059](pypa/pip#11059) &lt;https://github.com/pypa/pip/issues/11059&gt;</code>_)</li>
<li>Add support for Powershell autocompletion. (<code>[#9024](pypa/pip#9024) &lt;https://github.com/pypa/pip/issues/9024&gt;</code>_)</li>
<li>Explains why specified version cannot be retrieved when <em>Requires-Python</em> is not satisfied. (<code>[#9615](pypa/pip#9615) &lt;https://github.com/pypa/pip/issues/9615&gt;</code>_)</li>
<li>Validate build dependencies when using <code>--no-build-isolation</code>. (<code>[#9794](pypa/pip#9794) &lt;https://github.com/pypa/pip/issues/9794&gt;</code>_)</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Fix conditional checks to prevent <code>pip.exe</code> from trying to modify itself, on Windows. (<code>[#10560](pypa/pip#10560) &lt;https://github.com/pypa/pip/issues/10560&gt;</code>_)</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/3c953322c6683b3f0f4d465d9fa361de55358462"><code>3c95332</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/bd54382b59b45975fd4ea00533fb92bd85e3b98d"><code>bd54382</code></a> Update AUTHORS.txt</li>
<li><a href="https://github.com/pypa/pip/commit/c86f9f12594c0e05ed2de31a652bc1eaadb970d1"><code>c86f9f1</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11044">#11044</a> from uranusjr/importlib-metadata-backend-in-3.11</li>
<li><a href="https://github.com/pypa/pip/commit/bd9bcef8b3262c513a0dd614731cfe8db8d64125"><code>bd9bcef</code></a> Enable importlib.metadata backend on Python 3.11</li>
<li><a href="https://github.com/pypa/pip/commit/cb24fb4052ca8ab8009866b0de61980c81a7e13c"><code>cb24fb4</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11085">#11085</a> from pypa/revert-10962-fix-hashes</li>
<li><a href="https://github.com/pypa/pip/commit/6ad9a21a43b7fe5d436472de2492069c4541bf06"><code>6ad9a21</code></a> 📰</li>
<li><a href="https://github.com/pypa/pip/commit/cf3696a81b341925f82f20cb527e656176987565"><code>cf3696a</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11080">#11080</a> from sbidoul/requested-with-constraints</li>
<li><a href="https://github.com/pypa/pip/commit/bab5bfce502be78318ab2a3b364b4923d657c854"><code>bab5bfc</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11073">#11073</a> from wimglenn/issue-9330</li>
<li><a href="https://github.com/pypa/pip/commit/ae1c2e35e493d7a07cb0d300451625629ae07ce9"><code>ae1c2e3</code></a> Grammar fix in changelog</li>
<li><a href="https://github.com/pypa/pip/commit/8d51b8365501132c4f9fe929aa40bd66c9eaa60e"><code>8d51b83</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11087">#11087</a> from mkniewallner/fix-version-changelog</li>
<li>Additional commits viewable in <a href="https://github.com/pypa/pip/compare/22.0.4...22.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.0.4&new-version=22.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 May 12, 2022
Bumps [pip](https://github.com/pypa/pip) from 22.0.4 to 22.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.1 (2022-05-11)</h1>
<h2>Process</h2>
<ul>
<li>Enable the <code>importlib.metadata</code> metadata implementation by default on
Python 3.11 (or later). The environment variable <code>_PIP_USE_IMPORTLIB_METADATA</code>
can still be used to enable the implementation on 3.10 and earlier, or disable
it on 3.11 (by setting it to <code>0</code> or <code>false</code>).</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Revert <code>[#9243](pypa/pip#9243) &lt;https://github.com/pypa/pip/issues/9243&gt;</code>_ since it introduced a regression in certain edge cases. (<code>[#10962](pypa/pip#10962) &lt;https://github.com/pypa/pip/issues/10962&gt;</code>_)</li>
<li>Fix missing <code>REQUESTED</code> metadata when using URL constraints. (<code>[#11079](pypa/pip#11079) &lt;https://github.com/pypa/pip/issues/11079&gt;</code>_)</li>
<li><code>pip config</code> now normalizes names by converting underscores into dashes. (<code>[#9330](pypa/pip#9330) &lt;https://github.com/pypa/pip/issues/9330&gt;</code>_)</li>
</ul>
<h1>22.1b1 (2022-04-30)</h1>
<h2>Process</h2>
<ul>
<li>Start migration of distribution metadata implementation from <code>pkg_resources</code>
to <code>importlib.metadata</code>. The new implementation is currently not exposed in
any user-facing way, but included in the code base for easier development.</li>
</ul>
<h2>Deprecations and Removals</h2>
<ul>
<li>Drop <code>--use-deprecated=out-of-tree-build</code>, according to deprecation message. (<code>[#11001](pypa/pip#11001) &lt;https://github.com/pypa/pip/issues/11001&gt;</code>_)</li>
</ul>
<h2>Features</h2>
<ul>
<li>Add option to install and uninstall commands to opt-out from running-as-root warning. (<code>[#10556](pypa/pip#10556) &lt;https://github.com/pypa/pip/issues/10556&gt;</code>_)</li>
<li>Include Project-URLs in <code>pip show</code> output. (<code>[#10799](pypa/pip#10799) &lt;https://github.com/pypa/pip/issues/10799&gt;</code>_)</li>
<li>Improve error message when <code>pip config edit</code> is provided an editor that
doesn't exist. (<code>[#10812](pypa/pip#10812) &lt;https://github.com/pypa/pip/issues/10812&gt;</code>_)</li>
<li>Add a user interface for supplying config settings to build backends. (<code>[#11059](pypa/pip#11059) &lt;https://github.com/pypa/pip/issues/11059&gt;</code>_)</li>
<li>Add support for Powershell autocompletion. (<code>[#9024](pypa/pip#9024) &lt;https://github.com/pypa/pip/issues/9024&gt;</code>_)</li>
<li>Explains why specified version cannot be retrieved when <em>Requires-Python</em> is not satisfied. (<code>[#9615](pypa/pip#9615) &lt;https://github.com/pypa/pip/issues/9615&gt;</code>_)</li>
<li>Validate build dependencies when using <code>--no-build-isolation</code>. (<code>[#9794](pypa/pip#9794) &lt;https://github.com/pypa/pip/issues/9794&gt;</code>_)</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Fix conditional checks to prevent <code>pip.exe</code> from trying to modify itself, on Windows. (<code>[#10560](pypa/pip#10560) &lt;https://github.com/pypa/pip/issues/10560&gt;</code>_)</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/pip/commit/3c953322c6683b3f0f4d465d9fa361de55358462"><code>3c95332</code></a> Bump for release</li>
<li><a href="https://github.com/pypa/pip/commit/bd54382b59b45975fd4ea00533fb92bd85e3b98d"><code>bd54382</code></a> Update AUTHORS.txt</li>
<li><a href="https://github.com/pypa/pip/commit/c86f9f12594c0e05ed2de31a652bc1eaadb970d1"><code>c86f9f1</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11044">#11044</a> from uranusjr/importlib-metadata-backend-in-3.11</li>
<li><a href="https://github.com/pypa/pip/commit/bd9bcef8b3262c513a0dd614731cfe8db8d64125"><code>bd9bcef</code></a> Enable importlib.metadata backend on Python 3.11</li>
<li><a href="https://github.com/pypa/pip/commit/cb24fb4052ca8ab8009866b0de61980c81a7e13c"><code>cb24fb4</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11085">#11085</a> from pypa/revert-10962-fix-hashes</li>
<li><a href="https://github.com/pypa/pip/commit/6ad9a21a43b7fe5d436472de2492069c4541bf06"><code>6ad9a21</code></a> 📰</li>
<li><a href="https://github.com/pypa/pip/commit/cf3696a81b341925f82f20cb527e656176987565"><code>cf3696a</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11080">#11080</a> from sbidoul/requested-with-constraints</li>
<li><a href="https://github.com/pypa/pip/commit/bab5bfce502be78318ab2a3b364b4923d657c854"><code>bab5bfc</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11073">#11073</a> from wimglenn/issue-9330</li>
<li><a href="https://github.com/pypa/pip/commit/ae1c2e35e493d7a07cb0d300451625629ae07ce9"><code>ae1c2e3</code></a> Grammar fix in changelog</li>
<li><a href="https://github.com/pypa/pip/commit/8d51b8365501132c4f9fe929aa40bd66c9eaa60e"><code>8d51b83</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/pypa/pip/issues/11087">#11087</a> from mkniewallner/fix-version-changelog</li>
<li>Additional commits viewable in <a href="https://github.com/pypa/pip/compare/22.0.4...22.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.0.4&new-version=22.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 May 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants