Author jgehrcke
Recipients dries_desmet, dstufft, eric.araujo, geoffreyspear, giampaolo.rodola, jaraco, jgehrcke, loewis, ncoghlan, tarek, techtonik
Date 2015-02-03.23:09:52
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1423004994.49.0.0179395680429.issue18454@psf.upfronthosting.co.za>
In-reply-to
Content
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
History
Date User Action Args
2015-02-03 23:09:54jgehrckesetrecipients: + jgehrcke, loewis, jaraco, ncoghlan, techtonik, giampaolo.rodola, tarek, eric.araujo, geoffreyspear, dstufft, dries_desmet
2015-02-03 23:09:54jgehrckesetmessageid: <1423004994.49.0.0179395680429.issue18454@psf.upfronthosting.co.za>
2015-02-03 23:09:54jgehrckelinkissue18454 messages
2015-02-03 23:09:52jgehrckecreate