[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#288053: network-console - Regression: is not correctly added in menu



On Mon, Jan 03, 2005 at 12:07:05AM +0100, Frans Pop wrote:
> The original issue appears to be not a problem in network-console, but in 
> main-menu.
> Currently the order of menu items appears to be based on:
> - number in installer-menu-item
> - dependencies
> 
> Maybe a third factor should be taken into account: "not before the menu 
> item that was responsible for installing the component".
> These could be load-floppy, load-cdrom, download-installer and maybe 
> others.

This bug was introduced due to the fix for #224633. Perhaps the existing
fix for that is too crude. The basic goal there is that if a menu item
fails but a later one has succeeded, main-menu shouldn't keep on setting
the earlier failing one as the default. At the moment, main-menu does
this by remembering the last successful item and never defaulting to an
earlier one.

I don't really want to hardcode knowledge of the retriever items in
main-menu, though. I think this could equally be achieved by remembering
all failed items and never defaulting to any of them. The chief visible
difference from the current behaviour would be that if you drop to
expert mode and skip over an item by hand, you'd have to keep skipping
over it. Alternatively, we could make main-menu's defaulting behave
differently in expert mode, but that would mean that we wouldn't get
this fix for network-console in expert mode and it may not be worth it.
I suppose this wouldn't be an entirely trivial change.

One even more conservative possibility would be to remember what menu
items we've seen at all, and avoid suppressing any new items that appear
just because they're earlier in the menu order than the last successful
item. Here's an untested patch:

Index: main-menu.c
===================================================================
--- main-menu.c	(revision 43061)
+++ main-menu.c	(working copy)
@@ -31,6 +31,7 @@
 const int RAISE = 1;
 const int LOWER = 0;
 
+di_hash_table *seen_items;
 int last_successful_item = -1;
 
 /* Save default priority, to be able to return to it when we have to lower it */
@@ -61,6 +62,13 @@
 	return strcmp(p1->p.package, p2->p.package);
 }
 
+static void seen_items_key_destroy (void *key)
+{
+	di_rstring *s = key;
+	di_free(s->string);
+	di_free(s);
+}
+
 int isdefault(di_system_package *p) {
 	int check;
 
@@ -128,7 +136,8 @@
 			//di_log(DI_LOG_LEVEL_DEBUG, "not menu item; or not installed");
 			continue;
 		}
-		if (p->installer_menu_item < last_successful_item &&
+		if ((p->installer_menu_item < last_successful_item &&
+		     !di_hash_table_lookup(seen_items, &p->p.key)) &&
 		    p->installer_menu_item < NEVERDEFAULT) {
 			//di_log(DI_LOG_LEVEL_DEBUG, "not in range to be default");
 			continue;
@@ -582,9 +591,14 @@
 
 	menu_startup();
 
+	seen_items = di_hash_table_new_full(di_rstring_hash, di_rstring_equal,
+					    seen_items_key_destroy, NULL);
+
 	allocator = di_system_packages_allocator_alloc ();
 	packages = di_system_packages_status_read_file(DI_SYSTEM_DPKG_STATUSFILE, allocator);
 	while ((p=show_main_menu(packages, allocator))) {
+		di_slist_node *node;
+
 		ret = do_menu_item(p);
 		adjust_default_priority();
 		switch (ret) {
@@ -608,7 +622,17 @@
 				notify_user_of_failure(p);
 				modify_debconf_priority(LOWER);
 		}
-		
+
+		/* Remember all the packages we've seen so far */
+		for (node = packages->list.head; node; node = node->next) {
+			di_system_package *seen = node->data;
+			di_rstring *seen_name = di_new0(di_rstring, 1);
+			seen_name->string = di_stradup(seen->p.key.string,
+						       seen->p.key.size);
+			seen_name->size = seen->p.key.size;
+			di_hash_table_insert(seen_items, seen_name, seen_name);
+		}
+
 		di_packages_free (packages);
 		di_packages_allocator_free (allocator);
 		allocator = di_system_packages_allocator_alloc ();

Cheers,

-- 
Colin Watson                                       [cjwatson@debian.org]



Reply to: