Bug#151662: #151662: dselect: changes selections without notice
tags 151662 patch
thanks
On Fri, Oct 03, 2003 at 09:52:18PM +0100, Colin Watson wrote:
> On Fri, Apr 25, 2003 at 12:16:16AM +0200, Yann Dirson wrote:
> > Please consider the following sequnce of events:
> >
> > - in the package listing, I hit ENTER to validate my changes
> >
> > - because of unsatisfied recommends I don't want to honor, AND of
> > other legitimate relashionships, I get a resolution dialog
> >
> > - I hit R to be sure my wishes wrt Recommends are honored, and review
> > one by one the other packages
> >
> > - if I hit ENTER, the Recommended packages are selected against my
> > will.
>
> I've just stumbled across a reproduction case for what looks like the
> same bug: I put a package on hold, go through conflict resolution and
> put all mentioned packages on hold as well (which should be fine, it's
> the installed state and something I know to be a consistent state), and
> when I exit conflict resolution some of the packages have silently been
> shunted back to desired-state installed.
>
> I've grabbed a tarball of /var/lib/dpkg and will make it available, but
> since I've got this and a dpkg source tree in front of me I'm going to
> have a shot at debugging it now. More details should follow soon ...
Gotcha!
http://www.chiark.greenend.org.uk/~cjwatson/tmp/dselect-151662.tar.gz is
a reproduction case.
* Run 'dselect --admindir unpacked/var/lib/dpkg' or whatever.
* Move the cursor down a couple of lines to perl-base.
* Hit '=' to put perl-base on hold.
* The dependency/conflict resolution screen will pop up. OK so far.
* Put the other two packages (perl and libperl5.8) on hold as well.
* Press enter to exit dependency/conflict resolution.
* Note that perl-base is now marked to install again! Furthermore,
perl is marked to hold, but run the cursor down over it and it
changes to install. Freaky.
So, I dug into this a bit with gdb, and discovered that perl gets set
back to install because perl-doc depends on it and is still marked to
install, which is fair enough. But in that case dselect is supposed to
(and always used to) pop up another dependency/conflict resolution
screen, not just ignore it and drop back to the main screen.
The mechanism used to decide whether to pop up another resolution screen
is in packagelist::resolvesuggest()'s return code. The head comment kind
of explains it: return 0 if there's nothing new to suggest, 1 if the
most significant thing it found was a new Suggests, and 2 if there are
new Recommends, Depends, or Conflicts to consider. The return code of
packagelist::pkgdepcon() is supposed to follow the same scheme.
Up until Joey's patch to fix Recommends was applied, pkgdepcon() did
indeed return 2 if it found a new Depends. However, the new Recommends
handling required that it return 0 if the Recommends had been seen
before. The Recommends patch therefore changed 'return 2' to 'return r',
as follows:
diff -p -u -r1.11 -r1.12
--- pkgdepcon.cc 5 Nov 2000 15:06:48 -0000 1.11
+++ pkgdepcon.cc 13 Jul 2001 23:49:46 -0000 1.12
@@ -316,9 +285,13 @@ int packagelist::resolvedepcon(dependenc
fprintf(debug,"packagelist[%p]::resolvedepcon([%p]): select best=%s{%d}\n",
this,depends, best->pkg->name, best->spriority);
if (best->spriority >= sp_selecting) return r;
- best->selected= best->suggested= pkginfo::want_install;
- best->spriority= sp_selecting;
- return 2;
+ /* Always select depends. Only select recommends if we got here because
+ * of a manually-initiated install request. */
+ if (depends->type != dep_recommends || manual_install) {
+ best->selected= best->suggested= pkginfo::want_install;
+ best->spriority= sp_selecting;
+ }
+ return r;
mustdeselect:
best= depends->up->clientdata;
@@ -327,11 +300,14 @@ int packagelist::resolvedepcon(dependenc
this,depends, best->pkg->name, best->spriority);
if (best->spriority >= sp_deselecting) return r;
- best->selected= best->suggested=
- best->pkg->status == pkginfo::stat_notinstalled
- ? pkginfo::want_purge : pkginfo::want_deinstall; /* fixme: configurable */
- best->spriority= sp_deselecting;
- return 2;
+ /* Always remove depends, but never remove recommends. */
+ if (depends->type != dep_recommends) {
+ best->selected= best->suggested=
+ best->pkg->status == pkginfo::stat_notinstalled
+ ? pkginfo::want_purge : pkginfo::want_deinstall; /* fixme: configurable */
+ best->spriority= sp_deselecting;
+ }
+ return r;
case dep_conflicts:
Unfortunately, r here is the return value of int packagelist::add(),
which is only ever 0 if the dependency has already been seen or 1
otherwise. Thus, as of dselect 1.10, dependency/conflict resolution
screens are only ever shown when there's a new Conflicts (due to
deselect_one_of()), and never because of a Depends or Recommends that
the user hasn't been told about yet. This is bad.
I think the following patch is the right fix. I'm cc'ing Joey on this
because I'd like him to confirm that it makes sense with the original
intent of his change. I've tested this on a few packages with
Recommends, as well as obviously the perl-base test case above, and it
seems to behave properly.
diff -ru dpkg-1.10.15.orig/dselect/pkgdepcon.cc dpkg-1.10.15/dselect/pkgdepcon.cc
--- dpkg-1.10.15.orig/dselect/pkgdepcon.cc 2002-05-06 17:18:15.000000000 +0100
+++ dpkg-1.10.15/dselect/pkgdepcon.cc 2003-10-03 23:24:43.000000000 +0100
@@ -290,7 +290,7 @@
best->selected= best->suggested= pkginfo::want_install;
best->spriority= sp_selecting;
}
- return r;
+ return r ? 2 : 0;
mustdeselect:
best= depends->up->clientdata;
@@ -306,7 +306,7 @@
? pkginfo::want_purge : pkginfo::want_deinstall; /* fixme: configurable */
best->spriority= sp_deselecting;
}
- return r;
+ return r ? 2 : 0;
case dep_conflicts:
This bug explains an awful lot of low-level weirdness that I've been
seeing in dselect resolution screens for a while now but never got to
the point of tracking down, and it's a regression from dselect in woody.
I suspect this might also explain #188667. Could this patch please be
applied in time for sarge?
Thanks,
--
Colin Watson [cjwatson@flatline.org.uk]
Reply to: