Hi,
On Thu, 08 Mar 2012, Andreas Beckmann wrote:
> (regarding the original problem:)
> I'm seeing a lot of owned directories remaining in the piuparts tests for sid
> (they are treated as error for sid and warning for other distributions):
> http://piuparts.debian.org/sid/owned_files_by_many_packages_error.html
> http://piuparts.debian.org/sid/owned_files_after_purge_error.html
>
> I tried to create a minimal package that triggers the behaviour. Looks
> like it is related to directories owned by multiple packages and their
> removal order.
Basically, this is a variant of the test case "t-dir-leftover-deadlock":
http://anonscm.debian.org/gitweb/?p=dpkg/pkg-tests.git;a=tree;f=t-dir-leftover-deadlock;h=604270fa0caa72f31e5e1d73ec0f6fa02bd84f91;hb=master
And your analysis is correct.
We know a solution to the problem which is basically to not forget about
directories shared with other packages. But this approach leads us to
keep ownership of lots of directories which are not really needed.
But the more I think about it, the more I'm convinced that it's
the least evil solution until we have a way to register arbitrary files in
dpkg. (I don't really like the complexity of the dpkg-maintscript-helper
snippet we discussed)
I would suggest to go with the attached patch which imposes just a
supplementary restriction. It will only remember shared directories
if the package has a postrm script (and thus has a chance to do something
during purge).
I believe this would make a good enough compromise. The number of useless
directory ownerships is much less problematic than the failure to properly
cleanup.
Another restriction which I did not implement but that we could implement
is that we can safely forget about directories which are shared but which
are empty. (But it would not make a huge difference because I don't know
of many shared directories that stay empty)
Cheers,
PS: In the attached patch, I wondered whether it was best to duplicate
foundpostrm or to modify the prototype of removal_bulk_remove_files()
so that removal_bulk() could forward it (since it already computes it).
Since boolean parameter are best avoided I opted to duplicate it.
--
Raphaël Hertzog ◈ Debian Developer
Get the Debian Administrator's Handbook:
→ http://debian-handbook.info/get/
>From 0c28f55065c671572849f2cbc1f8cf37f9946c31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Hertzog?= <hert...@debian.org>
Date: Fri, 13 Jul 2012 10:45:33 +0200
Subject: [PATCH] dpkg: do not forget about shared directories if we have a
postrm script
This is meant to fix the case where the postrm purge is responsible of
removing a file that might be in those directories.
Closes: #316521
---
src/remove.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/remove.c b/src/remove.c
index 95ea98f..a1c0a2e 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -240,12 +240,15 @@ static void
removal_bulk_remove_files(struct pkginfo *pkg)
{
int before;
+ bool foundpostrm;
struct reversefilelistiter rlistit;
struct fileinlist *leftover;
struct filenamenode *namenode;
static struct varbuf fnvb;
struct stat stab;
+ foundpostrm = pkg_infodb_has_file(pkg, &pkg->installed, POSTRMFILE);
+
pkg_set_status(pkg, stat_halfinstalled);
modstatdb_note(pkg);
push_checkpoint(~ehflag_bombout, ehflag_normaltidy);
@@ -301,8 +304,9 @@ removal_bulk_remove_files(struct pkginfo *pkg)
if (is_dir) {
debug(dbg_eachfiledetail, "removal_bulk is a directory");
/* Only delete a directory or a link to one if we're the only
- * package which uses it. Other files should only be listed
- * in this package (but we don't check). */
+ * package which uses it. Ensure we don't forget about it
+ * in all the cases where we might have to retry its removal
+ * during purge. */
if (dir_has_conffiles(namenode, pkg)) {
push_leftover(&leftover,namenode);
continue;
@@ -311,8 +315,11 @@ 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 (foundpostrm)
+ push_leftover(&leftover, namenode);
continue;
+ }
}
debug(dbg_eachfiledetail, "removal_bulk removing '%s'", fnvb.buf);
if (!rmdir(fnvb.buf) || errno == ENOENT || errno == ELOOP) continue;
--
1.7.10.4
