update-alternatives: skip slave link if associated file doesn't exist

All slave links are optional in the sense that they are only installed
if the currently associated file does exist. This allows most
update-alternatives call to succeed even if the admin removed
documentation files or other optional files. Closes: #143701
A message is still displayed by default in this case to warn the
user that something uncommon has been detected.

Adjusted the test suite to verify this behaviour.

ChangeLog

@@ -1,3 +1,12 @@
+2009-02-22  Raphael Hertzog  <hertzog@debian.org>
+
+	* scripts/update-alternatives.pl: Installation of a slave
+	alternative link is only done if the corresponding slave file is
+	available.
+	* man/update-alternatives.8: Document this behaviour.
+	* scripts/t/900_update_alternatives.t: Add corresponding tests in
+	the test suite.
+
 2009-02-22  Raphael Hertzog  <hertzog@debian.org>
 
 	* scripts/update-alternatives.pl: Add more sanity check for

debian/changelog

@@ -161,6 +161,8 @@ dpkg (1.15.0) UNRELEASED; urgency=low
       Closes: #342566
     - it forbids reusing alternative links managed by other alternatives
     - new sanity checks on --install parameters. Closes: #423176
+    - install slave link only if the corresponding slave file is available.
+      Closes: #143701
 
   [ Pierre Habouzit ]
   * Add a --query option to update-alternatives. Closes: #336091, #441904

man/update-alternatives.8

@@ -215,7 +215,10 @@ alternatives directory and the alternative path for a slave link.
 Zero or more
 .B \-\-slave
 options, each followed by three arguments,
-may be specified.
+may be specified. Note that the master alternative must exist or the call
+will fail. However if a slave alternative doesn't exist, the corresponding
+slave alternative link will simply not be installed (a warning will still
+be displayed).
 .IP
 If the alternative name specified exists already
 in the alternatives system's records,

scripts/t/900_update_alternatives.t

@@ -52,8 +52,8 @@ my @choices = (
     },
 );
 my $nb_slaves = 2;
-plan tests => (4 * ($nb_slaves + 1) + 2) * 18 # number of check_choices
-		+ 49;			      # rest
+plan tests => (4 * ($nb_slaves + 1) + 2) * 21 # number of check_choices
+		+ 54;			      # rest
 
 sub cleanup {
     system("rm -rf $srcdir/t.tmp/ua && mkdir -p $admindir && mkdir -p $altdir");
@@ -123,12 +123,14 @@ sub get_slaves_status {
 	    $slaves{$alt->{slaves}[$i]{name}}{"installed"} = 0;
 	}
     }
-    # except those of the current alternative
+    # except those of the current alternative (minus optional slaves)
     if (defined($id)) {
 	my $alt = $choices[$id];
 	for(my $i = 0; $i < @{$alt->{slaves}}; $i++) {
 	    $slaves{$alt->{slaves}[$i]{name}} = $alt->{slaves}[$i];
-	    $slaves{$alt->{slaves}[$i]{name}}{"installed"} = 1;
+	    if (-e $alt->{slaves}[$i]{path}) {
+		$slaves{$alt->{slaves}[$i]{name}}{"installed"} = 1;
+	    }
 	}
     }
     return sort { $a->{name} cmp $b->{name} } values %slaves;
@@ -296,6 +298,33 @@ call_ua(["--install", "$bindir/testmaster", "test/master", "/bin/date", "10"],
 call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10",
 	 "--slave", "$bindir/testslave", "test slave", "/bin/true" ],
 	expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null");
+# install in non-existing dir should fail
+call_ua(["--install", "$bindir/doesntexist/testmaster", "testmaster", "/bin/date", "10",
+	 "--slave", "$bindir/testslave", "testslave", "/bin/true" ],
+	expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null");
+call_ua(["--install", "$bindir/testmaster", "testmaster", "/bin/date", "10",
+	 "--slave", "$bindir/doesntexist/testslave", "testslave", "/bin/true" ],
+	expect_failure => 1, to_file => "/dev/null", error_to_file => "/dev/null");
+
+# non-existing alternative path in slave is not a failure
+my $old_path = $choices[0]{"slaves"}[0]{"path"};
+$old_slave = $choices[0]{"slaves"}[0]{"link"};
+$choices[0]{"slaves"}[0]{"path"} = "$bindir/doesntexist";
+$choices[0]{"slaves"}[0]{"link"} = "$bindir/baddir/slave2";
+# test rename of slave link that existed but that doesnt anymore
+# and link is moved into non-existing dir at the same time
+install_choice(0);
+check_choice(0, "auto", "optional renamed slave2 in non-existing dir");
+# same but on fresh install
+cleanup();
+install_choice(0);
+check_choice(0, "auto", "optional slave2 in non-existing dir");
+$choices[0]{"slaves"}[0]{"link"} = $old_link;
+# test fresh install with a non-existing slave file
+cleanup();
+install_choice(0);
+check_choice(0, "auto", "optional slave2");
+$choices[0]{"slaves"}[0]{"path"} = $old_path;
+
 
-# TODO: install in non-existing dir, handle of pre-existing files in place
-# of alternative links
+# TODO: handle of pre-existing files in place of alternative links

scripts/update-alternatives.pl

@@ -266,8 +266,7 @@ if ($action eq 'set') {
         if ($old ne $new and -l $old) {
             pr(_g("Renaming %s link from %s to %s."), $inst_alt->name(),
                $old, $new) if $verbosemode >= 0;
-            rename_mv($old, $new, 1) ||
-                quit(_g("unable to rename %s to %s: see error above"), $old, $new);
+            checked_mv($old, $new);
         }
         # Check if new slaves have been added, or existing ones renamed
         foreach my $slave ($inst_alt->slaves()) {
@@ -278,11 +277,17 @@ if ($action eq 'set') {
             }
             $old = $alternative->slave_link($slave);
             $alternative->add_slave($slave, $new);
+            my $new_file = ($current_choice eq $fileset->master()) ?
+                            $fileset->slave($slave) :
+                            readlink("$admdir/$slave") || "";
             if ($old ne $new and -l $old) {
-                pr(_g("Renaming %s slave link from %s to %s."), $slave,
-                   $old, $new) if $verbosemode >= 0;
-                rename_mv($old, $new, 1) ||
-                    quit(_g("unable to rename %s to %s: see error above"), $old, $new);
+                if (-e $new_file) {
+                    pr(_g("Renaming %s slave link from %s to %s."), $slave,
+                       $old, $new) if $verbosemode >= 0;
+                    checked_mv($old, $new);
+                } else {
+                    checked_rm($old);
+                }
             }
         }
     } else {
@@ -451,12 +456,11 @@ sub pr {
 }
 
 sub rename_mv {
-    my ($source, $dest, $ignore_enoent) = @_;
-    $ignore_enoent = 0 unless defined($ignore_enoent);
-    return 1 if ($ignore_enoent and not -e $source);
+    my ($source, $dest) = @_;
+    lstat($source);
+    return 0 if not -e _;
     if (not rename($source, $dest)) {
         if (system("mv", $source, $dest) != 0) {
-            return $ignore_enoent if $! == ENOENT;
             return 0;
         }
     }
@@ -915,7 +919,7 @@ sub prepare_install {
     # Take care of slaves links
     foreach my $slave ($self->slaves()) {
         my ($slink, $spath) = ($self->slave_link($slave), $fileset->slave($slave));
-        if ($fileset->has_slave($slave)) {
+        if ($fileset->has_slave($slave) and -e $spath) {
             # Setup slave link
             main::checked_rm("$slink.dpkg-tmp");
             main::checked_symlink("$altdir/$slave", "$slink.dpkg-tmp");
@@ -928,6 +932,10 @@ sub prepare_install {
                 main::checked_mv("$altdir/$slave.dpkg-tmp", "$altdir/$slave");
             });
         } else {
+            main::pr(_g("skip creation of %s because associated file %s (of" .
+                        "link group %s) doesn't exist"), $slink, $spath,
+                     $self->name())
+                if $verbosemode >= 0 and $fileset->has_slave($slave);
             # Drop unused slave
             $self->add_commit_op(sub {
                 main::checked_rm($slink);
@@ -977,7 +985,7 @@ sub is_broken {
     my $fileset = $self->fileset($self->current());
     foreach my $slave ($self->slaves()) {
         $file = readlink($self->slave_link($slave));
-        if ($fileset->has_slave($slave)) {
+        if ($fileset->has_slave($slave) and -e $fileset->slave($slave)) {
             return 1 if not defined($file);
             return 1 if $file ne "$altdir/$slave";
             $file = readlink("$altdir/$slave");