Uploading a new release of one of my libraries to PYPI I got this error:

"""
Submitting dist/bsddb3-5.1.1.tar.gz to http://pypi.python.org/pypi
Upload failed (400): A file named "bsddb3-5.1.1.tar.gz" already exists for  bsddb3-5.1.1. To fix problems with that file you should create a new release.
Traceback (most recent call last):
  File "setup.py", line 5, in <module>
    import setup2
  File "/home/pybsddb/setup2.py", line 415, in <module>
    'Programming Language :: Python :: 3.2',
  File "/usr/local/lib/python2.7/distutils/core.py", line 152, in setup
    dist.run_commands()
  File "/usr/local/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/usr/local/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/usr/local/lib/python2.7/distutils/command/upload.py", line 60, in run
    self.upload_file(command, pyversion, filename)
  File "/usr/local/lib/python2.7/distutils/command/upload.py", line 189, in upload_file
    self.announce('-'*75, result.read(), '-'*75)
UnboundLocalError: local variable 'result' referenced before assignment
"""

Checking the code, I see this:

"""
        # send the data
        try:
            result = urlopen(request)
            status = result.getcode()
            reason = result.msg
        except socket.error, e:
            self.announce(str(e), log.ERROR)
            return
        except HTTPError, e:
            status = e.code
            reason = e.msg

        if status == 200:
            self.announce('Server response (%s): %s' % (status, reason),
                          log.INFO)
        else:
            self.announce('Upload failed (%s): %s' % (status, reason),
                          log.ERROR)
        if self.show_response:
            self.announce('-'*75, result.read(), '-'*75)
"""

Here, if we selected "show_response" *AND* some error happens, "result" will be undefined and the last line will fail.

This bug was introduced in Python 2.7. It used to work correctly under Python 2.6.

I didn't check Python 3.x.

I think this bug is trivial to reproduce: just try to upload any file with no permissions for it, using "--show_response" command line option.

Thanks for the report.  Can you paste your message and exact Python version on #9199?

This is not issue9199, even covering the same line code.

In this case the problem is an exception leaving a local variable undefined, crashing later.

To better show what the problem is, here's a change that would fix the problem (albeit in an ugly way):

--- upload.py   2010-07-07 20:16:33.000000000 -0400
+++ /home/pje/upload.new        2010-11-09 14:30:21.000000000 -0500
@@ -167,6 +167,9 @@
 
         request = Request(self.repository, data=body,
                           headers=headers)
+
+        result = None
+
         # send the data
         try:
             result = urlopen(request)
@@ -186,4 +189,4 @@
             self.announce('Upload failed (%s): %s' % (status, reason),
                           log.ERROR)
         if self.show_response:
-            self.announce('-'*75, result.read(), '-'*75)
+            self.announce('-'*75, result.read() if result is not None else 'ERROR', '-'*75)

Btw, a quick review of the 3.x trunk code for this shows that it does *not* have this problem; this is specific to 2.7 and AFAICT *only* 2.7.

(This problem was introduced in r73436, btw, as of the move to urllib2 vs. httplib.)

Well, I fixed the problem moving the "if" that instances "result" even though it getting HTTPError into the "try".

------------------------------------------------------
        # send the data
        try:
            result = urlopen(request)
            status = result.getcode()
            reason = result.msg
            if self.show_response:
                self.announce('-'*75, result.read(), '-'*75)
        except socket.error, e:
            self.announce(str(e), log.ERROR)
            return
        except HTTPError, e:
            status = e.code
            reason = e.msg

        if status == 200:
            self.announce('Server response (%s): %s' % (status, reason),
                          log.INFO)
        else:
            self.announce('Upload failed (%s): %s' % (status, reason),
                          log.ERROR)

-------------------------------------------------

So now it explains the real error:

###
python setup.py sdist upload
running sdist
running check
warning: sdist: manifest template 'MANIFEST.in' does not exist (using default file list)

writing manifest file 'MANIFEST'
creating Python-2.7
creating Python-2.7/Lib
creating Python-2.7/Tools
creating Python-2.7/Tools/scripts
making hard links in Python-2.7...
hard linking README -> Python-2.7
'_struct.c' not a regular file -- skipping
hard linking setup.py -> Python-2.7
hard linking Lib/smtpd.py -> Python-2.7/Lib
hard linking Tools/scripts/2to3 -> Python-2.7/Tools/scripts
hard linking Tools/scripts/idle -> Python-2.7/Tools/scripts
hard linking Tools/scripts/pydoc -> Python-2.7/Tools/scripts
Creating tar archive
removing 'Python-2.7' (and everything under it)
running upload
Submitting dist/Python-2.7.tar.gz to http://pypi.python.org/pypi
Upload failed (401): Unauthorized

###


Is this ok?

Thank you for the fix Priscila.  Can you submit it as a diff file?  (guidelines on http://www.python.org/dev/patches/)

Tarek: I wonder if we should backport pydoc_server from d2 to test behavior bugs like this one.

It looks to me as though this patch reintroduces issue9199, as it passes multiple arguments to self.announce() once again.  The patch needs to be made against the SVN version of Python, not the released version.

Here's an updated patch, which fixes the passing of multiple arguments to self.announce, based on Priscila's fix.

Thanks Daniel.  I have good and bad news for this patch.

The bad news is that I don’t want to accept patches without tests.  We need to stop guessing or experimenting with the real PyPI to fix bugs.

The good news is that we have a mock PyPI server in distutils2.  Its use is described at http://distutils2.notmyidea.org/library/distutils2.tests.pypi_server.html and there are examples in the current test suite.  Instructions for patching distutils2 are at http://wiki.python.org/moin/Distutils/FixingBugs .  When a patch that includes a test is written, I will backport the relevant parts to distutils1.

Given that this is a pure bugfix to revert a problem in 2.7 -- where no *new* development is being done -- a test isn't actually needed for this patch.  There is no point in holding up a distutils1 fix for distutils2's benefit.

Daniel: thanks for the patch, it looks good to me.

Committed Daniel's patch to r86978 in the 2.7 maintenance branch.

(The 2.x trunk still has this bug, but is permanently closed to new checkins.)

> a test isn't actually needed for this patch.

This is incorrect.

Philip, Eric is currently assigned to this issue, and was working on a test, obviously.

It means that commiting a fix without a test without asking him first is is quite rude.

He and I are maintaining Distutils. Your help is welcome but please do not commit in this area without discussion in particular when the bug is assigned to someone. 

Now if you could provide a test for your change, we would highly appreciate it. Thanks

I'll gladly work on the test, unless someone is working on it already. I
apologize for not sending it originally along with the patch. Since Eric
only requested the diff, I thought a test wasn't needed.

Cheers,
Daniel

On Fri, Dec 3, 2010 at 1:58 PM, Tarek Ziadé <report@bugs.python.org> wrote:

>
> Tarek Ziadé <ziade.tarek@gmail.com> added the comment:
>
> Philip, Eric is currently assigned to this issue, and was working on a
> test, obviously.
>
> It means that commiting a fix without a test without asking him first is is
> quite rude.
>
> He and I are maintaining Distutils. Your help is welcome but please do not
> commit in this area without discussion in particular when the bug is
> assigned to someone.
>
> Now if you could provide a test for your change, we would highly appreciate
> it. Thanks
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue10367>
> _______________________________________
>

Thanks Daniel.  The links in my last message should get you started, and any questions are welcome.

You may be interested in this sprint: http://wiki.montrealpython.org/index.php/Packaging_no.10  I will be on IRC for some hours that night.

Whoops, my bad...  I misread Eric's earlier message as throwing it back onto *Daniel* to produce a test, not that *he* (Eric) was working on the test.

IOW, I thought that progress had been stalled a second time on this, and went ahead to pick up the slack.  Sorry about that.

You understood right: I asked for a test using the mock PyPI server in distutils2.  “I can’t do it” is a valid reply, in which case it would have been picked up by someone else or me.

You could say that progress has stalled, but there was no urgency, given that the next 2.7 release is not tomorrow.  We have more than a hundred bugs on our hands, so waiting a few days for a reply is not a problem :)

Misunderstandings aside, thanks for your help.

Ugh sorry I thought Eric was working on a test. I  misunderstood.

The urgency was only that I didn't want the other contributors to this issue to feel as though the bar on their contributions were being raised higher every time they jumped the previous bar.

IOW, I did it to make them feel like somebody was doing *something*.  In hindsight, that was not necessarily the best tactic.  ;-)

(Given the nature of the bugfix and the bugfix-only status of the 2.7 line, I don't think the lack of an automated test is a big deal; the odds that anything other than trivial bugfixes will be applied to this module in the future seem negligible to me.  At least, I would *hope* that it is not planned to destabilize this module in 2.7.x any further than it already was by the changes that broke it in the first place.)

Here goes the test. Feel free to make any comments, critiques or suggestions, since this is my first take on this code base.

Hey Éric,

Just wanted to check with you about the status of this issue. Any chance this could be reviewed? It's been idle since December and it's still listed as "needs patch".

Thanks,
Daniel

Sorry for overlooking this.  The test is good, I could trigger the bug with it and then fix it with the patch.

Would you mind adding the same test for upload_docs?  The code was originally copied from upload, so we should test it too.


Philip: I understand your motives but respectfully disagree with them.

> The urgency was only that I didn't want the other contributors to
> this issue to feel as though the bar on their contributions were
> being raised higher every time they jumped the previous bar.
I try not to put off people by demanding a test, but asking if they want to write one and providing guidance.  I think it’s worked quite well with a lot of our recent contributors from Montréal and other places, but I’m open to critiques about my tone or phrasing, I don’t want to sound like I require a huge amount of work from people who report bugs.

> I did it to make them feel like somebody was doing *something*.  In
> hindsight, that was not necessarily the best tactic.  ;-)
Well, I don’t really care about appearances or feelings; the truth of the situation is that there are a lot of easy and hard bugs and a few people to look at them, I don’t think we should mask it.  Prior to December, I was reactive in this report.

> (Given the nature of the bugfix and the bugfix-only status of the 2.7
> line, I don't think the lack of an automated test is a big deal; the
> odds that anything other than trivial bugfixes will be applied to
> this module in the future seem negligible to me.
You know more about distutils than me, but from the short time I’ve worked with this codebase, read bug reports and mailing list archives, I’ve found that it’s too optimistic to change something without an automated test.  I have committed a bug in upload.py once :)  That’s why I decided to draw a line and stop guessing and hoping things would be right and rely on regression tests.  This adds a bit of work for our contributors, but my intention is not to put off people, but to make things more robust.  We owe that to our users and to future distutils contributors.

I do hope this explains why seemingly trivial changes require a test too.  I’d also like to thank you for your help in this report.

This has been fixed in issue 12853. The relevant line is here: https://hg.python.org/cpython/file/2.7/Lib/distutils/command/upload.py#l179

Closing this one as a duplicate.