Jan-Philip Gehrcke added the comment:
First, I want to address the situation in 2.7. Please have a look at my patch
and my reasoning.
This is my setup.py test file content:
from distutils.core import setup
setup(name='foo', version='1.0', packages=['testpackage'])
This is the current situation for 2.7 head, in summary:
1) Without a proper .pypirc file in place, uploading fails with
"Upload failed (401): You must be identified to edit
package information"
2) With a valid .pypirc in place which does not define a password
this exception is raised:
TypeError: cannot concatenate 'str' and 'NoneType' objects
(1) happens due to the fact that the default username/password strings are
empty, but the request is still fired against PyPI. This obviously fails. This
could be fixed by prompting for both, username and password or by aborting
before sending the request. However, this would be quite a significant change
in behavior. I think we should not do this, because AFAIK the 2.7 documentation
does not specify what the behavior should be *without* existing .pypirc file.
So, I think there might not be enough reason to consider such a change a bug
fix. What do you think?
(2) violates the docs and I guess this is what Nick meant when he said that the
stdlib should be updated. I propose a patch so that what the docs say ("If
omitted, the user will be prompt to type it when needed") is what actually
happens. The patch consists of two parts:
Part A: issue18454_py27_prompt.patch
------------------------------------
This uses getpass/sys.stdin.readline() for retrieving the password, depending
on whether the process is attached to a tty or not, respectively. The config
parsing and value propagation is a little messy in the command/upload.py
module, so I have also added comments for clarity and simplified the logic a
little.
Part B: issue18454_py27_prompt_test.patch
-----------------------------------------
Here it gets messy. Normally, we want to have a new test case that fails before
applying patch A, and succeeds after applying it. However, here we actually
have to deal with an already existing bogus test case which succeeds before and
fails after A gets applied. A very bad sign, usually. But in this case I am of
the strong opinion that we can and have to change the test implementation,
because:
- the test actually makes sure that `cmd.password` is set to None when
`cmd.finalize_options()` has done its work. However,`cmd.password` being
None leads to the TypeError exception described above. That is, this test
case basically ensures the very existance of the bug that we want to fix.
- it has a suspiciously useless name. It is called `test_saved_password`,
but actually tests the case when no password is saved in .pypirc.
- this test actually has quite low code coverage. AFAIK it *only* tests
the behavior of `cmd.finalize_options()`. We want to change this behavior,
so we have to consider changing the test.
My patch modifies this test case so that it has a proper name and makes sure
that `cmd.finalize_options()` obtains a password from stdin. The involvement of
stdin makes testing a little complicated. In a proper testing environment, we'd
probably spawn a subprocess and provide it with some real stdin data. The
distutils upload command test structure seems not to be prepared for this
endeavor, so I have decided to temporarily/locally patch sys.stdin, and ensure
restoration with a try/finally clause.
With A and B in place, all distutils unit tests validate. I used two methods of
invocation, for covering the two cases (with and without attached tty):
$ nohup python test_distutils.py 2>&1 > out.log &
$ python test_distutils.py
----------
keywords: +patch
Added file: http://bugs.python.org/file38005/issue18454_py27_prompt.patch
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue18454>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
