[jhbuild] Delete no longer shipped files at install time

[Colin Walters <walters src gnome org>]

commit 9dcbb9ad41d6fb919dd8c8e7c26d96ab939e1822
Author: Colin Walters <walters verbum org>
Date:   Mon Jul 18 16:50:25 2011 -0400

    Delete no longer shipped files at install time
    
    Many very difficult to debug build problems arise when a component at
    some point in time ships a file (like a shared library), then later
    switches to not shipping it, but the file remains in the jhbuild root.
    
    This patch adjusts jhbuild to finally take advantage of the manifests
    we have in the packagedb thanks to using make install DESTDIR.
    
    I unified some work being done in 'jhbuild uninstall' with the install
    process in a new 'fileutils.py'.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=654872

 jhbuild/modtypes/__init__.py |   30 +++++++++++++-
 jhbuild/utils/Makefile.am    |    1 +
 jhbuild/utils/fileutils.py   |   91 ++++++++++++++++++++++++++++++++++++++++++
 jhbuild/utils/packagedb.py   |   69 +++++++++----------------------
 4 files changed, 141 insertions(+), 50 deletions(-)
---
diff --git a/jhbuild/modtypes/__init__.py b/jhbuild/modtypes/__init__.py
index 5bb1fb1..487855c 100644
--- a/jhbuild/modtypes/__init__.py
+++ b/jhbuild/modtypes/__init__.py
@@ -34,6 +34,7 @@ import logging
 from jhbuild.errors import FatalError, CommandError, BuildStateError, \
              SkipToEnd, UndefinedRepositoryError
 from jhbuild.utils.sxml import sxml
+import jhbuild.utils.fileutils as fileutils
 
 _module_types = {}
 def register_module_type(name, parse_func):
@@ -236,7 +237,13 @@ them into the prefix."""
 
         stripped_prefix = buildscript.config.prefix[1:]
 
-        buildscript.moduleset.packagedb.add(self.name, revision or '', destdir)
+        previous_entry = buildscript.moduleset.packagedb.get(self.name)
+        if previous_entry:
+            previous_contents = previous_entry.get_manifest()
+        else:
+            previous_contents = None
+
+        new_contents = fileutils.accumulate_dirtree_contents(destdir)
 
         install_succeeded = False
         save_broken_tree = False
@@ -284,6 +291,27 @@ them into the prefix."""
 
         if not install_succeeded:
             raise CommandError(_("Module failed to install into DESTDIR %(dest)r") % {'dest': broken_name})
+        else:
+            absolute_new_contents = map(lambda x: '/' + x, new_contents)
+            to_delete = []
+            if previous_contents is not None:
+                for path in previous_contents:
+                    if path not in absolute_new_contents:
+                        to_delete.append(path)
+                # Ensure we're only attempting to delete files in the prefix
+                to_delete = fileutils.filter_files_by_prefix(self.config, to_delete)
+                logging.info(_('%d files remaining from previous build') % (len(to_delete),))
+                for (path, was_deleted, error_string) in fileutils.remove_files_and_dirs(to_delete, allow_nonempty_dirs=True):
+                    if was_deleted:
+                        logging.info(_('Deleted: %(file)r') % { 'file': path, })
+                    elif error_string is None:
+                        # We don't warn on not-empty directories
+                        pass
+                    else:
+                        logging.warn(_("Failed to delete no longer installed file %(file)r: %(msg)s") % { 'file': path,
+                                                                                                          'msg': error_string})
+
+            buildscript.moduleset.packagedb.add(self.name, revision or '', absolute_new_contents)
 
     def get_revision(self):
         return self.branch.tree_id()
diff --git a/jhbuild/utils/Makefile.am b/jhbuild/utils/Makefile.am
index b411d3b..81f2513 100644
--- a/jhbuild/utils/Makefile.am
+++ b/jhbuild/utils/Makefile.am
@@ -3,6 +3,7 @@ appdir = $(pythondir)/jhbuild/utils/
 app_PYTHON = \
 	__init__.py \
 	cmds.py \
+	fileutils.py \
 	httpcache.py \
 	lockfile.py \
 	notify.py \
diff --git a/jhbuild/utils/fileutils.py b/jhbuild/utils/fileutils.py
new file mode 100644
index 0000000..b78e230
--- /dev/null
+++ b/jhbuild/utils/fileutils.py
@@ -0,0 +1,91 @@
+# jhbuild - a build script for GNOME 1.x and 2.x
+# Copyright (C) 2011 Red Hat, Inc.
+#
+#   fileutils.py - Helper functions for filesystem operations
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+#
+# Author: Colin Walters <walters verbum org>
+
+import os
+import sys
+
+def _accumulate_dirtree_contents_recurse(path, contents):
+    names = os.listdir(path)
+    for name in names:
+        subpath = os.path.join(path, name)
+        if os.path.isdir(subpath):
+            previous_len = len(contents)
+            _accumulate_dirtree_contents_recurse(subpath, contents)
+            new_len = len(contents)
+            # Only add if the directory is empty, otherwise, its existence
+            # is implicit.
+            if previous_len == new_len:
+                contents.append(subpath + '/')
+        else:
+            contents.append(subpath)
+
+def accumulate_dirtree_contents(path):
+    """Return a list of files and empty directories in the directory at PATH.  Each item
+in the returned list is relative to the root path."""
+    contents = []
+    _accumulate_dirtree_contents_recurse(path, contents)
+    if not path.endswith('/'):
+        path = path + '/'
+    pathlen = len(path)
+    for i,subpath in enumerate(contents):
+        assert subpath.startswith(path)
+        contents[i] = subpath[pathlen:]
+    return contents
+
+def remove_files_and_dirs(file_paths, allow_nonempty_dirs=False):
+    """Given a list of file paths in any order, attempt to delete
+them.  The main intelligence in this function is removing files
+in a directory before removing the directory.
+
+Returns a list, where each item is a 2-tuple:
+(path, error_string or None)"""
+
+    results = []
+
+    for path in reversed(sorted(file_paths)):
+        isdir = os.path.isdir(path)
+        try:
+            if isdir:
+                os.rmdir(path)
+            else:
+                os.unlink(path)
+            results.append((path, True, ''))
+        except OSError, e:
+            if (isdir
+                and allow_nonempty_dirs
+                and len(os.listdir(path)) > 0):
+                results.append((path, False, None))
+            else:
+                results.append((path, False, e.strerror))
+    return results
+
+def filter_files_by_prefix(config, file_paths):
+    """Return the set of files in file_paths that are inside the prefix."""
+    canon_prefix = config.prefix
+    if not canon_prefix.endswith('/'):
+        canon_prefix = canon_prefix + '/'
+    result = []
+    for path in file_paths:
+        if path == canon_prefix or (not path.startswith(canon_prefix)):
+            continue
+        result.append(path)
+    return result
+
diff --git a/jhbuild/utils/packagedb.py b/jhbuild/utils/packagedb.py
index f291c3c..3d72334 100644
--- a/jhbuild/utils/packagedb.py
+++ b/jhbuild/utils/packagedb.py
@@ -33,6 +33,8 @@ except ImportError:
 
 from StringIO import StringIO
 
+from . import fileutils
+
 def _parse_isotime(string):
     if string[-1] != 'Z':
         return time.mktime(time.strptime(string, '%Y-%m-%dT%H:%M:%S'))
@@ -198,29 +200,6 @@ class PackageDB:
         # Ensure we don't reread what we already have cached
         self._entries_stat = os.stat(self.dbfile)
 
-    def _accumulate_dirtree_contents_recurse(self, path, contents):
-        assert os.path.isdir(path)
-        names = os.listdir(path)
-        for name in names:
-            subpath = os.path.join(path, name)
-            if os.path.isdir(subpath):
-                contents.append(subpath + '/')
-                self._accumulate_dirtree_contents_recurse(subpath, contents)
-            else:
-                contents.append(subpath)
-
-    def _accumulate_dirtree_contents(self, path):
-        contents = []
-        self._accumulate_dirtree_contents_recurse(path, contents)
-        if not path.endswith('/'):
-            path = path + '/'
-        pathlen = len(path)
-        for i,subpath in enumerate(contents):
-            assert subpath.startswith(path)
-            # Strip the temporary prefix, then make it absolute again for our target
-            contents[i] = '/' + subpath[pathlen:]
-        return contents
-
     def _locked(function):
         def decorate(self, *args, **kwargs):
             self._lock.lock()
@@ -237,12 +216,12 @@ class PackageDB:
 
     @_ensure_cache
     @_locked
-    def add(self, package, version, destdir):
+    def add(self, package, version, contents):
         '''Add a module to the install cache.'''
         now = time.time()
         metadata = {'installed-date': now}
         self._entries[package] = PackageEntry(package, version, metadata, self.manifests_dir)
-        self._entries[package].manifest = self._accumulate_dirtree_contents(destdir)
+        self._entries[package].manifest = contents
         self._write_cache()
 
     def check(self, package, version=None):
@@ -273,31 +252,23 @@ class PackageDB:
             logging.error(_("no manifest for '%s', can't uninstall.  Try building again, then uninstalling.") % (package_name,))
             return
 
-        directories = []
-        for path in entry.manifest:
-            assert os.path.isabs(path)
-            if os.path.isdir(path):
-                directories.append(path)
-            else:
-                try:
-                    os.unlink(path)
-                    logging.info(_("Deleted: %s" % (path, )))
-                except OSError, e:
-                    logging.warn(_("Failed to delete %(file)r: %(msg)s") % { 'file': path,
-                                                                             'msg': e.strerror})
-                        
-        for directory in directories:
-            if not directory.startswith(self.config.prefix):
-                # Skip non-prefix directories; otherwise we
-                # may try to remove the user's ~ or something
-                # (presumably we'd fail, but better not to try)
-                continue
-            try:
-                os.rmdir(directory)
-                logging.info(_("Deleted: %s" % (directory, )))
-            except OSError, e:
-                # Allow multiple components to use directories
+        # Skip files that aren't in the prefix; otherwise we
+        # may try to remove the user's ~ or something
+        # (presumably we'd fail, but better not to try)
+        to_delete = fileutils.filter_files_by_prefix(self.config, entry.manifest)
+
+        # Don't warn on non-empty directories; we want to allow multiple
+        # modules to share the same directory.  We could improve this by
+        # reference-counting directories.
+        for (path, was_deleted, error_string) in fileutils.remove_files_and_dirs(to_delete, allow_nonempty_dirs=True):
+            if was_deleted:
+                logging.info(_("Deleted: %(file)r") % {'file': path})
+            elif error_string is None:
                 pass
+            else:
+                logging.warn(_("Failed to delete %(file)r: %(msg)s") % { 'file': path,
+                                                                         'msg': error_string})
+
         del self._entries[package_name]
         self._write_cache()