Hi Steve and Jelmer,

Nice find Jelmer. It's a shame (but not a surprise :/), that we don't seem to have a test for the sourcepackagenames_only option. It's only referenced in the config. Steve, feel free to add one before fixing the issue :) Actually, I can't see it referenced anywhere in the lp-production-configs? Do we even use it? (if not, we could remove it).

I've just got a few style questions below.

Thanks for the fix!

> === modified file 'lib/lp/soyuz/scripts/gina/archive.py'
> --- lib/lp/soyuz/scripts/gina/archive.py        2009-06-25 04:06:00 +0000
> +++ lib/lp/soyuz/scripts/gina/archive.py        2010-03-17 11:15:37 +0000
> @@ -207,7 +207,7 @@
>                     log.exception("Invalid Sources stanza in %s" %
>                                   info_set.sources_tagfile)
>                     continue
> -                self.src_map[src_name] = src_tmp
> +                self.src_map.setdefault(src_name, []).append(src_tmp)

Not sure what you prefer, but I think this is more readable like this:

http://pastebin.ubuntu.com/399865/

which also passes the test. Up to you.


> === modified file 'scripts/gina.py'
> --- scripts/gina.py     2009-10-13 14:38:07 +0000
> +++ scripts/gina.py     2010-03-17 11:15:37 +0000
> @@ -229,44 +229,45 @@
>     npacks = len(packages_map.src_map)
>     log.info('%i Source Packages to be imported' % npacks)
>
> -    for source in sorted(packages_map.src_map.values(),
> -                         key=lambda x: x.get("Package")):
> -        count += 1
> -        package_name = source.get("Package", "unknown")
> -        try:
> +    for list_source in sorted(packages_map.src_map.values(),
> +                         key=lambda x: x[0].get("Package")):

The python style guide used to allow the previous indentation, but it now should be:

   for list_source in sorted(
       packages_map.src_map.values(), key=lambda x: x[0].get("Package")):

See https://dev.launchpad.net/PythonStyleGuide#Multiline%20function%20calls

Or another option, if you rename your var 'list_source' to simply 'source_list', then you can create your source_lists prior to the iteration (for source_list in source_lists:). See what you think.

> +        for source in list_source:
> +            count += 1
> +            package_name = source.get("Package", "unknown")
>             try:
> -                do_one_sourcepackage(source, kdb, package_root, keyrings,
> +                try:
> +                    do_one_sourcepackage(source, kdb, package_root, keyrings,
>                                      importer_handler)

Similar, if you need to wrap the args to a fn call, they should start on the next line.

> -            except psycopg2.Error:
> -                log.exception("Database error: unable to create "
> +                except psycopg2.Error:
> +                    log.exception("Database error: unable to create "
>                               "SourcePackage for %s. Retrying once.."
>                               % package_name)

And again.

> -                importer_handler.abort()
> -                time.sleep(15)
> -                do_one_sourcepackage(source, kdb, package_root, keyrings,
> +                    importer_handler.abort()
> +                    time.sleep(15)
> +                    do_one_sourcepackage(source, kdb, package_root, keyrings,
>                                      importer_handler)

Here too.

> -        except (InvalidVersionError, MissingRequiredArguments,
> +            except (InvalidVersionError, MissingRequiredArguments,
>                 DisplayNameDecodingError):
> -            log.exception("Unable to create SourcePackageData for %s" %
> -                          package_name)
> -            continue
> -        except (PoolFileNotFound, ExecutionError):
> -            # Problems with katie db stuff of opening files
> -            log.exception("Error processing package files for %s" %
> -                          package_name)
> -            continue
> -        except psycopg2.Error:
> -            log.exception("Database errors made me give up: unable to create "
> +                log.exception("Unable to create SourcePackageData for %s" %
> +                          package_name)

Ditto.

> +                continue
> +            except (PoolFileNotFound, ExecutionError):
> +                # Problems with katie db stuff of opening files
> +                log.exception("Error processing package files for %s" %
> +                          package_name)

Ditto.

> +                continue
> +            except psycopg2.Error:
> +                log.exception("Database errors made me give up: unable to create "
>                           "SourcePackage for %s" % package_name)
> -            importer_handler.abort()
> -            continue
> -        except MultiplePackageReleaseError:
> -            log.exception("Database duplication processing %s" %
> +                importer_handler.abort()
> +                continue
> +            except MultiplePackageReleaseError:
> +                log.exception("Database duplication processing %s" %
>                           package_name)

Ditto.

> -            continue
> +                continue
>
> -        if COUNTDOWN and count % COUNTDOWN == 0:
> -            log.warn('%i/%i sourcepackages processed' % (count, npacks))
> +            if COUNTDOWN and count % COUNTDOWN == 0:
> +                log.warn('%i/%i sourcepackages processed' % (count, npacks))
>
>
>  def do_one_sourcepackage(source, kdb, package_root, keyrings,
>
>