Comment 8 for bug 717879

On Thu, Mar 31, 2011 at 08:42:41PM -0000, Barry Warsaw wrote:
> Why do you add the empty string to the suffix list in the for loop? I
> see that the original code unconditionally adds the unsuffixed value of
> open_tag_file() if both .bz2 and .gz fail. Is that the real source of
> the bug (iow, open_tag_file(mirror, '') may not produce a non-None
> value)?

The bug is that open_tag_file raises an exception when the file is not
found, and we were only passing the exception for the ".bz2" and ".gz"
cases - which means that when we fall through to the uncompressed case, if
the file isn't there, we get an immediate exception where what we want is to
only raise an exception if we don't find a file for that component on *any*
mirror.

> Notice that I moved tag_files=None to inside the inner loop. I think
> the way the old code was written, it was possible to add a tag_file more
> than once inadvertently. Let's say mirrors=('A', 'B').
> open_tag_file('A', '.gz') returns something non-None and it gets added
> to tag_files. Now mirror 'B' is processed, but all its suffixes raise
> IOError. When the inner loop for mirror='B' completes, tag_file is
> still set to the value it had during the mirror='A' iteration and it
> gets added twice.

I'm failing to see how this would happen. tag_file is initialized to None
for each mirror, which is still the value it should have at the end of the
loop if open_tag_file() fails for all B, right?

Cheers,
--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>