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