Environment
Description
Starting with SVN 1.8, SVN is non-interactive by default. Before that, it will prompt for a password when the user performs a svn checkout
.
The problem is that when pip
calls out to svn checkout
it is not interactive, and will not allow the user to enter their password. One solution is to store SVN passwords, but that may not be allowed by company rules or that may simply not be desirable for security reasons.
For some more context see
The solution seems to be:
<1.8
, work as it does now (no extra arguments needed).>=1.8
, add the --force-interactive
command line flag.Some context from svn checkout --help
--non-interactive : do no interactive prompting (default is to prompt
only if standard input is a terminal device)
--force-interactive : do interactive prompting even if standard input
is not a terminal device
Perhaps another solution would be to make svn
think it's being called from a terminal device when called from pip
?
Using environment variables is another potential option, but it runs into the same fundamental issue: users have to store their password (and in this case in plaintext). This doesn't seem to be appropriate for user workstations that could be shared.
Subversion versions on popular supported Linux distros:
Expected behavior
On SVN 1.7, 1.8, and 1.9, when pip
installs an SVN dependency, it prompts the user for their password if they have not saved it locally.
How to Reproduce
Install Subversion >=1.8.
sudo apt install subversion
. svn --version
will return 1.9.7
.Make a venv
:
$ python3 -m venv venv
$ source venv/bin/activate
(venv) $ pip install pip==19.0.3
pip install
an SVN URL from a URL that requires authentication. pip
's invocation of svn
will not prompt for a password, so it will always fail to install.(venv) $ pip install svn+https://my-svn-server.com/project
Collecting svn+https://my-svn-server.com/project
Checking out https://my-svn-server.com/project /tmp/pip-req-build-1g_qavb
svn: E170013: Unable to connect to repository at URL 'https://https://my-svn-server.com/project'
svn: E215004: No more credentials or we tried too many times.
Authentication failed.
Command "svn checkout -q https://my-svn-server.com/project /tmp/pip-req-build-1g_qavb" failed with error code 1 in None
References
It seems like the most "proper" way would be to make Popen
calls to SVN with a psuedo tty, so that SVN behaves as if it were called from the terminal directly.
There does seem to be precedent for special casing based on Subversion version:
My initial thought on this is that passing --force-interactive
would be better than trying to use a pseudo-terminal.
Also, it seems like this shouldn't be done by default, but perhaps only if we can be sure pip is being used interactively (e.g. if sys.stdout.isatty()
returns True
) -- a bit like how svn
is described as behaving.
It's worth pointing out that ptys aren't an option on Windows.
@cjerdonek Yeah, I agree that a pseudo-terminal would be a more complex solution.
The only issue with trying to pass --force-interactive
regardless of SVN version would be that SVN 1.7 and 1.6 don't come with this option, so doing that would break compatibility with RHEL/CentOS 6 and RHEL/CentOS 7. (Gotta love ancient supported toolchains...)
We could work around this with svn --version
, but I don't know if that is a bridge too far for a tool like pip
.
I think this could be a proposed solution (in pseudocode):
svn_version = get_version() # `svn --version`
if svn_version >= 1.8 and sys.stdout.isatty():
svn_options.append('--force-interactive')
else:
# Do nothing
pass
Thoughts?
While this might look easy, I think this might take a bit more work in certain ways to fully get right. Some thoughts:
call_subprocess()
.svn
to get the version, I think we'd only want to do that once (lazily), and then cache the result. (The Subversion
class isn't structured in the way that I'd personally like for this yet, but it's close.)VersionControl
class (and subclasses) isn't really set up yet to accept command-line options.Lastly, I don't think we'd need to worry about providing new functionality like this for older versions of SVN.
Okay, so just so I'm clear, you mean not supporting Subversion 1.6 and 1.7 (RHEL 6/7 and CentOS 6/7)? So we would push the --force-interactive
flag without regard for what version of Subversion they are running (since 1.6/1.7 don't have the flag?
# CentOS 7
$ svn --version
svn, version 1.7.14 (r1542130)
compiled Apr 11 2018, 02:40:28
$ svn --force-interactive
svn: invalid option: --force-interactive
I don't really disagree in principle (it's a burden to support old tools), but just wanted to be clear that pip
would then no longer be able to work with the platform installed Subversion on RHEL 6/7 and CentOS 6/7. RHEL/CentOS 7 will be supported until 2024 馃槙.
(The
Subversion
class isn't structured in the way that I'd personally like for this yet, but it's close.)
If we can agree on a way forward I'm willing to try to put together a PR. But if it involves refactoring the Subversion
class, a pip
maintainer would probably be better suited. If a pip
maintainer made the refactorings you suggest, I would be willing give it a shot to add --force-interactive
on top when needed.
So we would push the --force-interactive flag without regard for what version of Subversion they are running (since 1.6/1.7 don't have the flag?
I meant passing the flag only for SVN 1.8+ (which is why I also suggested caching the value of getting the version). But sorry, I had missed or forgotten that older versions were interactive by default in the absence of any option. So you can disregard that last bullet. :)
I would be willing give it a shot to add --force-interactive on top when needed.
Okay, great. But yeah, I'd prefer if you waited a bit, if that's okay. I actually have a VCS-related PR queued up (as a follow-up to PR #6356) that I'm now planning to modify in response to this issue. (The PR should become simpler in fact, and should help for this.) I'd also like to think about how call_subprocess()
should best be changed for this, because the logging there was just reworked to be simpler, and I'd like to avoid re-introducing too much more complexity there, if possible.
If you want I can describe some of the VCS changes I'd like to see. They aren't super hard but may involve touching a few things.
I'll also think to see what, if any, of this PR can be done in advance of the refactorings.
One thing you could do now in preparation is add a get_vcs_version()
class method to the Subversion
class that returns a tuple, and add a test for it. There's already a Git.get_git_version()
method you could look at for ideas. The test could be marked to run only if SVN is installed, and at least one of the test environments should require that SVN be installed. (You can look at the last Bazaar PR to see how that was done.)
Btw, is there a way to prevent interactivity on older versions of SVN (e.g. to prevent hangs)?
Btw, is there a way to prevent interactivity on older versions of SVN (e.g. to prevent hangs)?
# CentOS 7.6.
$ svn --version
svn, version 1.7.14 (r1542130)
compiled Apr 11 2018, 02:40:28
$ svn checkout --help
...
Global options:
--username ARG : specify a username ARG
--password ARG : specify a password ARG
--no-auth-cache : do not cache authentication tokens
--non-interactive : do no interactive prompting
--non-interactive
should be what you want, and it is available at least in SVN 1.7.
@cjerdonek I submitted #6390, let me know if that is what you were looking for.
--non-interactive should be what you want, and it is available at least in SVN 1.7.
Thanks. A related positive change IMO would be to pass that flag to SVN when SVN is 1.7 and isatty()
returns False
(the flip-side of the case you're interested in, to prevent hangs when pip isn't being used interactively).
A related positive change IMO would be to pass that flag to SVN when SVN is 1.7 and isatty() returns False
@cjerdonek I think we need to be very cautious about this. For example, it's currently very useful that when pip
is called from tox
on CentOS 7 with SVN 1.7, that svn
prompts for a password. I doubt pip
is in a tty at that point.
tox
-> pip
-> svn
: prompt user for password.
This allows us to use tox
from the terminal against private SVN repos, but not have users store their passwords on the system.
It's not a difficult configuration to make sure svn
only prompts you when you want it to prompt you (e.g. for a password you are intending not to store).
For example, it's currently very useful that when pip is called from tox on CentOS 7 with SVN 1.7, that svn prompts for a password. I doubt pip is in a tty at that point.
Okay, good to know. This is also why I said the following above:
- To do this properly, we would probably want to do what SVN is doing and provide command-line options to force the behavior one way or the other (e.g. if the user is redirecting stdout but still wants interactivity).
By the way, your point would also apply to the SVN 1.8+ case, with the approach for this PR of checking isatty()
, right?
For example, it's currently very useful that when pip is called from tox on CentOS 7 with SVN 1.7, that svn prompts for a password.
Actually, can you confirm whether you can really do this with recent versions of pip? It appears as if this isn't possible as of pip 10.0 (since stdin
is closed after the call to subprocess.Popen()
): https://github.com/pypa/pip/pull/4982
By the way, your point would also apply to the SVN 1.8+ case, with the approach for this PR of checking isatty(), right?
Yeah, I'm wondering if we should avoid suppressing interactive in either case based on isatty()
. It's a tough one, because on one hand getting svn
to ignore superfluous prompts is nice, but if it makes it so it's not possible to input a password at all when run from a tool like tox
, that is a problem. I'm not sure how tox
calls pip
, to know whether it thinks its in a tty
or not, but we should look into it when fixing this problem.
Actually, can you confirm whether you can really do this with recent versions of pip? It appears as if this isn't possible as of pip 10.0 (since stdin is closed after the call to subprocess.Popen()): #4982
I can confirm the following configurations all work with stock installed tools on CentOS 7.
svn
1.7.14 from the base CentOS 7 repopip
19.0.3tox
3.7.0~/.subversion/servers
has the following option set so that svn
does not prompt the user whether they want to store passwords (which would break pip
usage).
store-plaintext-passwords = no
pip
from terminal$ pip install svn+https://my-private-svn.com/svn/project/project-1
# Prompts for password, user enters
# pip successfully installs `project-1`
pip
from terminal with requirements.txt
requirements.txt
:
svn+https://my-private-svn.com/svn/project/project-1
$ pip install -r requirements.txt
# Prompts for password, user enters
# pip successfully installs `project-1`
tox
requirements.txt
:
svn+https://my-private-svn.com/svn/project/project-1
tox.ini
:
...
[testenv]
deps =
-r{toxinidir}/requirements.txt
...
$ tox --recreate
py36 create: ...
py36 installdeps -r /home/user/requirements.txt
# Prompts for password, user enters
# pip successfully installs `project-1`
...
I don't know the intricacies of Linux terminals well, but it would appear that stdin
prompts are different in some ways than these password prompts? For one, you don't see what you type when you enter passwords, so is there some other password/authentication-specific Linux command going on here?
@cjerdonek After some digging into the Subversion source code (always a fun activity), I found where svn
is opening the special device node /dev/tty
:
https://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_subr/prompt.c
As I understand it, /dev/tty
is a special device node that refers to the controlling terminal for a process. This allows svn
to prompt for a password directly to the terminal regardless of what other processes have called it.
So I believe this explains why stdin
doesn't apply in the case of password prompts from svn
.
After some more testing, perhaps the tox
case not making pip
think it's in a tty
is not actually a concern.
I wrote a small Python script that checked if stdout
, stderr
, and stdin
.isatty()
.
# tox.ini
[testenv]
commands = python tty_test.py
$ tox -e py36
All three returned True
. This doesn't 100% prove that when pip
is called (it is used in the deps
section of tox.ini
), it thinks it's in a tty
, but gives me hope that this may not be an issue.
Thanks for all your research and digging! 馃憤
@johnthagen Let me know if you want my thoughts on the next steps, or if you want to propose something and I can react to that. I think it might be helpful to do at least one more incremental PR before landing the functionality.
For reference, svn
options are:
# svn <= 1.7
--non-interactive : do no interactive prompting
# svn >= 1.8
--non-interactive : do no interactive prompting (default is to prompt
only if standard input is a terminal device)
--force-interactive : do interactive prompting even if standard input
is not a terminal device
Just so we're on the same page, our current pseudocode proposal is:
svn_version = get_vcs_version() # `svn --version`
if not sys.stdin.isatty():
# TODO: Test that `tox` `deps` install step runs `pip` in a TTY.
svn_options.append('--non-interactive')
elif svn_version >= 1.8:
svn_options.append('--force-interactive')
Or a more conservative proposal is to just address needing to get the password prompt on 1.8+.
svn_version = get_vcs_version() # `svn --version`
if svn_version >= 1.8 and sys.stdin.isatty():
svn_options.append('--force-interactive')
The second has less risk of breaking situations were you want to be prompted on 1.7, but has the risk that 1.7 and 1.8 could behave differently from pip
. The first approach is probably better (I just wish I had a better handle on when isatty()
will be False
).
What are your thoughts?
Do you know for sure that --non-interactive
is present in all SVN versions <= 1.7?
I was thinking the next PR could define and test a method on the Subversion
class that returns the list of global options to use (empty list, ['--non-interactive']
, or ['--force-interactive']
), but not start including them in the actual invocations yet. (Do you know which SVN invocations these options will need to be passed to, by the way?)
I think the Subversion
class's __init__()
method should start accepting an optional use_interactive
argument (or some other name) that defaults to a value based on sys.stdin.isatty()
. This will help with unit-testing the method above. It can also be used later to force certain behavior e.g. via pip command-line options.
I also think the class should grow a private _vcs_version
instance attribute to cache the version value from the method you just wrote. It can initially be set to None
(unset), with a None
version value being stored as some other non-None
value (perhaps False
). The get_vcs_version()
method can be changed to check this instance attribute, and call a class method called something like call_vcs_version()
that would contain your current logic. This private instance attribute will also help with unit-testing the method that returns the interactive options, because it will make it easy to test different "versions" of SVN.
I agree with your summary, by the way. Thanks!
Do you know for sure that --non-interactive is present in all SVN versions <= 1.7?
According to: https://svn.apache.org/repos/asf/subversion/trunk/CHANGES
Version 0.14.4 [Alpha Interim 4] (released 29 Oct 2002, revision 3553)
- new --non-interactive switch for commandline client
(Do you know which SVN invocations these options will need to be passed to, by the way?)
I believe this is where the options would need to be applied:
pip
checks out the Subversion project to a folder in /tmp
before installing it from that folder.
Regarding the various SVN invocations to consider, there are also invocations of commands like "export", "update", etc. When editable mode is being used, a different directory is used, and in-place updates can occur. I'm guessing at least "update" would also need the options passed.
Re: tox, I did a quick check of the tox code base, and it looks like stdin
is never passed to its subprocess calls. Perhaps this means any TTY is preserved (unlike pip which passes stdin=subprocess.PIPE
).
Can confirm the following commands are using in subversion.py
and support --non-interactive
and --force-interactive
:
export
checkout
switch
update
info
@cjerdonek With #6439 merged, I think the last thing that needs to be completed is for us to actually add the get_remote_call_options()
result to the invocations of svn
where they are needed:
Since this is when we actually change functionality, I plan to additionally test out the proposed pip
egg with this change on some private SVN repos (on SVN 1.7 & 1.8+) to make sure everything is working as we expect before we merge the PR.
How would you like to proceed?
@johnthagen That sounds good (and glad to hear you can test out on private repos). I don't _think_ there will be much code. But if you have any questions in advance, feel free to let me know.
@cjerdonek. Do you know which pip
version this will land in? I'm planning to test the current master branch and then also the first released version as well to triple check everything.
The next one (19.2).
Also, the next release will probably happen sometime in July (every 3 months).
Wanted to report that I tested the master branch at 83d813c on a private SVN repository in the following configurations:
svn 1.9.7
svn 1.7.14
Everything worked correctly, so I'd say everything is good to go for 19.2.
I'll test one final time when 19.2 is released to give a final smoke test as it goes into the wild.
Great! Thank you! 馃憤
Most helpful comment
It's worth pointing out that ptys aren't an option on Windows.