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

Specify pip config settings after package #7686

Closed
wants to merge 1 commit into from

Conversation

nulano
Copy link
Contributor

@nulano nulano commented Jan 2, 2024

Suggested by @webknjaz in #7658 (comment)

@radarhere
Copy link
Member

https://pip.pypa.io/en/stable/cli/pip_install/ would indicate the opposite to me
Screenshot 2024-01-03 at 10 42 34 pm
where "--config-settings" is listed later on the page under "Options".

Similarly, see https://pip.pypa.io/en/stable/cli/pip_install/
Screenshot 2024-01-03 at 10 40 46 pm

This is a minor change, I would just like to have reasons for things, and to be able to cite those reasons if someone comes along and requests that we change it back.

@webknjaz
Copy link

webknjaz commented Jan 3, 2024

@radarhere your screenshots don't appear related at all. What's opposite there?

@webknjaz
Copy link

webknjaz commented Jan 3, 2024

Also, pypa/build has entirely separate CLI. And it can't build several packages in one invocation. So in its case the args order is not really relevant.

@radarhere
Copy link
Member

Here's one of the changes in this PR, moving the package from the end to before the settings.
Screenshot 2024-01-04 at 10 08 10 am

https://pip.pypa.io/en/stable/cli/pip_install/ indicates to me that the package should come after the settings,
Screenshot 2024-01-03 at 10 42 34 pm
where "--config-settings" is listed later on the page under "Options".

If that isn't clear, in the "Examples" on https://pip.pypa.io/en/stable/cli/pip_install/, it lists the following, where "--upgrade", also listed as one of the "Options" on that page, comes before the package.
Screenshot 2024-01-04 at 10 11 08 am

@radarhere
Copy link
Member

@webknjaz did you find my last comment any clearer? Even if not, could you offer any documentation that indicates that the change in this PR should be made?

@webknjaz
Copy link

I think that pip's own docs need clarification. Pip's CLI parser can process options in any places. But there's a catch. In the past, some of the options were package-specific, while the rest were global. There used to be --install-option that was passed to a package listed before it, and --global-option that was passed to all the packages.
Nowadays, this setuptools-specific interface is being phased out, I think. But the point is that in the PEP 517 world, it would be reasonable to expect that the --config-setting arguments might need to be different for different packages, which makes it similar to that old --install-option.

Your example with --upgrade is an example of a global option. Although, -e package-name would be a package-specific --editable that does not install all other packages in the same install request (each would need it's own -e). Though, my understanding is that -e has to come before packages, as opposed to --install-option.

I don't think there's a clear doc on this, sadly. And the PEP 517 --config-setting probably has unspecified behavior today. Though, I'd expect it to behave like --install-option, as in only be passed to the package it precedes, over time.

@radarhere
Copy link
Member

The changes in this PR are to our CI jobs, or to install instructions for Windows.

In both cases, for the sake of clarity and simplicity, the dependencies are installed separately to Pillow, and as a general rule, one would hope that users execute the installation instructions more-or-less as-is.

So any discussion about how pip will behave with multiple packages seems somewhat irrelevant - it might be good form to encourage users to use pip in a way that allows for other scenarios, but if the pip documentation doesn't lead the way in that, I don't think we need to consider it our responsibility to prepare users for complex scenarios involving a tool that isn't Pillow.

@radarhere radarhere closed this Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants