On Sun, Jul 04, 2010 at 06:57:16PM +0200, Andreas Beckmann wrote:
> dpkg-divert --add --rename incorrectly renames an existing file that is
> owned by the package given with --package. For example see the following
> commands:
> 
> # dpkg -S /usr/bin/users
> coreutils: /usr/bin/users
> # ls -la /usr/bin/users*
> -rwxr-xr-x 1 root root 28368 Apr 28 04:05 /usr/bin/users
> # dpkg-divert --add --rename --package coreutils --divert 
> /usr/bin/users.diverted /usr/bin/users
> Adding `diversion of /usr/bin/users to /usr/bin/users.diverted by coreutils'
> # ls -la /usr/bin/users*
> -rwxr-xr-x 1 root root 28368 Apr 28 04:05 /usr/bin/users.diverted
> 
> Oops, that should not have happened!
> 
> 
> A patch that adds a check for the owning package of the to be renamed
> file is attached.

We ran into this in Ubuntu due to chaos with linker conflicts in
libc6-i386 vs. libc6:i386, and wished that we'd had this in advance.

How about something like this for a C version?  I'd appreciate review as
I'm not entirely familiar with all the internal interfaces here.

diff --git a/src/divertcmd.c b/src/divertcmd.c
index 6d3c8bf..39fa25b 100644
--- a/src/divertcmd.c
+++ b/src/divertcmd.c
@@ -47,6 +47,7 @@
 #include <dpkg/options.h>
 
 #include "filesdb.h"
+#include "main.h"
 
 
 static const char printforhelp[] = N_(
@@ -171,8 +172,10 @@ check_writable_dir(struct file *f)
 }
 
 static bool
-check_rename(struct file *src, struct file *dst)
+check_rename(struct file *src, struct file *dst, struct pkgset *set)
 {
+       struct pkginfo *pkg;
+
        file_stat(src);
 
        /* If the source file is not present and we are not going to do
@@ -202,6 +205,24 @@ check_rename(struct file *src, struct file *dst)
                         "  different file `%s', not allowed"),
                        dst->name, src->name);
 
+       if (set) {
+               for (pkg = &set->pkg; pkg; pkg = pkg->arch_next) {
+                       struct fileinlist *file;
+
+                       ensure_packagefiles_available(pkg);
+                       file = pkg->clientdata->files;
+                       while (file) {
+                               if (strcmp (file->namenode->name,
+                                           src->name) == 0) {
+                                       /* Owned by the same package as
+                                        * given in --package; don't rename.
+                                        */
+                                       return false;
+                               }
+                       }
+               }
+       }
+
        return true;
 }
 
@@ -467,7 +488,7 @@ diversion_add(const char *const *argv)
        if (opt_verbose > 0)
                printf(_("Adding '%s'\n"), diversion_describe(contest));
        if (opt_rename)
-               opt_rename = check_rename(&file_from, &file_to);
+               opt_rename = check_rename(&file_from, &file_to, pkgset);
        if (!opt_test) {
                divertdb_write();
                if (opt_rename)
@@ -576,7 +597,7 @@ diversion_remove(const char *const *argv)
        altname->camefrom->divert = NULL;
 
        if (opt_rename)
-               opt_rename = check_rename(&file_to, &file_from);
+               opt_rename = check_rename(&file_to, &file_from, pkgset);
        if (opt_rename && !opt_test)
                file_rename(&file_to, &file_from);
 
diff --git a/src/t/100_dpkg_divert.t b/src/t/100_dpkg_divert.t
index 6d62fe5..cbd0436 100644
--- a/src/t/100_dpkg_divert.t
+++ b/src/t/100_dpkg_divert.t
@@ -33,11 +33,12 @@ if (! -x "@dd") {
     exit(0);
 }
 
-plan tests => 251;
+plan tests => 257;
 
 sub cleanup {
     system("rm -rf $tmpdir && mkdir -p $testdir");
-    system("mkdir -p $admindir/updates && touch $admindir/status");
+    system("mkdir -p $admindir/updates && rm -f $admindir/status && touch 
$admindir/status");
+    system("rm -rf $admindir/info && mkdir -p $admindir/info");
 }
 
 sub install_diversions {
@@ -47,6 +48,27 @@ sub install_diversions {
     close(O);
 }
 
+sub install_filelist {
+    my ($pkg, $arch, @files) = @_;
+    open(L, "> $admindir/info/$pkg.list");
+    for my $file (@files) {
+        print L "$file\n";
+    }
+    close(L);
+    # Only installed packages have their files list considered.
+    open(S, ">> $admindir/status");
+    print S <<EOF;
+Package: $pkg
+Status: install ok installed
+Version: 0
+Architecture: $arch
+Maintainer: dummy
+Description: dummy
+
+EOF
+    close(S);
+}
+
 sub call {
     my ($prog, $args, %opts) = @_;
 
@@ -392,6 +414,25 @@ EOF
 
 cleanup();
 
+note("Adding diversion of file owned by --package");
+
+install_filelist("coreutils", "i386", "$testdir/foo");
+system("cat $admindir/info/coreutils.list >&2");
+install_diversions('');
+system("touch $testdir/foo");
+
+call_divert(['--quiet', '--rename', '--add', '--package', 'coreutils', 
"$testdir/foo"],
+            expect_stderr => '', expect_stdout => '');
+ok(-e "$testdir/foo", "foo not renamed");
+ok(!-e "$testdir/foo.distrib", "foo renamed");
+diversions_eq(<<EOF);
+$testdir/foo
+$testdir/foo.distrib
+coreutils
+EOF
+
+cleanup();
+
 note("Remove diversions");
 
 install_diversions('');

-- 
Colin Watson                                       [cjwat...@ubuntu.com]



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

Reply via email to