Debian Bug report logs - #562398
libdebian-installer: strange behavior with more than one version of a package in a Packages file

version graph

Package: libdebian-installer4-udeb; Maintainer for libdebian-installer4-udeb is Debian Install System Team <debian-boot@lists.debian.org>; Source for libdebian-installer4-udeb is src:libdebian-installer.

Reported by: Frans Pop <elendil@planet.nl>

Date: Thu, 24 Dec 2009 12:21:02 UTC

Severity: important

Tags: patch

Found in version libdebian-installer/0.69

Reply or subscribe to this bug.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Debian Install System Team <debian-boot@lists.debian.org>:
Bug#562398; Package anna. (Thu, 24 Dec 2009 12:21:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Frans Pop <elendil@planet.nl>:
New Bug report received and forwarded. Copy sent to Debian Install System Team <debian-boot@lists.debian.org>. (Thu, 24 Dec 2009 12:21:05 GMT) Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Frans Pop <elendil@planet.nl>
To: Debian BTS Submit <submit@bugs.debian.org>
Subject: anna: fails if multiple versions of a udeb in Packages file
Date: Thu, 24 Dec 2009 13:18:12 +0100
Package: anna
Severity: serious

Yesterday I uploaded some packages, including base-installer and hw-detect. 
Today during a test install from a local build this resulted in:

                     Failed to load installer component
       loading base-installer failed for unknown reasons. Aborting.

The syslog does not show much info, but this is interesting:
[...]
anna: DEBUG: retrieving base-installer 1.104
anna: DEBUG: retrieving base-installer 1.104
anna: DEBUG: retrieving bootstrap-base 1.104
[...]
anna: DEBUG: retrieving disk-detect 1.74
anna: DEBUG: retrieving disk-detect 1.74
[...]

So the error is probably the result of anna trying to install the same 
package twice.

I ran anna a second time and that finished without errors and the rest of 
the installation also completed without problems, so the impact is 
limited.

The packages file on my local mirror shows the cause of the problem. It 
contains two versions of both base-installer (1.103, 1.104) and 
disk-detect (1.73, 1.74). Both are arch:all packages, so this is probably 
the simple result of the recent changes in the archive and the packages 
not yet being built for all arches.

We need to at least make sure anna does not try to process the same package 
twice. It already seems to return the correct (highest) version.

The actual fix is probably in libd-i rather than anna itself.

This issue will only manifest for packages that have both arch:any and 
arch:all udebs, and only while it has not been built for all arches.
So the problem will be fairly rare [1], but IMO it should still be fixed as 
it looks extremely fatal.




Information forwarded to debian-bugs-dist@lists.debian.org, Debian Install System Team <debian-boot@lists.debian.org>:
Bug#562398; Package anna. (Thu, 24 Dec 2009 12:33:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Frans Pop <elendil@planet.nl>:
Extra info received and forwarded to list. Copy sent to Debian Install System Team <debian-boot@lists.debian.org>. (Thu, 24 Dec 2009 12:33:03 GMT) Full text and rfc822 format available.

Message #10 received at 562398@bugs.debian.org (full text, mbox):

From: Frans Pop <elendil@planet.nl>
To: 562398@bugs.debian.org
Subject: Re: Bug#562398: anna: fails if multiple versions of a udeb in Packages file
Date: Thu, 24 Dec 2009 13:30:04 +0100
On Thursday 24 December 2009, Frans Pop wrote:
> So the error is probably the result of anna trying to install the same
> package twice.

The exit code of anna was 8, which matches an error during unpack or 
configure.




Information forwarded to debian-bugs-dist@lists.debian.org, Debian Install System Team <debian-boot@lists.debian.org>:
Bug#562398; Package anna. (Sun, 02 May 2010 21:18:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to John Wright <jsw@debian.org>:
Extra info received and forwarded to list. Copy sent to Debian Install System Team <debian-boot@lists.debian.org>. (Sun, 02 May 2010 21:18:05 GMT) Full text and rfc822 format available.

Message #15 received at 562398@bugs.debian.org (full text, mbox):

From: John Wright <jsw@debian.org>
To: Frans Pop <elendil@planet.nl>
Cc: 562398@bugs.debian.org
Subject: Re: Bug#562398: anna: fails if multiple versions of a udeb in Packages file
Date: Sun, 2 May 2010 15:08:08 -0600
On Thu, Dec 24, 2009 at 01:30:04PM +0100, Frans Pop wrote:
> On Thursday 24 December 2009, Frans Pop wrote:
> > So the error is probably the result of anna trying to install the same
> > package twice.
> 
> The exit code of anna was 8, which matches an error during unpack or 
> configure.

I've run into this with a custom installer suite with udebs overriding
those in lenny's main/debian-installer.  My solution at the time was
just to filter the Packages file in net-retriever so that anna was only
given one version of any particular package.  (Ubuntu did something
similar, see [1].)  However, it should probably be fixed in anna or
libdebian-installer itself. :)

Specifically, what happens is that anna unpacks all the packages in one
batch, and then it configures all of them.  But while unpacking another
version of a package while another vesrion is in an
unpacked-but-not-configured state is ok, it's not ok to configure a
package that's already in the configured state.  So if a package is in
the list twice, it fails at the second configure for that package.

I'm trying to better understand anna and libd-i to come up with a
suitable patch...

 [1]: https://bugs.launchpad.net/ubuntu/+source/net-retriever/+bug/234486

-- 
John Wright <jsw@debian.org>




Information forwarded to debian-bugs-dist@lists.debian.org, Debian Install System Team <debian-boot@lists.debian.org>:
Bug#562398; Package anna. (Mon, 03 May 2010 08:18:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to John Wright <jsw@debian.org>:
Extra info received and forwarded to list. Copy sent to Debian Install System Team <debian-boot@lists.debian.org>. (Mon, 03 May 2010 08:18:02 GMT) Full text and rfc822 format available.

Message #20 received at 562398@bugs.debian.org (full text, mbox):

From: John Wright <jsw@debian.org>
To: Frans Pop <elendil@planet.nl>
Cc: 562398@bugs.debian.org, control@bugs.debian.org
Subject: Re: Bug#562398: anna: fails if multiple versions of a udeb in Packages file
Date: Mon, 3 May 2010 02:15:06 -0600
[Message part 1 (text/plain, inline)]
reassign 562398 libdebian-installer4-udeb
found 562398 0.69
retitle 562398 libdebian-installer: strange behavior with more than one version of a package in a Packages file
affects 562398 + anna
tags 562398 + patch
thanks

On Sun, May 02, 2010 at 03:08:08PM -0600, John Wright wrote:
> Specifically, what happens is that anna unpacks all the packages in one
> batch, and then it configures all of them.  But while unpacking another
> version of a package while another vesrion is in an
> unpacked-but-not-configured state is ok, it's not ok to configure a
> package that's already in the configured state.  So if a package is in
> the list twice, it fails at the second configure for that package.

Here's what's happing in libd-i:  Upon encountering a Packages stanza
with the same Package field as one it's previously seen,
di_packages_parser_read_name sets the data pointer the rest of the
parsing functions will use to the previously-used di_packages pointer.
At first glance, this would appear simply to prefer packages that appear
later in the Packages file, irrespective of version.  Unfortunately, it
appends the di_package to the package list
(&parser->data->packages->list) whether it's a newly allocated one or an
old one.  So while only one actual di_package exists for a particular
package name, it might appear multiple times in the list.

The simple way to fix the anna issue is to make sure we only append
newly allocated di_package objects to the list.  But it would be nice to
favor new versions rather than whatever shows up latest in the Packages
file (for example, to facilitate #389430 or LP#234486).

I've attached a quick reproducer to demonstrate the issue, and a patch.
I would prefer if the version comparison could happen during the
Packages file parsing, rather than after the fact (since this way
requires creating a temporary hash table and traversing the list a
couple of extra times), but that change would seem to be very invasive.
In any case, after pounding my head for a couple of hours, I couldn't
figure out how to do it any better with the current parsing
infrastructure. :)

-- 
John Wright <jsw@debian.org>
[test.c (text/x-csrc, inline)]
#include <stdio.h>
#include <debian-installer/system/packages.h>
#include <debian-installer/slist.h>

int main(int argc, char **argv)
{
    di_packages_allocator *allocator = di_system_packages_allocator_alloc();
    di_packages *packages;
    di_package *package, *last_package = NULL;
    di_slist_node *node;
    const char *packages_file;

    if (argc == 2)
        packages_file = argv[1];
    else {
        printf("Usage: %s <packages-file>\n", argv[0]);
        return 1;
    }

    packages = di_system_packages_read_file(packages_file, allocator);

    for (node = packages->list.head; node; node = node->next) {
        package = node->data;
        if (!package)
            printf("package == NULL\n");
        if (package == last_package)
            printf("Eek!  package == last_package\n");
        printf("Package: %s\n", package->package);
        printf("Version: %s\n", package->version);
        printf("\n");
        last_package = package;
    }

    return 0;
}
[Packages-test (text/plain, inline)]
Package: baz
Version: 0.0~hpde1

Package: bang
Version: 0.133

Package: baz
Version: 0.0hpde1

Package: foo
Version: 1.2.3

Package: foo
Version: 1.2.3hpde1

Package: bar
Version: 2.3.4hpde1

Package: bar
Version: 2.3.4

Package: baz
Version: 0.1
[0001-Filter-out-old-versions-after-parsing-a-Packages-fil.patch (text/x-diff, inline)]
From 64d06247e4fdb35fd7f33eb7020ec84584e23b42 Mon Sep 17 00:00:00 2001
From: John Wright <john@johnwright.org>
Date: Sun, 2 May 2010 22:15:11 -0600
Subject: [PATCH] Filter out old versions after parsing a Packages file

The previous behavior favored packages showing up later in the Packages
file rather than packages with later versions, and wound up putting
multiple entries for the same package in the di_packages list.  This
patch ensures a new di_package is created for each stanza, and then
filters out all but the latest versions before returning the di_packages
pointer.

There's a small memory leak for each di_slist_node corresponding to an
old package version.  It can't be helped, as far as I can tell, because
of how the memory for those are allocated (using mem_chunk).  The bulk
of the memory used for those is freed, just not the di_slist_node glue.

This patch also fixes a bug in di_hash_table_insert, where the key was
not being reset after potentially destroying the old key and changing
the value for a key (which may contain the key itself).
---
 include/debian-installer/packages.h |    4 ++
 src/hash.c                          |    1 +
 src/packages.c                      |   69 +++++++++++++++++++++++++++++++++++
 src/packages_parser.c               |    8 +++-
 4 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/include/debian-installer/packages.h b/include/debian-installer/packages.h
index c5e4576..0d7dd1c 100644
--- a/include/debian-installer/packages.h
+++ b/include/debian-installer/packages.h
@@ -71,6 +71,10 @@ void di_packages_append_package (di_packages *packages, di_package *package, di_
 di_package *di_packages_get_package (di_packages *packages, const char *name, size_t n);
 di_package *di_packages_get_package_new (di_packages *packages, di_packages_allocator *allocator, char *name, size_t n);
 
+di_hash_table *di_packages_get_latest_map (di_packages *packages);
+void di_packages_filter_older_versions(di_packages *packages);
+void di_packages_update_hash_table(di_packages *packages);
+
 di_slist *di_packages_resolve_dependencies (di_packages *packages, di_slist *list, di_packages_allocator *allocator);
 di_slist *di_packages_resolve_dependencies_array (di_packages *packages, di_package **array, di_packages_allocator *allocator);
 void di_packages_resolve_dependencies_mark (di_packages *packages);
diff --git a/src/hash.c b/src/hash.c
index 302ea5a..78a2f5d 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -192,6 +192,7 @@ void di_hash_table_insert (di_hash_table *hash_table, void *key, void *value)
     if (hash_table->value_destroy_func)
       hash_table->value_destroy_func ((*node)->value);
 
+    (*node)->key = key;
     (*node)->value = value;
   }
   else
diff --git a/src/packages.c b/src/packages.c
index 699f82c..e0f9d59 100644
--- a/src/packages.c
+++ b/src/packages.c
@@ -164,6 +164,75 @@ di_package *di_packages_get_package_new (di_packages *packages, di_packages_allo
   return ret;
 }
 
+di_hash_table *di_packages_get_latest_map (di_packages *packages)
+{
+  di_slist_node *node;
+  di_package *package, *latest_package;
+  di_package_version *this_version = NULL, *latest_version = NULL;
+  di_hash_table *latest = di_hash_table_new (di_rstring_hash, di_rstring_equal);
+
+  for (node = packages->list.head; node; node = node->next) {
+    package = node->data;
+    latest_package = di_hash_table_lookup(latest, &package->key);
+    this_version = latest_version = NULL;
+
+    /* With missing or broken versions, just take the last-added package */
+    if (!latest_package ||
+        !package->version || !latest_package->version ||
+        !(this_version = di_package_version_parse (package)) ||
+        !(latest_version = di_package_version_parse (latest_package)) ||
+        di_package_version_compare (this_version, latest_version) > 0)
+    {
+      di_hash_table_insert (latest, &package->key, package);
+      di_free (this_version);
+      di_free (latest_version);
+    }
+  }
+
+  return latest;
+}
+
+void di_packages_filter_older_versions (di_packages *packages)
+{
+  di_slist_node *node, *last_node;
+  di_package *package, *latest_package;
+  di_hash_table *latest = di_packages_get_latest_map (packages);
+
+  /* Remove non-latest packages from the list */
+  last_node = NULL;
+  for (node = packages->list.head; node; node = node->next) {
+    package = node->data;
+    latest_package = di_hash_table_lookup (latest, &package->key);
+    if (package != latest_package) {
+      /* Remove it from the list */
+      if (last_node)
+        last_node->next = node->next;
+      else
+        packages->list.head = node->next;
+
+      if (!node->next)
+        packages->list.bottom = last_node;
+
+      di_package_destroy(package);
+    }
+    else
+      last_node = node;
+  }
+
+  di_hash_table_destroy (latest);
+}
+
+void di_packages_update_hash_table (di_packages *packages)
+{
+  di_slist_node *node;
+  di_package *package;
+
+  for (node = packages->list.head; node; node = node->next) {
+    package = node->data;
+    di_hash_table_insert (packages->table, &package->key, package);
+  }
+}
+
 bool di_packages_resolve_dependencies_recurse (di_packages_resolve_dependencies_check *r, di_package *package, di_package *dependend_package)
 {
   di_slist_node *node;
diff --git a/src/packages_parser.c b/src/packages_parser.c
index 5b4ccbd..24f303d 100644
--- a/src/packages_parser.c
+++ b/src/packages_parser.c
@@ -182,6 +182,9 @@ di_packages *di_packages_special_read_file (const char *file, di_packages_alloca
 
   di_parser_info_free (info);
 
+  di_packages_filter_older_versions (data.packages);
+  di_packages_update_hash_table (data.packages);
+
   return data.packages;
 }
 
@@ -240,8 +243,9 @@ void di_packages_parser_read_name (data, fip, field_modifier, value, user_data)
   void *user_data;
 {
   internal_di_package_parser_data *parser_data = user_data;
-  di_package *p;
-  p = di_packages_get_package_new (parser_data->packages, parser_data->allocator, value->string, value->size);
+  di_package *p = di_package_alloc (parser_data->allocator);
+  p->key.string = di_stradup (value->string, value->size);
+  p->key.size = value->size;
   p->type = di_package_type_real_package;
   di_slist_append_chunk (&parser_data->packages->list, p, parser_data->allocator->slist_node_mem_chunk);
   *data = p;
-- 
1.6.5


Bug reassigned from package 'anna' to 'libdebian-installer4-udeb'. Request was from John Wright <jsw@debian.org> to control@bugs.debian.org. (Mon, 03 May 2010 08:18:04 GMT) Full text and rfc822 format available.

Bug Marked as found in versions libdebian-installer/0.69. Request was from John Wright <jsw@debian.org> to control@bugs.debian.org. (Mon, 03 May 2010 08:18:05 GMT) Full text and rfc822 format available.

Changed Bug title to 'libdebian-installer: strange behavior with more than one version of a package in a Packages file' from 'anna: fails if multiple versions of a udeb in Packages file' Request was from John Wright <jsw@debian.org> to control@bugs.debian.org. (Mon, 03 May 2010 08:18:05 GMT) Full text and rfc822 format available.

Added indication that 562398 affects anna Request was from John Wright <jsw@debian.org> to control@bugs.debian.org. (Mon, 03 May 2010 08:18:06 GMT) Full text and rfc822 format available.

Added tag(s) patch. Request was from John Wright <jsw@debian.org> to control@bugs.debian.org. (Mon, 03 May 2010 08:18:06 GMT) Full text and rfc822 format available.

Severity set to 'important' from 'serious' Request was from Gaudenz Steinlin <gaudenz@debian.org> to control@bugs.debian.org. (Mon, 29 Nov 2010 10:39:05 GMT) Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, Debian Install System Team <debian-boot@lists.debian.org>:
Bug#562398; Package libdebian-installer4-udeb. (Mon, 29 Nov 2010 11:33:09 GMT) Full text and rfc822 format available.

Acknowledgement sent to "Didier 'OdyX' Raboud" <didier@raboud.com>:
Extra info received and forwarded to list. Copy sent to Debian Install System Team <debian-boot@lists.debian.org>. (Mon, 29 Nov 2010 11:33:10 GMT) Full text and rfc822 format available.

Message #37 received at 562398@bugs.debian.org (full text, mbox):

From: "Didier 'OdyX' Raboud" <didier@raboud.com>
To: Gaudenz Steinlin <gaudenz@debian.org>, 562398@bugs.debian.org
Subject: Re: Bug#562398: anna: fails if multiple versions of a udeb in Packages file
Date: Mon, 29 Nov 2010 12:31:05 +0100
[Message part 1 (text/plain, inline)]
Hi Gaudenz,

Just forwarding your mail to the bugreport for completeness (it seems you forgot 
it).

Cheers, 

OdyX

----------  Gaudenz's mail  ----------

Sujet : Re: Bug#562398: anna: fails if multiple versions of a udeb in Packages 
file
Date : Monday 29 November 2010
De : Gaudenz Steinlin <gaudenz@debian.org>
À : "debian-boot" <debian-boot@lists.debian.org>

severity 562398 important
Thanks

Hi

It also seems quite risky to introduce this change now (see below for
my test results). As this bug is only triggered in very rare
circumstances and as far as I understand these circumstances can't
happen in a stable release, I don't think this issue is really release
critical. I'm therefore downgrading this bug. Feel free to upgrade it
again if you disagree.
 
Excerpts from John Wright's message of Mon Mai 03 10:15:06 +0200 2010:
> On Sun, May 02, 2010 at 03:08:08PM -0600, John Wright wrote:
> > Specifically, what happens is that anna unpacks all the packages in one
> > batch, and then it configures all of them.  But while unpacking another
> > version of a package while another vesrion is in an
> > unpacked-but-not-configured state is ok, it's not ok to configure a
> > package that's already in the configured state.  So if a package is in
> > the list twice, it fails at the second configure for that package.
> 
> Here's what's happing in libd-i:  Upon encountering a Packages stanza
> with the same Package field as one it's previously seen,
> di_packages_parser_read_name sets the data pointer the rest of the
> parsing functions will use to the previously-used di_packages pointer.
> At first glance, this would appear simply to prefer packages that appear
> later in the Packages file, irrespective of version.  Unfortunately, it
> appends the di_package to the package list
> (&parser->data->packages->list) whether it's a newly allocated one or an
> old one.  So while only one actual di_package exists for a particular
> package name, it might appear multiple times in the list.
> 
> The simple way to fix the anna issue is to make sure we only append
> newly allocated di_package objects to the list.  But it would be nice to
> favor new versions rather than whatever shows up latest in the Packages
> file (for example, to facilitate #389430 or LP#234486).
> 
> I've attached a quick reproducer to demonstrate the issue, and a patch.
> I would prefer if the version comparison could happen during the
> Packages file parsing, rather than after the fact (since this way
> requires creating a temporary hash table and traversing the list a
> couple of extra times), but that change would seem to be very invasive.
> In any case, after pounding my head for a couple of hours, I couldn't
> figure out how to do it any better with the current parsing
> infrastructure. :)

I tested you patch during the BSP in Bern. The patch still applies
cleanly to the current libdebian-installer. It also still fixes your
test case. But when testing a d-i image with your patch (patched
libdebian-installer installed on the build host and in localudebs!)
anna fails to download (some) dependencies. The install then fails
during clock-setup because tzsetup-udeb is not installed. I'm pretty
sure this is related to the patch because it does not happen with the
daily images.

John did you test your patch in d-i? And did you install the patched
libdebian-installer on your build host? If you don't install it on the
build host, the first part of the installer will run with the
unpatched library.

Gaudenz
-- 
Ever tried. Ever failed. No matter.
Try again. Fail again. Fail better.
~ Samuel Beckett ~
[signature.asc (application/pgp-signature, inline)]

Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Fri Apr 18 08:16:37 2014; Machine Name: buxtehude.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.