make --no-input flag work with install git+http and git+ssh schemes

[iTrooz]

Fix #12718

[ichard26]

This does fix the specific problem in the original issue, but shouldn't we set these environment variables globally so every Git command is run with interactivity turned off?

We could achieve this by overriding the run_command() method in the Git VSC class to extend the environment before delegating to the parent run_command() method.

I'm also not a huge fan of relying on os.environ to pass configuration state down to the VSC code, but I haven't investigated how painful it would be to do it properly with the current VSC backend design. This is accepted practice in the codebase, so unless it's easy to pass the configuration state properly, I wouldn't bother redoing this.

[ichard26]

Also hi! Welcome to pip. Let us know if you need anything. Thanks for your interest in improving pip!

[iTrooz]

shouldn't we set these environment variables globally so every Git command is run with interactivity turned off?
We could achieve this by overriding the run_command() method in the Git VSC class to extend the environment before delegating to the parent run_command() method.

Seems like a good idea ! Done

I'm also not a huge fan of relying on os.environ to pass configuration state down to the VSC code, but I haven't investigated how painful it would be to do it properly with the current VSC backend design. This is accepted practice in the codebase, so unless it's easy to pass the configuration state properly, I wouldn't bother redoing this.

Unfortunately, this looks complicated to do (seems like I have to pass the data through 15 stack calls or so) and I don't have enough experience with the codebase to make a change like that the right way

[ichard26]

The new test is going to hang ~forever if we ever regress which is not great. Unfortunately, scripttest (the testing framework that implements the subprocess functionality) doesn't support the timeout subprocess parameter.

Looking into the state of scripttest, it turns out that being a pip maintainer gives me the commit bit on pypa/scripttest as well. The project seems rather unmaintained (although not-last-release-from-2013 level of unmaintained). I'll take a look at the project at some point.

[iTrooz]

I just tried, and removing the --no-input flag make the test fail instead of hanging, probably because you disable stdin in the tests

Plus, you also have a test above to check for the authentication prompt:

pip/tests/functional/test_install_config.py

Line 262 in 5616426

def test_prompt_for_authentication(

[ichard26]

The point of this bugfix is to ensure --no-input is respected by any Git or SSH commands pip invokes. Removing that from the test (and relying on lack of a STDIN) would render the test pointless IMO. We should update scripttest to support a timeout parameter and use that instead.

[iTrooz]

I'm not sure what the problem is. Why would the test hang in any case ? stdin is always disabled (AFAIK), so the test will never actually prompt for a username/password. It will just display the prompt message, and then crash because stdin will not be available.

[ichard26]

I dunno. It seems to hang locally if I keep the test but revert the fix ¯\_(ツ)_/¯

[iTrooz]

Oh ! Indeed it does. I'm not sure why. I will investigate, thank you

[iTrooz]

Okay, I found out the problem ! git clone uses terminal device instead of stdin to prompt for username/password, that's why you still get the prompt locally. I'm not sure how to properly fix this

[iTrooz]

Ok, done ! I used start_new_session=True to isolate the process and disable terminal device prompts
Now even in case of regressing, the test won't hang locally (and shouldn't hang in the server in any case since there won't be a terminal associated to the test, I think ?)