Merge lp:~spiv/bzr-builddeb/unbound-local-623736 into lp:bzr-builddeb

[Andrew Bennetts]

This should fix an UnboundLocalError in the case where file_ids_from is an empty list.

I took a look at writing a test, but I couldn't find unit tests for the bzrtools_import functions, and I'm not sure how to provoke that situation via less direct APIs. I can at least say it doesn't break any tests :)

[Robert Collins (lifeless)]

If you look back a few commits in the file you'll see a change by me;
that commit has tests.

-Rob

[James Westby (james-w)]

On Wed, 25 Aug 2010 05:20:55 -0000, Andrew Bennetts <email address hidden> wrote:
> Andrew Bennetts has proposed merging lp:~spiv/bzr-builddeb/unbound-local-623736 into lp:bzr-builddeb.
>
> Requested reviews:
> Bzr-builddeb-hackers (bzr-builddeb-hackers)
> Related bugs:
> #623736 bzr crashed with UnboundLocalError in _import_archive()
> https://bugs.launchpad.net/bugs/623736
>
>
> This should fix an UnboundLocalError in the case where file_ids_from
> is an empty list.

Thanks.

> I took a look at writing a test, but I couldn't find unit tests for
> the bzrtools_import functions, and I'm not sure how to provoke that
> situation via less direct APIs. I can at least say it doesn't break
> any tests :)

Feel free to add a TestCase for testing bzrtools_import directly. That
would seem to be the best thing here.

It shouldn't be hard to set up the conditions, as you can import a
directory, and file_ids_from is just passed in directly.

Let me know if you would like me to do it.

Thanks,

James

[James Westby (james-w)]

Hi,

This has been superseded with my work on this code today.

Thanks for the contribution.

James

review: Disapprove

Unmerged revisions

476. By Andrew Bennetts on 2010-08-25

Fix UnboundLocalError in _import_archive.

=== modified file 'bzrtools_import.py'
--- bzrtools_import.py	2010-08-19 19:28:08 +0000
+++ bzrtools_import.py	2010-08-25 05:20:55 +0000
@@ -260,12 +260,13 @@
                             tt.delete_versioned(trans_id)
                         trans_id = tt.trans_id_file_id(found_file_id)
                     break
-            if not found_file_id and not existing_file_id:
-                # No file_id in any of the source trees and no file id in the base
-                # tree.
-                name = basename(member.name.rstrip('/'))
-                file_id = generate_ids.gen_file_id(name)
-                tt.version_file(file_id, trans_id)
+            else:
+                if not existing_file_id:
+                    # No file_id in any of the source trees and no file id in
+                    # the base tree.
+                    name = basename(member.name.rstrip('/'))
+                    file_id = generate_ids.gen_file_id(name)
+                    tt.version_file(file_id, trans_id)
             path = tree.abspath(relative_path)
             if member.name in seen:
                 if tt.final_kind(trans_id) == 'file':