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

Bug#444462: still fails to insert new menu items after the current item



In #288053, Colin said:
> 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.

And the resulting patch looked like this:

+               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")

I don't understand why the < last_successful_item test was retained
when adding the seen_items test. Isn't it now unnecessary? It seems that
removing it from the test would fix this bug. (I also don't understand
why these tests are ANDed to the NEVERDEFAULT test, or why it checks for
an item _not_ existing in the seen_items hash.) 

Shouldn't the code look like this?

		if (di_hash_table_lookup(seen_items, &p->p.key)) {
			// di_log(DI_LOG_LEVEL_DEBUG, "already seen");
			continue;
		}
		if (p->installer_menu_item >= NEVERDEFAULT) {
			// di_log(DI_LOG_LEVEL_DEBUG, "not in range to be default");
			continue;
		}


On the dependency issue, I was wrong; if clock-setup is changed to
depend on localechooser, the bug should be avoided (so I'll be working
around the problem like that). OTOH, if clock-setup depends on tzsetup
which depends on localechooser, the unconfigured localechooser is not
noticed, since anna marks non-menu-item udebs as configured when
unpacking them. So tzsetup appears to main-menu to be configured and its
deps arn't checked. I kind of feel that this is a bug somewhere, and it
could be avoided by making main-menu check all the way down the
dependency tree like this:

Index: main-menu.c
===================================================================
--- main-menu.c	(revision 49361)
+++ main-menu.c	(working copy)
@@ -699,8 +699,6 @@
 	for (node = p->p.depends.head; node; node = node->next) {
 		di_package_dependency *d = node->data;
 		dep = (di_system_package *)d->ptr;
-		if (dep->p.status == di_package_status_installed)
-			continue;
 		if (d->type != di_package_dependency_type_depends)
 			continue;
 		/* Recursively configure this package */

-- 
see shy jo

Attachment: signature.asc
Description: Digital signature


Reply to: