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