Просмотр исходного кода

update-alternatives: don't replace real files by alternative links without --force

update-alternatives should not replace real files by symlinks. When
--install is called, the alternative link should either not exist
or already be a link. It that's not the case, it will simply skip
installing that link and display a warning. However if --force is given,
it will (try to) replace the file.
Ensure that behaviour within the test suite.
Raphael Hertzog лет назад: 17
Родитель
Сommit
a7cc8e2eb3
4 измененных файлов с 65 добавлено и 21 удалено
  1. 12 0
      ChangeLog
  2. 6 1
      man/update-alternatives.8
  3. 17 7
      scripts/t/900_update_alternatives.t
  4. 30 13
      scripts/update-alternatives.pl

+ 12 - 0
ChangeLog

@@ -1,3 +1,15 @@
+2009-02-22  Raphael Hertzog  <hertzog@debian.org>
+
+	* scripts/update-alternatives.pl: update-alternatives should not
+	replace real files by symlinks. When --install is called, the
+	alternative link should either not exist or already be a link. It
+	that's not the case, it will simply skip installing that link and
+	display a warning. However if --force is given, it will (try to)
+	replace the file.
+	* 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>
 
 	* man/update-alternatives.8: Document how --all can be used to

+ 6 - 1
man/update-alternatives.8

@@ -218,7 +218,8 @@ options, each followed by three arguments,
 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).
+be displayed). If some real file is installed where an alternative link
+has to be installed, it is kept unless \fB\-\-force\fR is used.
 .IP
 If the alternative name specified exists already
 in the alternatives system's records,
@@ -330,6 +331,10 @@ different from the default.
 Specifies the administrative directory, when this is to be
 different from the default.
 .TP
+.TP
+.BI \-\-force
+Let \fBupdate-alternatives\fR replace any real file that is installed
+where an alternative link has to be installed.
 .BI \-\-skip\-auto
 Skip configuration prompt for alternatives which are properly configured
 in automatic mode. This option is only relevant with \fB\-\-config\fR or

+ 17 - 7
scripts/t/900_update_alternatives.t

@@ -30,7 +30,7 @@ my @choices = (
 	    {
 		"link" => "$bindir/slave1",
 		name => "slave1",
-		path => "/bin/yes",
+		path => "/usr/bin/yes",
 	    },
 	],
     },
@@ -52,8 +52,8 @@ my @choices = (
     },
 );
 my $nb_slaves = 2;
-plan tests => (4 * ($nb_slaves + 1) + 2) * 23 # number of check_choices
-		+ 56;			      # rest
+plan tests => (4 * ($nb_slaves + 1) + 2) * 24 # number of check_choices
+		+ 60;			      # rest
 
 sub cleanup {
     system("rm -rf $srcdir/t.tmp/ua && mkdir -p $admindir && mkdir -p $altdir");
@@ -74,8 +74,10 @@ sub call_ua {
 sub install_choice {
     my ($id, %opts) = @_;
     my $alt = $choices[$id];
-    my @params = ("--install", "$main_link", "$main_name",
-		  $alt->{path}, $alt->{priority});
+    my @params;
+    push @params, @{$opts{params}} if exists $opts{params};
+    push @params, "--install", "$main_link", "$main_name",
+		  $alt->{path}, $alt->{priority};
     foreach my $slave (@{ $alt->{slaves} }) {
 	push @params, "--slave", $slave->{"link"}, $slave->{"name"}, $slave->{"path"};
     }
@@ -266,6 +268,7 @@ $choices[0]{"slaves"}[0]{"link"} = "$bindir/generic-slave-bis";
 install_choice(0);
 check_choice(0, "auto", "rename lost file");
 check_no_link($old_slave, "rename lost file");
+$choices[0]{"slaves"}[0]{"link"} = "$bindir/slave2";
 # test install with empty admin file (#457863)
 cleanup();
 system("touch $admindir/generic-test");
@@ -328,12 +331,19 @@ check_choice(0, "auto", "optional renamed slave2 in non-existing dir");
 cleanup();
 install_choice(0);
 check_choice(0, "auto", "optional slave2 in non-existing dir");
-$choices[0]{"slaves"}[0]{"link"} = $old_link;
+$choices[0]{"slaves"}[0]{"link"} = $old_slave;
 # 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;
 
+# test management of pre-existing files
+cleanup();
+system("touch $main_link $bindir/slave1");
+install_choice(0);
+ok(!-l $main_link, "install preserves files that should be links");
+ok(!-l "$bindir/slave1", "install preserves files that should be slave links");
+install_choice(0, params => ["--force"]);
+check_choice(0, "auto", "install --force replaces files with links");
 
-# TODO: handle of pre-existing files in place of alternative links

+ 30 - 13
scripts/update-alternatives.pl

@@ -19,12 +19,13 @@ my $altdir = '/etc/alternatives';
 my $admdir = $admindir . '/alternatives';
 
 my $action = '';      # Action to perform (display / query / install / remove / auto / config)
-my $skip_auto = '';   # Skip alternatives properly configured in auto mode (for --config)
 my $alternative;      # Alternative worked on
 my $inst_alt;         # Alternative to install
 my $fileset;          # Set of files to install in the alternative
 my $path;             # Path of alternative we are offering
+my $skip_auto = 0;    # Skip alternatives properly configured in auto mode (for --config)
 my $verbosemode = 0;
+my $force = 0;
 my @pass_opts;
 
 $| = 1;
@@ -106,6 +107,9 @@ while (@ARGV) {
     } elsif (m/^--skip-auto$/) {
 	$skip_auto = 1;
         push @pass_opts, $_;
+    } elsif (m/^--force$/) {
+	$force = 1;
+        push @pass_opts, $_;
     } else {
         badusage(_g("unknown option \`%s'"), $_);
     }
@@ -572,6 +576,7 @@ sub slave {
 package Alternative;
 
 use Dpkg::Gettext;
+use POSIX qw(:errno_h);
 
 sub pr { main::pr(@_) }
 sub quit { main::quit(@_) }
@@ -951,32 +956,44 @@ sub prepare_install {
     my ($self, $choice) = @_;
     my ($link, $name) = ($self->link(), $self->name());
     my $fileset = $self->fileset($choice);
-    # Setup main link
-    main::checked_rm("$link.dpkg-tmp");
-    main::checked_symlink("$altdir/$name", "$link.dpkg-tmp");
-    # Setup main alternative link
+    # Create link in /etc/alternatives
     main::checked_rm("$altdir/$name.dpkg-tmp");
     main::checked_symlink($choice, "$altdir/$name.dpkg-tmp");
-    # Add commit operations
     $self->add_commit_op(sub {
-        main::checked_mv("$link.dpkg-tmp", $link);
         main::checked_mv("$altdir/$name.dpkg-tmp", "$altdir/$name");
     });
+    $! = 0; lstat($link);
+    if (-l _ or $! == ENOENT or $force) {
+        # Create alternative link
+        main::checked_rm("$link.dpkg-tmp");
+        main::checked_symlink("$altdir/$name", "$link.dpkg-tmp");
+        $self->add_commit_op(sub {
+            main::checked_mv("$link.dpkg-tmp", $link);
+        });
+    } else {
+        pr(_g("Not replacing %s with a link."), $link) if $verbosemode >= 0;
+    }
     # 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) and -e $spath) {
-            # Setup slave link
-            main::checked_rm("$slink.dpkg-tmp");
-            main::checked_symlink("$altdir/$slave", "$slink.dpkg-tmp");
-            # Setup slave alternative link
+            # Create link in /etc/alternatives
             main::checked_rm("$altdir/$slave.dpkg-tmp");
             main::checked_symlink($spath, "$altdir/$slave.dpkg-tmp");
-            # Add commit operations
             $self->add_commit_op(sub {
-                main::checked_mv("$slink.dpkg-tmp", $slink);
                 main::checked_mv("$altdir/$slave.dpkg-tmp", "$altdir/$slave");
             });
+            $! = 0; lstat($slink);
+            if (-l _ or $! == ENOENT or $force) {
+                # Create alternative link
+                main::checked_rm("$slink.dpkg-tmp");
+                main::checked_symlink("$altdir/$slave", "$slink.dpkg-tmp");
+                $self->add_commit_op(sub {
+                    main::checked_mv("$slink.dpkg-tmp", $slink);
+                });
+            } else {
+                pr(_g("Not replacing %s with a link."), $link) if $verbosemode >= 0;
+            }
         } else {
             main::pr(_g("skip creation of %s because associated file %s (of" .
                         "link group %s) doesn't exist"), $slink, $spath,