PIP_NO_CACHE_DIR and PIP_NO_BUILD_ISOLATION behave opposite to how they read

[jakirkham]

trafficstars

The *_NO_* environment variables are confusing and counterintuitive. It seems all of them are enabled by setting them to off, false, or 0. However to disable them one must set them to on, true, or 1. It seems like I'm in good company as this behavior has confused a good number of other people as well.

ref: https://github.com/pypa/pip/issues/2897 ref: https://github.com/pypa/pip/issues/5385

[sinscary]

@pradyunsg Can I work on this?

[pfmoore]

@sinscary Feel free to pick this up. But please be aware of the referenced discussions - in particular, https://github.com/pypa/pip/issues/5385#issuecomment-415734783 points out that there's quite likely code that depends on the current behaviour, so something simple like switching the meaning of "yes" and "no" for the environment variables isn't going to be acceptable.

[jakirkham]

It's true. We have the same issue. We probably need a deprecation cycle.

[sinscary]

@pfmoore Yeah I have gone through all the discussion and related issues. And I understand that it's not as easy as it looks. :)

[cjerdonek]

The "reversal" in meaning of the environment variables doesn't seem to affect all of the environment variables, as the title of this issue suggests.

I believe only PIP_NO_CACHE_DIR and PIP_NO_BUILD_ISOLATION are affected. This is because the code for these two sets the value to the oppositely named attribute, for example --no-cache-dir has its value set to cache_dir:

https://github.com/pypa/pip/blob/1228f64ec00819eabd6027d82b1a829980ac2b00/src/pip/_internal/cli/cmdoptions.py#L525-L526

And similarly for --no-build-isolation:

https://github.com/pypa/pip/blob/1228f64ec00819eabd6027d82b1a829980ac2b00/src/pip/_internal/cli/cmdoptions.py#L562-L563

This doesn't seem to be true for--

• --no-clean
• --no-color
• --no-input
• --no-index
• --no-binary
• --no-deps

For example, --no-color is set to no_color, and similarly for the others:

https://github.com/pypa/pip/blob/1228f64ec00819eabd6027d82b1a829980ac2b00/src/pip/_internal/cli/cmdoptions.py#L146-L147

[cjerdonek]

I proposed a PR for this issue (the PIP_NO_CACHE_DIR half) here: https://github.com/pypa/pip/pull/5884

[sinscary]

@cjerdonek Thanks for Taking this up. I got busy with work in office and couldn't get time to work on this. So thank you. :+1:

[cjerdonek]

PR #5884 fixed this for the PIP_NO_CACHE_DIR variable. PIP_NO_BUILD_ISOLATION is still unaddressed, though, I believe.

[jakirkham]

I think PIP_NO_DEPENDENCIES is another one.

[mxcoder]

Hi, just a hunch, I'll work on some tests but I think --no-binary messes up with --no-warn-script-location, I added the first and suddenly the warnings about the PATH came back.

[ngoldbaum]

Barring a deprecation cycle, another way to head off the footgun is to explain it in the the docs for the option.

[jakirkham]

Think it would be better to improve the status quo (even if we need to adapt to it)

Could we spend some time discussing what an acceptable deprecation cycle would look like?

[wolfv]

Could we change these to read PIP_BUILD_ISOLATION={enabled, disabled} and similar (new!) env vars with sane behavior?

[pradyunsg]

Could we change these to read PIP_BUILD_ISOLATION={enabled, disabled} and similar (new!) env vars with sane behavior?

A PR for this would be welcome!

[pradyunsg]

Could we spend some time discussing what an acceptable deprecation cycle would look like?

My 2 cents... an acceptable approach for resolving this would be:

• Add an alternative spelling that is not confusing + release.
• Deprecate the old spelling + release.
• (after at least 6 months AKA 2 releases) Remove the old spelling + release.

This fits with our deprecation policy in spirit, which is mainly to allow users to transition gracefully over a few months rather than requiring immediate action.

I'll note that I think the old spelling should become an error after this transition, and should stay an error for all future versions -- i.e. we should add a test to verify that the old spelling results in an error with clear context in an inline-comment, commit message and/or PR description in the PR adding the test (so that we don't accidentally remove it).

[pfmoore]

Are we talking about changing both the option and the environment variable? Because if we just change the environment variable, we break the current rule that every option has a matching environment variable.

If the proposal is to make the new option --build-isolation=[enabled|disabled] with a corresponding environment variable, I'm fine with that. But deprecating the --no-build-isolation option might be more impactful than simply deprectating the PIP_NO_BUILD_ISOLATION environment variable.

Having said that, I'm generally in support of renaming the option to remove the confusing inverted behaviour.

[notatallshaw]

(after at least 6 months AKA 2 releases) Remove the old spelling + release.

Is it possible to hide options? i.e. remove them from the docs / docstring but not get rid of them. There tend to be a lot of use cases out there where the user doesn't always control the install options.

[jezdez]

Are we talking about changing both the option and the environment variable? Because if we just change the environment variable, we break the current rule that every option has a matching environment variable.

Please let's not break this rule, this has worked well over the years, this is just a poorly named config option.

Having said that, I'm generally in support of renaming the option to remove the confusing inverted behaviour.

Same.

As further feedback, this came up during a conda-ecosystem topic recently: https://github.com/conda/grayskull/issues/582 and https://github.com/conda/conda-build/pull/5541.

[ichard26]

I updated the issue title to reflect that it's only the build isolation option that is affected.