tags 559655 patch
thanks

Hi,

I hit this issue when trying to build agains backport.org packages.
Please consider this log fragment:

    
┌──────────────────────────────────────────────────────────────────────────────┐
    │ Install build dependencies                                                
   │
    
└──────────────────────────────────────────────────────────────────────────────┘
    
    Checking for already installed source dependencies...
    debhelper: missing
    Default version of debhelper not sufficient, using version 7.2.6~bpo50+1
    flex: missing
    bison: missing
    perl: already installed (5.10.0-19lenny2)
    autotools-dev: missing
    linux-libc-dev: non-matching version installed (2.6.26-19lenny2 ! >= 2.6.27)
    Default version of linux-libc-dev not sufficient, using version 
2.6.30-8~bpo50+2
    Package installation not possible

This is bogus, each of the above dependencies are installable by apt-get
if I enter the same chroot.  The explicit offender can be found in
Sbuild/Build.pm:

    foreach my $cvers (@{$policy->{$name}->{versions}}) {
        if(version_compare($cvers, $rel, $vers)) {
            $self->log("using version $cvers\n");
            $upgradeable = $name if ! $upgradeable;
            last;
        }
    }
    $self->log("no suitable alternative found. I probably should dep-wait this 
one.\n") if !$upgradeable;
    return 0;

I think there are several problems with this code, but the killer is
that the return 0 is not conditional on !$upgradeable.  That alone
wouldn't fix the issue, though, because
 1. instead of returning it should check the next Alternative, and
 2. $upgradeable should contain the exact version found, just like
    $installable does a little bit earlier (which means #272955 could
    probably be closed).

I took the liberty to go though the filter_dependencies subroutine, and
make some other changes as well, in part to avoid looping in vain and
also to make the code a little more readable and explicit.  Please find
the patch attached, I wonder how you feel about it.  Beware, I didn't
test it too extensively.  (Actually, I'd prefer shoving the "not
installed" logic into an else branch and treat it on equal footing with
the "installed" logic, but that would've introduced too much distracting
whitespace changes.)
-- 
Regards,
Feri.

--- /usr/share/perl5/Sbuild/Build.pm	2009-08-02 13:30:48.000000000 +0200
+++ /etc/perl/Sbuild/Build.pm	2010-01-05 21:43:11.000000000 +0100
@@ -1371,6 +1371,7 @@ sub filter_dependencies {
 	my $is_satisfied = 0;
 	my $installable = "";
 	my $upgradeable = "";
+      ALTERNATIVE:
 	foreach $d ($dep, @{$dep->{'Alternatives'}}) {
 	    my ($name, $rel, $vers) =
 		($d->{'Package'}, $d->{'Rel'}, $d->{'Version'});
@@ -1387,20 +1388,19 @@ sub filter_dependencies {
 			foreach my $cvers (@{$policy->{$name}->{versions}}) {
 			    if (version_compare($cvers, $rel, $vers)) {
 				$self->log("using version $cvers\n");
-				$installable = $name . "=" . $cvers if !$installable;
-				last;
+				$installable = $name . "=" . $cvers;
+				last ALTERNATIVE;
 			    }
 			}
-			if(!$installable) {
-			    $self->log("no suitable version found. Skipping for now, maybe there are alternatives.\n");
-			    next if ($self->get_conf('CHECK_DEPENDS_ALGORITHM') eq "alternatives");
-			}
-		    } else {
+                        $self->log("no suitable version found. Skipping for now, maybe there are alternatives.\n");
+			last ALTERNATIVE unless ($self->get_conf('CHECK_DEPENDS_ALGORITHM') eq "alternatives");
+		    }
+                } else {
 			$self->log("Using default version " . $policy->{$name}->{defversion} . "\n");
+                        $installable = $name;
+                        last ALTERNATIVE;
 		    }
-		}
-		$installable = $name if !$installable;
-		next;
+		next ALTERNATIVE;
 	    }
 	    my $ivers = $stat->{'Version'};
 	    if (!$rel || version_compare( $ivers, $rel, $vers )) {
@@ -1411,7 +1411,7 @@ sub filter_dependencies {
 		    if $rel;
 		$self->log(")\n");
 		$is_satisfied = 1;
-		last;
+		last ALTERNATIVE;
 	    }
 	    debug("$name: vers dep, installed $ivers ! $rel $vers\n");
 	    $self->log("$name: non-matching version installed ".
@@ -1427,18 +1427,18 @@ sub filter_dependencies {
 		    foreach my $cvers (@{$policy->{$name}->{versions}}) {
 			if(version_compare($cvers, $rel, $vers)) {
 			    $self->log("using version $cvers\n");
-			    $upgradeable = $name if ! $upgradeable;
-			    last;
+			    $upgradeable = $name . "=" . $cvers;
+			    last ALTERNATIVE;
 			}
 		    }
-		    $self->log("no suitable alternative found. I probably should dep-wait this one.\n") if !$upgradeable;
-		    return 0;
+		    $self->log("no suitable alternative found. I probably should dep-wait this one.\n");
 		} else {
 		    $self->log("Using default version " . $policy->{$name}->{defversion} . "\n");
+                    $upgradeable = $name;
+                    last ALTERNATIVE;
 		}
-		$upgradeable = $name if !$upgradeable;
 	    }
-	}
+	} # next ALTERNATIVE
 	if (!$is_satisfied) {
 	    if ($upgradeable) {
 		debug("using $upgradeable for upgrade\n");

Reply via email to