The following commit has been merged in the master branch:
commit 665afa1af569fe4500d1bef39c62af4fe714043c
Author: Raphael Hertzog <hert...@debian.org>
Date:   Sun Feb 22 14:49:06 2009 +0100

    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.

diff --git a/ChangeLog b/ChangeLog
index d9b42de..c042ac8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2009-02-22  Raphael Hertzog  <hert...@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  <hert...@debian.org>
+
        * scripts/update-alternatives.pl: Add more sanity check for
        --install: ensure alternative path and links are absolute, ensure
        master alternative path exists, forbid / and spaces in alternative
diff --git a/debian/changelog b/debian/changelog
index ce32c5c..9d19387 100644
--- a/debian/changelog
+++ b/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
diff --git a/man/update-alternatives.8 b/man/update-alternatives.8
index daa8670..122168c 100644
--- a/man/update-alternatives.8
+++ b/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,
diff --git a/scripts/t/900_update_alternatives.t 
b/scripts/t/900_update_alternatives.t
index 997d663..00ca86f 100644
--- a/scripts/t/900_update_alternatives.t
+++ b/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
diff --git a/scripts/update-alternatives.pl b/scripts/update-alternatives.pl
index cdf6675..6f79d70 100755
--- a/scripts/update-alternatives.pl
+++ b/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");

-- 
dpkg's main repository


-- 
To UNSUBSCRIBE, email to debian-dpkg-cvs-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to