Re: pkg_rolling-replace infinite loop?

[Ian D. Leroux <idleroux%fastmail.fm@localhost>]

At Sun, 22 Aug 2010 10:01:25 -0400,
Greg Troxel wrote:
> 
> [1  <text/plain (quoted-printable)>]
> 
> "Ian D. Leroux" <idleroux%fastmail.fm@localhost> writes:
> 
> > On Wed, 11 Aug 2010 23:54 -0400, "Ian D. Leroux" 
> > <idleroux%fastmail.fm@localhost>
> > wrote:
> > OK, I think I roughly understand what is going on.  pkg_rr builds a
> > dependency list, tsorts it, and then iterates through it looking for the
> > first package that needs to be rebuilt.  Lines 416--420:
> >
> >     for pkg in $TSORTED; do
> >         if is_member $pkg $REPLACE_TODO; then
> >             break;
> >         fi
> >     done
> >
> > If, for some reason, the packages in $REPLACE_TODO don't appear in
> > $TSORTED, the loop terminates normally (i.e., without ever breaking),
> > and the last $pkg in $TSORTED gets rebuilt.  In my case, the packages in
> > $REPLACE_TODO were py26-gnupg and py26-setuptools, while $TSORTED
> > contained py25-gnupg and py25-setuptools, so the last package in my
> > $TSORTED list (wl-snapshot) just got repeatedly reinstalled; pkg_rr
> > never finished, because the packages that actually needed rebuilding
> > never got rebuilt.  I manually installed the py26-versions and now
> > pkg_rolling-replace finishes properly.
> >
> > I can pretty easily patch the loop so that it recognizes when it
> > finished without breaking and exits with an error, but that's a bandaid.
> > The real fix would be to guarantee that $REPLACE_TODO really is a
> > subset of $TSORTED.
> 
> I think you are right.
> 
> While i see you your point about the correct fix is to preserve the
> invariant that $REPLACE_TODO is a subset of $TSORTED, I think it's more
> important that there not be infinite loops.  So I would definitely
> appreciate a tested patch to error out if this case happens, which I
> think is just a fatal error between the 'fi' and 'done' above.  Or is it
> harder than that?

That would just stop the loop on the first iteration; you want to let
it run until it finds a package to replace, and error out only if it
fails to do so. It's going to look something like (untested
pseudocode):

pkg_selected = 0
for pkg in $TSORTED; do
   if is_member $pkg $REPLACE_TODO; then
        pkg_selected = 1
        break;
    fi
done
if [ $pkg_selected -eq 0 ]; then
  echo "The following packages to be upgraded do not appear"
  echo " in the list of installed packages:"
  echo $REPLACE_TODO
  echo "Please install these packages manually."
fi

My pkgsrc installation is busy rebuilding itself at the moment; once
it finishes I can have a look at testing that and turning it into a
proper patch.  Comments and suggestions for improvements to style or
substance are, or course, welcome.

Ian Leroux