Hi Guillem,
your patch doesn't fix the problem, and even introduces more problems,
because as it is written it leaves all directories which have
conffiles on the disk.
This section:
static void
removal_bulk_remove_files(struct pkginfo *pkg)
{
[...]
if (namenode->flags & fnnf_old_conff) {
push_leftover(&leftover,namenode);
continue;
}
causes every conffile to be added to leftover list triggering
dir_is_used_by_pkg for every parent directory of the conffile.
However if you put back the logic to run the dir_is_used_by_pkg only
if the file is used by others then it works correctly because the
directory gets removed by last package owning the directory. (0001
does that and works)
Another option would be to split the loops and add conffiles to
leftover list after the first run (something like in the second
attached patch).
> It would be nice to have a
> test case for our functional test-suite too. :)
>
> <http://git.debian.org/?p=dpkg/pkg-tests.git>
Test case is attached.
O.
2011/5/6 Guillem Jover <guil...@debian.org>:
> Hi!
>
> On Wed, 2011-05-04 at 11:11:10 +0200, Ondřej Surý wrote:
>> The attached patch keeps the directory in the <pkg>.list (aka add it
>> to the leftover) if the children directory is found on the leftover
>> list.
>
> The problem is that the check is not correct, isdirectoryinuse (now
> dir_is_used_by_others) checks for the directory being referenced by
> other packages, and here we only care if it's owned by the package
> being acted on.
>
>> There could be a slight modification to keep /. in the list as well to
>> be formally correct, but IMO it's not needed.
>
> Right, and it actually needs to be ignored on removal or dpkg fails on
> non dpkg native systems (or fake environments), where the last package
> can be easily removed.
>
> Anyway I'm attaching a modified version, but I've only build tested
> it. If you could test it that'd be nice. It would be nice to have a
> test case for our functional test-suite too. :)
>
> <http://git.debian.org/?p=dpkg/pkg-tests.git>
>
>> Also I have found a small bug in the matching function
>> hasdirectoryconffiles and I am inheriting it (since I don't think it's
>> major)...
>>
>> If there is a conffile named:
>>
>> /etc/pkg/foobar/foo.conf
>>
>> and directory
>>
>> /etc/pkg/foo
>>
>> then the /etc/pkg/foo is matched and not removed.
>>
>> If you want this fixed, just ping me, it's easy to add something like
>> "&& (....->name[namelen] == \0 or ...->name[namelen] == '/')", and
>> I'll fix it at both places.
>
> Ah nice catch! I've pushed a fix for this (commit
> 2c9a342dc4e1ad3e9e58ac89957b9068664d1930). Thanks!
>
> regards,
> guillem
>
--
Ondřej Surý <ond...@sury.org>
http://blog.rfc1925.org/
From bc5b2fb02cc6474f259050c92e1c568eda738d16 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ond...@sury.org>
Date: Wed, 11 May 2011 09:45:51 +0200
Subject: [PATCH] Keep parent directories in the leftover list if the child is not removed.
---
src/help.c | 26 ++++++++++++++++++++++++++
src/main.h | 2 ++
src/remove.c | 12 ++++++++++--
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/src/help.c b/src/help.c
index 4a2256f..5b0fd39 100644
--- a/src/help.c
+++ b/src/help.c
@@ -512,6 +512,32 @@ dir_is_used_by_others(struct filenamenode *file, struct pkginfo *pkg)
return false;
}
+bool
+dir_is_used_by_pkg(struct filenamenode *file, struct pkginfo *pkg,
+ struct fileinlist *list)
+{
+ struct fileinlist *node;
+ size_t namelen;
+
+ debug(dbg_veryverbose, "dir_is_used_by_pkg '%s' (by %s)",
+ file->name, pkg ? pkg->name : "<none>");
+
+ namelen = strlen(file->name);
+
+ for (node = list; node; node = node->next) {
+ debug(dbg_veryverbose, "dir_is_used_by_pkg considering %s ...",
+ node->namenode->name);
+
+ if (strncmp(file->name, node->namenode->name, namelen) == 0 &&
+ node->namenode->name[namelen] == '/')
+ return true;
+ }
+
+ debug(dbg_veryverbose, "dir_is_used_by_pkg no");
+
+ return false;
+}
+
void oldconffsetflags(const struct conffile *searchconff) {
struct filenamenode *namenode;
diff --git a/src/main.h b/src/main.h
index 12d12e7..3b8e094 100644
--- a/src/main.h
+++ b/src/main.h
@@ -249,6 +249,8 @@ void post_postinst_tasks(struct pkginfo *pkg, enum pkgstatus new_status);
void clear_istobes(void);
bool dir_is_used_by_others(struct filenamenode *namenode, struct pkginfo *pkg);
+bool dir_is_used_by_pkg(struct filenamenode *namenode, struct pkginfo *pkg,
+ struct fileinlist *list);
bool dir_has_conffiles(struct filenamenode *namenode, struct pkginfo *pkg);
void log_action(const char *action, struct pkginfo *pkg);
diff --git a/src/remove.c b/src/remove.c
index a33518d..b2e706a 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -257,8 +257,12 @@ removal_bulk_remove_files(struct pkginfo *pkg)
push_leftover(&leftover,namenode);
continue;
}
- if (dir_is_used_by_others(namenode, pkg))
+ if (dir_is_used_by_others(namenode, pkg)) {
+ if (dir_is_used_by_pkg(namenode, pkg, leftover)) {
+ push_leftover(&leftover, namenode);
+ }
continue;
+ }
}
debug(dbg_eachfiledetail, "removal_bulk removing `%s'", fnvb.buf);
if (!rmdir(fnvb.buf) || errno == ENOENT || errno == ELOOP) continue;
@@ -337,8 +341,12 @@ static void removal_bulk_remove_leftover_dirs(struct pkginfo *pkg) {
push_leftover(&leftover,namenode);
continue;
}
- if (dir_is_used_by_others(namenode, pkg))
+ if (dir_is_used_by_others(namenode, pkg)) {
+ if (dir_is_used_by_pkg(namenode, pkg, leftover)) {
+ push_leftover(&leftover, namenode);
+ }
continue;
+ }
}
debug(dbg_eachfiledetail, "removal_bulk removing `%s'", fnvb.buf);
--
1.7.2.5
From 553f0cefde56fb274abd2362800aabc0f37d83cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ond...@sury.org>
Date: Wed, 11 May 2011 09:45:51 +0200
Subject: [PATCH] Add conffiles in second loop.
---
src/help.c | 26 ++++++++++++++++++++++++++
src/main.h | 2 ++
src/remove.c | 45 +++++++++++++++++++++++++++++++++++++--------
3 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/src/help.c b/src/help.c
index 4a2256f..5b0fd39 100644
--- a/src/help.c
+++ b/src/help.c
@@ -512,6 +512,32 @@ dir_is_used_by_others(struct filenamenode *file, struct pkginfo *pkg)
return false;
}
+bool
+dir_is_used_by_pkg(struct filenamenode *file, struct pkginfo *pkg,
+ struct fileinlist *list)
+{
+ struct fileinlist *node;
+ size_t namelen;
+
+ debug(dbg_veryverbose, "dir_is_used_by_pkg '%s' (by %s)",
+ file->name, pkg ? pkg->name : "<none>");
+
+ namelen = strlen(file->name);
+
+ for (node = list; node; node = node->next) {
+ debug(dbg_veryverbose, "dir_is_used_by_pkg considering %s ...",
+ node->namenode->name);
+
+ if (strncmp(file->name, node->namenode->name, namelen) == 0 &&
+ node->namenode->name[namelen] == '/')
+ return true;
+ }
+
+ debug(dbg_veryverbose, "dir_is_used_by_pkg no");
+
+ return false;
+}
+
void oldconffsetflags(const struct conffile *searchconff) {
struct filenamenode *namenode;
diff --git a/src/main.h b/src/main.h
index 12d12e7..3b8e094 100644
--- a/src/main.h
+++ b/src/main.h
@@ -249,6 +249,8 @@ void post_postinst_tasks(struct pkginfo *pkg, enum pkgstatus new_status);
void clear_istobes(void);
bool dir_is_used_by_others(struct filenamenode *namenode, struct pkginfo *pkg);
+bool dir_is_used_by_pkg(struct filenamenode *namenode, struct pkginfo *pkg,
+ struct fileinlist *list);
bool dir_has_conffiles(struct filenamenode *namenode, struct pkginfo *pkg);
void log_action(const char *action, struct pkginfo *pkg);
diff --git a/src/remove.c b/src/remove.c
index a33518d..06ec05c 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -219,12 +219,11 @@ removal_bulk_remove_files(struct pkginfo *pkg)
while ((namenode= reversefilelist_next(&rlistit))) {
struct filenamenode *usenode;
- debug(dbg_eachfile, "removal_bulk `%s' flags=%o",
- namenode->name, namenode->flags);
if (namenode->flags & fnnf_old_conff) {
- push_leftover(&leftover,namenode);
continue;
}
+ debug(dbg_eachfile, "removal_bulk `%s' flags=%o",
+ namenode->name, namenode->flags);
usenode = namenodetouse(namenode, pkg);
trig_file_activate(usenode, pkg);
@@ -257,8 +256,13 @@ removal_bulk_remove_files(struct pkginfo *pkg)
push_leftover(&leftover,namenode);
continue;
}
- if (dir_is_used_by_others(namenode, pkg))
+ if (dir_is_used_by_pkg(namenode, pkg, leftover)) {
+ push_leftover(&leftover, namenode);
continue;
+ }
+ if (dir_is_used_by_others(namenode, pkg)) {
+ continue;
+ }
}
debug(dbg_eachfiledetail, "removal_bulk removing `%s'", fnvb.buf);
if (!rmdir(fnvb.buf) || errno == ENOENT || errno == ELOOP) continue;
@@ -284,6 +288,17 @@ removal_bulk_remove_files(struct pkginfo *pkg)
if (secure_unlink(fnvb.buf))
ohshite(_("unable to securely remove '%.250s'"), fnvb.buf);
}
+
+ reversefilelist_init(&rlistit,pkg->clientdata->files);
+ while ((namenode= reversefilelist_next(&rlistit))) {
+ debug(dbg_eachfile, "removal_bulk `%s' flags=%o",
+ namenode->name, namenode->flags);
+ if (namenode->flags & fnnf_old_conff) {
+ push_leftover(&leftover,namenode);
+ continue;
+ }
+ }
+
write_filelist_except(pkg,leftover,0);
maintainer_script_installed(pkg, POSTRMFILE, "post-removal",
"remove", NULL);
@@ -313,12 +328,11 @@ static void removal_bulk_remove_leftover_dirs(struct pkginfo *pkg) {
while ((namenode= reversefilelist_next(&rlistit))) {
struct filenamenode *usenode;
- debug(dbg_eachfile, "removal_bulk `%s' flags=%o",
- namenode->name, namenode->flags);
if (namenode->flags & fnnf_old_conff) {
- push_leftover(&leftover,namenode);
continue;
}
+ debug(dbg_eachfile, "removal_bulk `%s' flags=%o",
+ namenode->name, namenode->flags);
usenode = namenodetouse(namenode, pkg);
trig_file_activate(usenode, pkg);
@@ -337,8 +351,13 @@ static void removal_bulk_remove_leftover_dirs(struct pkginfo *pkg) {
push_leftover(&leftover,namenode);
continue;
}
- if (dir_is_used_by_others(namenode, pkg))
+ if (dir_is_used_by_pkg(namenode, pkg, leftover)) {
+ push_leftover(&leftover, namenode);
+ continue;
+ }
+ if (dir_is_used_by_others(namenode, pkg)) {
continue;
+ }
}
debug(dbg_eachfiledetail, "removal_bulk removing `%s'", fnvb.buf);
@@ -365,6 +384,16 @@ static void removal_bulk_remove_leftover_dirs(struct pkginfo *pkg) {
push_leftover(&leftover,namenode);
continue;
}
+
+ reversefilelist_init(&rlistit,pkg->clientdata->files);
+ while ((namenode= reversefilelist_next(&rlistit))) {
+ debug(dbg_eachfile, "removal_bulk `%s' flags=%o",
+ namenode->name, namenode->flags);
+ if (namenode->flags & fnnf_old_conff) {
+ push_leftover(&leftover,namenode);
+ continue;
+ }
+ }
write_filelist_except(pkg,leftover,0);
modstatdb_note(pkg);
--
1.7.2.5
From 805da7cd066fa8ddf50c4954a8ef24f7eda0fe85 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ond...@sury.org>
Date: Wed, 11 May 2011 10:58:20 +0200
Subject: [PATCH] Add test for leftover dirs for directories containing files purged
from postrm (all ucf conffiles).
---
Makefile | 1 +
t-dir-leftover/Makefile | 15 +++++++++++++++
t-dir-leftover/pkg-dir-leftover-0/DEBIAN/control | 8 ++++++++
t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postinst | 10 ++++++++++
t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postrm | 7 +++++++
t-dir-leftover/pkg-dir-leftover-1/DEBIAN/control | 9 +++++++++
t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postinst | 10 ++++++++++
t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postrm | 7 +++++++
8 files changed, 67 insertions(+), 0 deletions(-)
create mode 100644 t-dir-leftover/Makefile
create mode 100644 t-dir-leftover/pkg-dir-leftover-0/DEBIAN/control
create mode 100755 t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postinst
create mode 100755 t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postrm
create mode 100644 t-dir-leftover/pkg-dir-leftover-1/DEBIAN/control
create mode 100755 t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postinst
create mode 100755 t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postrm
diff --git a/Makefile b/Makefile
index 0458cef..ac28e97 100644
--- a/Makefile
+++ b/Makefile
@@ -57,6 +57,7 @@ TESTS_PASS := \
t-symlink-dir \
t-substvars \
t-failinst-failrm \
+ t-dir-leftover \
t-dir-extension-check
ifneq (,$(filter test-all,$(DPKG_TESTSUITE_OPTIONS)))
diff --git a/t-dir-leftover/Makefile b/t-dir-leftover/Makefile
new file mode 100644
index 0000000..1599370
--- /dev/null
+++ b/t-dir-leftover/Makefile
@@ -0,0 +1,15 @@
+TESTS_DEB := pkg-dir-leftover-0 pkg-dir-leftover-1
+
+include ../Test.mk
+
+test-case:
+ $(DPKG_INSTALL) pkg-dir-leftover-0.deb
+ $(DPKG_INSTALL) pkg-dir-leftover-1.deb
+ $(DPKG_REMOVE) pkg-dir-leftover-1
+ $(DPKG_REMOVE) pkg-dir-leftover-0
+ $(DPKG_PURGE) pkg-dir-leftover-0
+ $(DPKG_PURGE) pkg-dir-leftover-1
+ test ! -d /test-confdir
+
+test-clean:
+ rmdir /test-confdir
diff --git a/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/control b/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/control
new file mode 100644
index 0000000..c24544d
--- /dev/null
+++ b/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/control
@@ -0,0 +1,8 @@
+Package: pkg-dir-leftover-0
+Version: 0
+Section: test
+Priority: extra
+Maintainer: Ondřej Surý <ond...@debian.org>
+Architecture: all
+Description: test package - confdir not removed
+
diff --git a/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postinst b/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postinst
new file mode 100755
index 0000000..11f39e3
--- /dev/null
+++ b/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postinst
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+case "$1" in
+ configure)
+ touch /test-confdir/test-conffile-0
+ ;;
+esac
+
+
+
\ No newline at end of file
diff --git a/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postrm b/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postrm
new file mode 100755
index 0000000..1624f61
--- /dev/null
+++ b/t-dir-leftover/pkg-dir-leftover-0/DEBIAN/postrm
@@ -0,0 +1,7 @@
+#!/bin/sh
+
+case "$1" in
+ purge)
+ rm /test-confdir/test-conffile-0
+ ;;
+esac
diff --git a/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/control b/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/control
new file mode 100644
index 0000000..d7e5dc2
--- /dev/null
+++ b/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/control
@@ -0,0 +1,9 @@
+Package: pkg-dir-leftover-1
+Version: 0
+Section: test
+Priority: extra
+Maintainer: Ondřej Surý <ond...@debian.org>
+Architecture: all
+Depends: pkg-dir-leftover-0
+Description: test package - confdir not removed
+
diff --git a/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postinst b/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postinst
new file mode 100755
index 0000000..1415d73
--- /dev/null
+++ b/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postinst
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+case "$1" in
+ configure)
+ touch /test-confdir/test-conffile-1
+ ;;
+esac
+
+
+
\ No newline at end of file
diff --git a/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postrm b/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postrm
new file mode 100755
index 0000000..e9d7079
--- /dev/null
+++ b/t-dir-leftover/pkg-dir-leftover-1/DEBIAN/postrm
@@ -0,0 +1,7 @@
+#!/bin/sh
+
+case "$1" in
+ purge)
+ rm /test-confdir/test-conffile-1
+ ;;
+esac
--
1.7.2.5
