[pacman-dev] [RFC] Correct directory permissions if the extracting package is the only owner

[Allan McRae]

Adjusting permissions of directories in their install scripts results in
warnings such as the following when the package update:

warning: directory ownership differs on /var/lib/postfix/
filesystem: 73:0 package: 0:0

If the package being installed is the only owner of the directory, update
the directory's permissions to those in the package file.

This also allows directory permissions for a package to be changed directly
in the PKGBUILD file rather that requiring adjustment in an install scriptlet.

Signed-off-by: Allan McRae <allan at archlinux.org>
---

This does require searching through all installed packages filelist to see if
any other directories own this file, which does create a slowdown in these
error cases. Once the ability to use symbolic names for owners is added, this
code path will run only if there is an acutal error or in the rare case that
directory permissions are genuinely being changed.

Packages can currently avoid this being run by reverting their directory
permission changes in pre_upgrade().

I'd like to push this to maint if people agree.  These "false" warnings are
causing too many issues.

 lib/libalpm/add.c | 61 +++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 15 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index 82461ee..60db22f 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -167,6 +167,20 @@ static int extract_db_file(alpm_handle_t *handle, struct archive *archive,
 	return perform_extraction(handle, archive, entry, filename);
 }
 
+static int other_directory_owners(alpm_handle_t *handle, const char *entryname, const char *pkgname) {
+	alpm_list_t *packages = alpm_db_get_pkgcache(handle->db_local);
+	alpm_list_t *i;
+
+	for(i = packages; i ; i = alpm_list_next(i)) {
+		if(alpm_filelist_contains(alpm_pkg_get_files(i->data), entryname) &&
+				strcmp(((alpm_pkg_t *)(i->data))->name, pkgname) != 0) {
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
 		struct archive_entry *entry, alpm_pkg_t *newpkg, alpm_pkg_t *oldpkg)
 {
@@ -232,25 +246,42 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
 
 		/* case 6: existing dir, ignore it */
 		if(lsbuf.st_mode != entrymode) {
-			/* if filesystem perms are different than pkg perms, warn user */
 			mode_t mask = 07777;
-			_alpm_log(handle, ALPM_LOG_WARNING, _("directory permissions differ on %s\n"
-					"filesystem: %o  package: %o\n"), filename, lsbuf.st_mode & mask,
-					entrymode & mask);
-			alpm_logaction(handle, ALPM_CALLER_PREFIX,
-					"warning: directory permissions differ on %s\n"
-					"filesystem: %o  package: %o\n", filename, lsbuf.st_mode & mask,
-					entrymode & mask);
+			if(other_directory_owners(handle, entryname, newpkg->name) == 0) {
+				/* no other package owns this directory, "extract" with correct permissions */
+				if(chmod(filename, entrymode) != 0) {
+					goto warn_permissions;
+				}
+			} else {
+warn_permissions:
+				/* if filesystem perms are different than pkg perms, warn user */
+				_alpm_log(handle, ALPM_LOG_WARNING, _("directory permissions differ on %s\n"
+						"filesystem: %o  package: %o\n"), filename, lsbuf.st_mode & mask,
+						entrymode & mask);
+				alpm_logaction(handle, ALPM_CALLER_PREFIX,
+						"warning: directory permissions differ on %s\n"
+						"filesystem: %o  package: %o\n", filename, lsbuf.st_mode & mask,
+						entrymode & mask);
+
+			}
 		}
 
 		if((entryuid != lsbuf.st_uid) || (entrygid != lsbuf.st_gid)) {
-			_alpm_log(handle, ALPM_LOG_WARNING, _("directory ownership differs on %s\n"
-					"filesystem: %u:%u  package: %u:%u\n"), filename,
-					lsbuf.st_uid, lsbuf.st_gid, entryuid, entrygid);
-			alpm_logaction(handle, ALPM_CALLER_PREFIX,
-					"warning: directory ownership differs on %s\n"
-					"filesystem: %u:%u  package: %u:%u\n", filename,
-					lsbuf.st_uid, lsbuf.st_gid, entryuid, entrygid);
+			if(other_directory_owners(handle, entryname, newpkg->name) == 0) {
+				/* no other package owns this directory, "extract" with correct owners */
+				if(chown(filename, entryuid, entrygid) != 0) {
+					goto warn_owners;
+				}
+			} else {
+warn_owners:
+				_alpm_log(handle, ALPM_LOG_WARNING, _("directory ownership differs on %s\n"
+						"filesystem: %u:%u  package: %u:%u\n"), filename,
+						lsbuf.st_uid, lsbuf.st_gid, entryuid, entrygid);
+				alpm_logaction(handle, ALPM_CALLER_PREFIX,
+						"warning: directory ownership differs on %s\n"
+						"filesystem: %u:%u  package: %u:%u\n", filename,
+						lsbuf.st_uid, lsbuf.st_gid, entryuid, entrygid);
+			}
 		}
 
 		_alpm_log(handle, ALPM_LOG_DEBUG, "extract: skipping dir extraction of %s\n",
-- 
2.2.2