env: pop PIP_REQUIRE_VIRTUALENV from isolated environment

[adamchainz]

Fixes #90.

[adamchainz]

If this is found to break anything, the approach from pypa/pyproject-hooks@a03b4e8 could be copied

[FFY00]

Hum, will --isolated break PIP_USE_FEATURE=2020-resolver? Maybe we should be setting that all ourselves when it is required.

@pradyunsg could you recommend anything here? In some cases we might end up with duplicated requirements and the dependency resolver would be able to handle that. Are any possible drawbacks to using the dependency resolver in all cases?

What I've been doing is.

PIP_USE_FEATURE=2020-resolver python -m build

But I agree here that passing --isolated would be good.

I think we should remove PIP_REQUIRE_VIRTUALENV regardless, can you please add that to the PR? Thanks!

[adamchainz]

Hum, will --isolated break PIP_USE_FEATURE=2020-resolver?

I imagine so

I think we should remove PIP_REQUIRE_VIRTUALENV regardless, can you please add that to the PR? Thanks!

I don't see why we'd do both fixes? PIP_REQUIRE_VIRTUALENV has no effect when --isolated is passed.

[pganssle]

I think we should go with the approach where we pop PIP_REQUIRE_VIRTUALENV.

To me, the whole purpose of the PIP_* environment variables is to allow you to configure pip invocations nested within other programs. For PIP_REQUIRE_VIRTUALENV, it makes sense to remove it because we are installing to a virtual environment of sorts (I actually wonder if there's something we can do to make pip recognize this), so the actual criterion is satisfied.

For other things like the resolver, setting --no-binary :all:, etc, we don't want to be overriding people's pip settings.

[FFY00]

Okay, so you can add a _pop_env method to IsolatedEnvironment that will pop the env if present and add it to self._env to be restored later. Please also add a test for this if you can 😊

[pradyunsg]

I see I've been mentioned here, but I don't have anything to add beyond what @pganssle said (all of which I agree with). :)

[FFY00]

@pradyunsg my question for you was if it was a good idea to set PIP_USE_FEATURE=2020-resolver as we need it in some cases.

[pradyunsg]

Unconditionally in build, I'd say don't.

pip's going to flip the switch soon, and we'll remove that flag eventually as well. Sticking to what pip does by default will be easier to explain and justify. It might be a good idea to pass-through environment variables starting with PIP_ into the pip call TBH.

[FFY00]

@adamchainz you are just missing the type hint, # type: (str) -> None, if you could add it it would be great! Thanks 😁

[adamchainz]

Use github suggestions for such minutiae.

[FFY00]

Thank you 😊