update-alternatives: fix regression when dropping slave alternatives

When the alternative is changed, it might have to drop a slave alternative
that's no longer provide by the new choice. This must only remove the slave
link if it's actually a link. If it's a file, it must be kept except if
--force is given.

The non-regression tests have been contributed by Andreas Beckmann.

Reported-by: Andreas Beckmann <debian@abeckmann.de>

debian/changelog

@@ -74,6 +74,9 @@ dpkg (1.16.1) UNRELEASED; urgency=low
   * dpkg-mergechangelogs now checks the return value of the close() call.
     Thanks to Niels Thykier <niels@thykier.net> for the patch. Closes: #633539
   * Similar changes to dpkg-shlibdeps and dpkg-gencontrol, also by Niels.
+  * Fix update-alternatives to not remove a real file when dropping a
+    symlink for a slave that's not provided by the new current choice.
+    Closes: #633627
 
   [ Guillem Jover ]
   * Install deb-src-control(5) man pages in dpkg-dev. Closes: #620520

utils/t/100_update_alternatives.t

@@ -80,8 +80,8 @@ my @choices = (
     },
 );
 my $nb_slaves = 4;
-plan tests => (4 * ($nb_slaves + 1) + 2) * 25 # number of check_choices
-		+ 70;			      # rest
+plan tests => (4 * ($nb_slaves + 1) + 2) * 26 # number of check_choices
+               + 100;                         # rest
 
 sub cleanup {
     system("rm -rf $tmpdir && mkdir -p $admindir && mkdir -p $altdir");
@@ -132,7 +132,9 @@ sub remove_all_choices {
 sub set_choice {
     my ($id, %opts) = @_;
     my $alt = $choices[$id];
-    my @params = ("--set", $main_name, $alt->{path});
+    my @params;
+    push @params, @{$opts{params}} if exists $opts{params};
+    push @params, "--set", $main_name, $alt->{path};
     call_ua(\@params, %opts);
 }
 
@@ -444,3 +446,42 @@ ok(-f $main_link, "removal keeps real file installed as master link");
 ok(-f "$bindir/slave1", "removal keeps real files installed as slave links");
 install_choice(0, params => ["--force"]);
 check_choice(0, "auto", "install --force replaces files with links");
+
+# test management of pre-existing files #2
+cleanup();
+system("touch $main_link $bindir/slave2");
+install_choice(0);
+install_choice(1);
+ok(!-l $main_link, "inactive install preserves files that should be links");
+ok(!-l "$bindir/slave2", "inactive install preserves files that should be slave links");
+ok(-f $main_link, "inactive install keeps real file installed as master link");
+ok(-f "$bindir/slave2", "inactive install keeps real files installed as slave links");
+set_choice(1);
+ok(!-l $main_link, "manual switching preserves files that should be links");
+ok(!-l "$bindir/slave2", "manual switching preserves files that should be slave links");
+ok(-f $main_link, "manual switching keeps real file installed as master link");
+ok(-f "$bindir/slave2", "manual switching keeps real files installed as slave links");
+remove_choice(1);
+ok(!-l $main_link, "auto switching preserves files that should be links");
+ok(!-l "$bindir/slave2", "auto switching preserves files that should be slave links");
+ok(-f $main_link, "auto switching keeps real file installed as master link");
+ok(-f "$bindir/slave2", "auto switching keeps real files installed as slave links");
+
+# test management of pre-existing files #3
+cleanup();
+system("touch $main_link $bindir/slave2");
+install_choice(0);
+install_choice(1);
+remove_choice(0);
+ok(!-l $main_link, "removal + switching preserves files that should be links");
+ok(!-l "$bindir/slave2", "removal + switching preserves files that should be slave links");
+ok(-f $main_link, "removal + switching keeps real file installed as master link");
+ok(-f "$bindir/slave2", "removal + switching keeps real files installed as slave links");
+install_choice(0);
+ok(!-l $main_link, "install + switching preserves files that should be links");
+ok(!-l "$bindir/slave2", "install + switching preserves files that should be slave links");
+ok(-f $main_link, "install + switching keeps real file installed as master link");
+ok(-f "$bindir/slave2", "install + switching keeps real files installed as slave links");
+set_choice(1, params => ["--force"]);
+ok(!-e "$bindir/slave2", "forced switching w/o slave drops real files installed as slave links");
+check_choice(1, "manual", "set --force replaces files with links");

utils/update-alternatives.c

@@ -1623,13 +1623,29 @@ alternative_commit(struct alternative *a)
 	alternative_commit_operations_free(a);
 }
 
+static bool
+alternative_can_replace_link(const char *linkname)
+{
+	struct stat st;
+	bool replace_link;
+
+	errno = 0;
+	if (lstat(linkname, &st) == -1) {
+		if (errno != ENOENT)
+			syserr(_("cannot stat file '%s'"), linkname);
+		replace_link = true;
+	} else {
+		replace_link = S_ISLNK(st.st_mode);
+	}
+
+	return (replace_link || opt_force);
+}
+
 static void
 alternative_prepare_install_single(struct alternative *a, const char *name,
 				   const char *linkname, const char *file)
 {
 	char *fntmp, *fn;
-	struct stat st;
-	bool create_link;
 
 	/* Create link in /etc/alternatives. */
 	xasprintf(&fntmp, "%s/%s" DPKG_TMP_EXT, altdir, name);
@@ -1639,15 +1655,7 @@ alternative_prepare_install_single(struct alternative *a, const char *name,
 	alternative_add_commit_op(a, opcode_mv, fntmp, fn);
 	free(fntmp);
 
-	errno = 0;
-	if (lstat(linkname, &st) == -1) {
-		if (errno != ENOENT)
-			syserr(_("cannot stat file '%s'"), linkname);
-		create_link = true;
-	} else {
-		create_link = S_ISLNK(st.st_mode);
-	}
-	if (create_link || opt_force) {
+	if (alternative_can_replace_link(linkname)) {
 		/* Create alternative link. */
 		xasprintf(&fntmp, "%s" DPKG_TMP_EXT, linkname);
 		checked_rm(fntmp);
@@ -1693,7 +1701,11 @@ alternative_prepare_install(struct alternative *a, const char *choice)
 
 		/* Drop unused slave. */
 		xasprintf(&fn, "%s/%s", altdir, sl->name);
-		alternative_add_commit_op(a, opcode_rm, sl->link, NULL);
+		if (alternative_can_replace_link(sl->link))
+			alternative_add_commit_op(a, opcode_rm, sl->link, NULL);
+		else
+			warning(_("not removing %s since it's not a symlink."),
+			        sl->link);
 		alternative_add_commit_op(a, opcode_rm, fn, NULL);
 		free(fn);
 	}