Better, more global way of dealing with forward dependencies, and to

ask just once per-set for exec/unexec.

Todo: forward-deps could be merged, but this might lead to dependency
loops, we have to detect them and act first

Todo: rewrite the other part of forward dependency handling (dep adjustment)

4 files changed, 180 insertions, 122 deletions

diff --git a/usr.sbin/pkg_add/Makefile b/usr.sbin/pkg_add/Makefile
index c5fe685b2ba..37f6847984a 100644
--- a/usr.sbin/pkg_add/Makefile
+++ b/usr.sbin/pkg_add/Makefile
@@ -1,4 +1,4 @@
-# $OpenBSD: Makefile,v 1.55 2009/11/16 12:20:32 espie Exp $
+# $OpenBSD: Makefile,v 1.56 2009/12/26 17:00:49 espie Exp $
 
 .include <bsd.own.mk>
 
@@ -18,6 +18,7 @@ PACKAGES= \
     OpenBSD/Delete.pm \
     OpenBSD/Dependencies.pm \
     OpenBSD/Error.pm \
+    OpenBSD/ForwardDependencies.pm \
     OpenBSD/Getopt.pm \
     OpenBSD/Handle.pm \
     OpenBSD/IdCache.pm \
diff --git a/usr.sbin/pkg_add/OpenBSD/ForwardDependencies.pm b/usr.sbin/pkg_add/OpenBSD/ForwardDependencies.pm
new file mode 100644
index 00000000000..56e950a9e28
--- /dev/null
+++ b/usr.sbin/pkg_add/OpenBSD/ForwardDependencies.pm
@@ -0,0 +1,95 @@
+# ex:ts=8 sw=4:
+# $OpenBSD: ForwardDependencies.pm,v 1.1 2009/12/26 17:00:49 espie Exp $
+#
+# Copyright (c) 2009 Marc Espie <espie@openbsd.org>
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+
+# handling of forward dependency adjustments
+
+use strict;
+use warnings;
+
+package OpenBSD::ForwardDependencies;
+
+require OpenBSD::RequiredBy;
+
+sub find
+{
+	my ($class, $set) = @_;
+	my $forward = {};
+	for my $old ($set->older) {
+		for my $f (OpenBSD::RequiredBy->new($old->pkgname)->list) {
+			next if defined $set->{older}->{$f};
+			next if defined $set->{kept}->{$f};
+			$forward->{$f} = 1;
+		}
+	}
+	bless $forward, $class;
+}
+
+sub check
+{
+	my ($forward, $set, $state) = @_;
+
+	my @r = keys %$forward;
+
+	return if @r == 0;
+	$state->say("Verifying dependencies still match for ", 
+	    join(', ', @r)) if $state->verbose >= 2;
+
+	my @new = ($set->newer_names, $set->kept_names);
+	my @old = $set->older_names;
+	my $result = {};
+
+	for my $f (@r) {
+		my $p2 = OpenBSD::PackingList->from_installation(
+		    $f, \&OpenBSD::PackingList::DependOnly);
+		if (!defined $p2) {
+			$state->errsay("Error: $f missing from installation");
+		} else {
+			$p2->check_forward_dependency($f, \@old, \@new, $result);
+		}
+	}
+	if (keys %$result > 0) {
+		$state->say($set->print, " forward dependencies:");
+		while (my ($pkg, $l) = each %$result) {
+			my $deps = join(',', map {$_->{pattern}} @$l);
+			if (@$l == 1) {
+				$state->say("| Dependency of $pkg on $deps doesn't match");
+			} else {
+				$state->say("| Dependencies of $pkg on $deps don't match");
+			}
+		}
+	}
+	return $result;
+}
+
+package OpenBSD::PackingElement;
+sub check_forward_dependency
+{
+}
+
+package OpenBSD::PackingElement::Dependency;
+sub check_forward_dependency
+{
+	my ($self, $f, $old, $new, $r) = @_;
+
+	# nothing to validate if old dependency doesn't concern us.
+	return unless $self->spec->filter(@$old);
+	# nothing to do if new dependency just matches
+	return if $self->spec->filter(@$new);
+
+	push(@{$r->{$f}}, $self);
+}
+
+1;
diff --git a/usr.sbin/pkg_add/OpenBSD/Replace.pm b/usr.sbin/pkg_add/OpenBSD/Replace.pm
index 7783ec4fc74..f62b21f7f27 100644
--- a/usr.sbin/pkg_add/OpenBSD/Replace.pm
+++ b/usr.sbin/pkg_add/OpenBSD/Replace.pm
@@ -1,5 +1,5 @@
 # ex:ts=8 sw=4:
-# $OpenBSD: Replace.pm,v 1.62 2009/12/20 22:38:45 espie Exp $
+# $OpenBSD: Replace.pm,v 1.63 2009/12/26 17:00:49 espie Exp $
 #
 # Copyright (c) 2004-2006 Marc Espie <espie@openbsd.org>
 #
@@ -27,15 +27,10 @@ sub can_update
 	my $issue = $self->update_issue($installing);
 	
 	if (defined $issue) {
-		$state->{okay} = 0;
 	    	push(@{$state->{journal}}, $issue);
 	}
 }
 
-sub validate_depend
-{
-}
-
 sub update_issue { undef }
 
 sub extract
@@ -223,33 +218,6 @@ sub separate_element
 package OpenBSD::PackingElement::Dependency;
 use OpenBSD::Error;
 
-sub validate_depend
-{
-	my ($self, $state, $wanting, $toreplace, @replacement) = @_;
-
-	# nothing to validate if old dependency doesn't concern us.
-	return unless $self->spec->filter($toreplace);
-	# nothing to do if new dependency just matches
-	return if $self->spec->filter(@replacement);
-
-	if ($state->{defines}->{updatedepends}) {
-	    $state->errsay("Forward dependency of $wanting on $toreplace doesn't match @replacement, forcing it");
-	    $state->{forcedupdates} = {} unless defined $state->{forcedupdates};
-	    $state->{forcedupdates}->{$wanting} = 1;
-	} elsif ($state->{interactive}) {
-
-	    if ($state->confirm("Forward dependency of $wanting on $toreplace doesn't match @replacement, proceed with update anyways", 0)) {
-		$state->{forcedupdates} = {} unless defined $state->{forcedupdates};
-		$state->{forcedupdates}->{$wanting} = 1;
-	    } else {
-		$state->{okay} = 0;
-	    }
-	} else {
-	    $state->{okay} = 0;
-	    $state->errsay("Can't update forward dependency of $wanting on $toreplace: @replacement doesn't match (use -F updatedepends to force it)");
-	}
-}
-
 package OpenBSD::PackingElement::Lib;
 sub mark_lib
 {
@@ -306,76 +274,66 @@ sub perform_extraction
 	$handle->{plist}->extract_and_progress($state, \$donesize, $totsize);
 }
 
-sub can_old_package_be_replaced
+sub check_plist_exec
 {
-	my ($old_plist, $set, $state) = @_;
+	my ($plist, $state, $new) = @_;
 
-	$state->{okay} = 1;
 	$state->{journal} = [];
-	$old_plist->can_update(0, $state);
-	if ($state->{okay} == 0) {
-		$state->errsay("Old package ", $old_plist->pkgname, 
-		    " contains potentially unsafe operations");
-		for my $i (@{$state->{journal}}) {
-			$state->errsay("\t", $i);
-		}
-		if ($state->{defines}->{update}) {
-			$state->errsay("(forcing update)");
-			$state->{okay} = 1;
-		} elsif ($state->{interactive}) {
-
-			if ($state->confirm("proceed with update anyways", 0)) {
-			    $state->{okay} = 1;
-			}
-		}
-	}
-	my @wantlist = OpenBSD::RequiredBy->new($old_plist->pkgname)->list;
-	my @r = ();
-	for my $wanting (@wantlist) {
-		push(@r, $wanting) if !defined $set->{older}->{$wanting};
-	}
-	if (@r) {
-		$state->say("Verifying dependencies still match for ", 
-		    join(', ', @r)) if $state->verbose >= 2;
-		for my $wanting (@wantlist) {
-			my $p2 = OpenBSD::PackingList->from_installation(
-			    $wanting, \&OpenBSD::PackingList::DependOnly);
-			if (!defined $p2) {
-				$state->errsay("Error: $wanting missing from installation");
-			} else {
-				$p2->validate_depend($state, $wanting, 
-				    $old_plist->pkgname, $set->newer_names, 
-				    $set->kept_names);
-			}
-		}
+	$plist->can_update($new, $state);
+	return 1 if @{$state->{journal}} == 0;
+
+	$state->errsay($new ? "New": "Old", " package ", $plist->pkgname, 
+	    " contains potentially unsafe operations");
+	for my $i (@{$state->{journal}}) {
+		$state->errsay("| ", $i);
 	}
-	return $state->{okay};
+	return 0;
+}
+
+sub can_old_package_be_replaced
+{
+	my ($plist, $state) = @_;
+	return check_plist_exec($plist, $state, 0);
 }
 
 sub is_new_package_safe
 {
 	my ($plist, $state) = @_;
-	$state->{okay} = 1;
-	$state->{journal} = [];
-	$plist->can_update(1, $state);
-	if ($state->{okay} == 0) {
-		$state->errsay("New package ", $plist->pkgname, 
-		    " contains potentially unsafe operations");
-		for my $i (@{$state->{journal}}) {
-			$state->errsay("\t", $i);
-		}
-		if ($state->{defines}->{update}) {
-			$state->errsay("(forcing update)");
-			$state->{okay} = 1;
-		} elsif ($state->{interactive}) {
-			if ($state->confirm("proceed with update anyways", 0)) {
-			    $state->{okay} = 1;
-			}
-		}
+	return check_plist_exec($plist, $state, 1);
+}
+
+sub is_set_safe
+{
+	my ($set, $state) = @_;
+
+	my $ok = 1;
+
+	for my $pkg ($set->older) {
+		$ok = 0 unless can_old_package_be_replaced($pkg->plist, $state);
 	}
-	return $state->{okay};
+	for my $pkg ($set->newer) {
+		$ok = 0 unless is_new_package_safe($pkg->plist, $state);
+	}
+	return 1 if $ok;
+
+	if ($state->{defines}->{update}) {
+		$state->errsay("Forcing update");
+		return 1;
+	} elsif ($state->{interactive}) {
+
+		if ($state->confirm("proceed with update anyways", 0)) {
+			return 1;
+		} else {
+			return 0;
+		}
+	} else {
+		$state->errsay("Cannot install ", $set->print, 
+		    " (use -F update)");
+		return 0;
+    	}
 }
 
+
 sub split_some_libs
 {
 	my ($plist, $libs) = @_;
diff --git a/usr.sbin/pkg_add/pkg_add b/usr.sbin/pkg_add/pkg_add
index e1bba3b1be1..68b1557dc43 100644
--- a/usr.sbin/pkg_add/pkg_add
+++ b/usr.sbin/pkg_add/pkg_add
@@ -1,7 +1,7 @@
 #! /usr/bin/perl
 
 # ex:ts=8 sw=4:
-# $OpenBSD: pkg_add,v 1.434 2009/12/21 17:15:46 espie Exp $
+# $OpenBSD: pkg_add,v 1.435 2009/12/26 17:00:49 espie Exp $
 #
 # Copyright (c) 2003-2009 Marc Espie <espie@openbsd.org>
 #
@@ -202,25 +202,6 @@ sub find_conflicts
 	return keys %conflicts;
 }
 
-sub is_new_stuff_safe
-{
-	my ($set, $state) = @_;
-
-	my $r = 1;
-
-	for my $handle ($set->newer) {
-		require OpenBSD::Replace;
-
-		my $plist = $handle->plist;
-		if (!OpenBSD::Replace::is_new_package_safe($plist, $state)) {
-			$state->say("Can't safely update to ", 
-			    $handle->pkgname," (use -F update to force it)");
-			$r = 0;
-		}
-	}
-	return $r;
-}
-
 sub mark_as_manual_install
 {
 	my $set = shift;
@@ -266,12 +247,37 @@ sub can_install
 		}
 	}
 
-	return if $later;
 
-	if (!$set->is_new_stuff_safe($state)) {
-		return 0;
+	require OpenBSD::ForwardDependencies;
+	my $forward = OpenBSD::ForwardDependencies->find($set);
+	my $bad = $forward->check($set, $state);
+
+	if (keys %$bad > 0) {
+		if ($state->{defines}->{updatedepends}) {
+		    $state->errsay("Forcing update");
+		} elsif ($state->{interactive}) {
+			if ($state->confirm("Proceed with update anyways", 0)) {
+			}
+		} else {
+			return 0;
+		}
+#		XXX can't do this yet, have to protect updates against loops
+#		$later = 1;
+#		for my $m (keys %$bad) {
+#			my $s = $state->tracker->is_to_update($m);
+#			if (defined $s) {
+#				$set->merge($state->tracker, $s);
+#			} else {
+#				$set->add_older(OpenBSD::Handle->create_old($m, 
+#				    $state));
+#			}
+#		}
 	}
 
+	return if $later;
+
+	$set->{forward} = $forward;
+
 	my $manual_install = 0;
 
 	for my $old ($set->older) {
@@ -284,19 +290,17 @@ sub can_install
 			Fatal "Couldn't find packing-list for $name\n";
 		}
 
-		if (!OpenBSD::Replace::can_old_package_be_replaced($old->plist, 
-		    $set, $state)) {
-			$state->errsay("Can't update $name into ", 
-			    $set->print);
-			return 0;
-		}
-
 		if ($old->plist->has('manual-installation')) {
 			$manual_install = 1;
 		}
 	}
 
 	$set->mark_as_manual_install if $manual_install;
+
+	require OpenBSD::Replace;
+
+	return 0 unless OpenBSD::Replace::is_set_safe($set, $state);
+
 	return 1;
 }