Merge lp:~gary/launchpad/bug562828 into lp:launchpad

[Gary Poster]

Switch lib/canonical/librarian/librariangc.py to using a copy of Python 2.6's os.walk in order to support symlinks in our librarian file directory.

This switches lib/canonical/librarian/librariangc.py to using a copy of Python 2.6's os.walk per the discussion of bug 562828, in order to support symlinks in our librarian file directory.

[Gary Poster (gary)]

Note that _walk in the diff is an almost-pure copy and paste from Python 2.6's os.walk. The only difference is that listdir became os.listdir.

[Gary Poster (gary)]

Note that I initialized content_id because it was failing when I did a "does my test fail without my fix" test. I don't know if it is a "real" problem but it seemed reasonable to do.

[Aaron Bentley (abentley)]

review: Approve

=== modified file 'lib/canonical/librarian/ftests/test_gc.py'
--- lib/canonical/librarian/ftests/test_gc.py	2010-03-16 10:08:38 +0000
+++ lib/canonical/librarian/ftests/test_gc.py	2010-04-23 22:13:24 +0000
@@ -7,6 +7,7 @@
 
 import shutil
 import sys
+import tempfile
 import os
 from subprocess import Popen, PIPE, STDOUT
 from cStringIO import StringIO
@@ -559,6 +560,36 @@
         # This should cope.
         librariangc.delete_unwanted_files(self.con)
 
+    def test_delete_unwanted_files_follows_symlinks(self):
+        # In production, our tree has symlinks in it now.  We need to be able
+        # to cope.
+        # First, let's make sure we have some trash.
+        self.layer.switchDbUser(dbuser='testadmin')
+        content = 'foo'
+        self.client.addFile(
+            'foo.txt', len(content), StringIO(content), 'text/plain')
+        # Roll back the database changes, leaving the file on disk.
+        transaction.abort()
+
+        self.layer.switchDbUser(config.librarian_gc.dbuser)
+
+        # Now, we will move the directory containing the trash somewhere else
+        # and make a symlink to it.
+        original = os.path.join(config.librarian_server.root, '00', '00')
+        newdir = tempfile.mkdtemp()
+        alt = os.path.join(newdir, '00')
+        shutil.move(original, alt)
+        os.symlink(alt, original)
+
+        # Now we will do our thing.  This is the actual test.  It used to
+        # fail.
+        librariangc.delete_unwanted_files(self.con)
+
+        # Clean up.
+        os.remove(original)
+        shutil.move(alt, original)
+        shutil.rmtree(newdir)
+
     def test_cronscript(self):
         script_path = os.path.join(
                 config.root, 'cronscripts', 'librarian-gc.py'

=== modified file 'lib/canonical/librarian/librariangc.py'
--- lib/canonical/librarian/librariangc.py	2010-03-16 10:08:38 +0000
+++ lib/canonical/librarian/librariangc.py	2010-04-23 22:13:24 +0000
@@ -495,6 +495,99 @@
     loop_tuner = DBLoopTuner(UnreferencedContentPruner(con), 5, log=log)
     loop_tuner.run()
 
+# XXX gary 2010-04-22 bug=569217
+# We should remove this and use Python 2.6's os.walk once we switch to
+# Python 2.6.
+def _walk(top, topdown=True, onerror=None, followlinks=False):
+    """Directory tree generator.
+
+    For each directory in the directory tree rooted at top (including top
+    itself, but excluding '.' and '..'), yields a 3-tuple
+
+        dirpath, dirnames, filenames
+
+    dirpath is a string, the path to the directory.  dirnames is a list of
+    the names of the subdirectories in dirpath (excluding '.' and '..').
+    filenames is a list of the names of the non-directory files in dirpath.
+    Note that the names in the lists are just names, with no path components.
+    To get a full path (which begins with top) to a file or directory in
+    dirpath, do os.path.join(dirpath, name).
+
+    If optional arg 'topdown' is true or not specified, the triple for a
+    directory is generated before the triples for any of its subdirectories
+    (directories are generated top down).  If topdown is false, the triple
+    for a directory is generated after the triples for all of its
+    subdirectories (directories are generated bottom up).
+
+    When topdown is true, the caller can modify the dirnames list in-place
+    (e.g., via del or slice assignment), and walk will only recurse into the
+    subdirectories whose names remain in dirnames; this can be used to prune
+    the search, or to impose a specific order of visiting.  Modifying
+    dirnames when topdown is false is ineffective, since the directories in
+    dirnames have already been generated by the time dirnames itself is
+    generated.
+
+    By default errors from the os.listdir() call are ignored.  If
+    optional arg 'onerror' is specified, it should be a function; it
+    will be called with one argument, an os.error instance.  It can
+    report the error to continue with the walk, or raise the exception
+    to abort the walk.  Note that the filename is available as the
+    filename attribute of the exception object.
+
+    By default, os.walk does not follow symbolic links to subdirectories on
+    systems that support them.  In order to get this functionality, set the
+    optional argument 'followlinks' to true.
+
+    Caution:  if you pass a relative pathname for top, don't change the
+    current working directory between resumptions of walk.  walk never
+    changes the current directory, and assumes that the client doesn't
+    either.
+
+    Example:
+
+    import os
+    from os.path import join, getsize
+    for root, dirs, files in os.walk('python/Lib/email'):
+        print root, "consumes",
+        print sum([getsize(join(root, name)) for name in files]),
+        print "bytes in", len(files), "non-directory files"
+        if 'CVS' in dirs:
+            dirs.remove('CVS')  # don't visit CVS directories
+    """
+
+    from os.path import join, isdir, islink
+
+    # We may not have read permission for top, in which case we can't
+    # get a list of the files the directory contains.  os.path.walk
+    # always suppressed the exception then, rather than blow up for a
+    # minor reason when (say) a thousand readable directories are still
+    # left to visit.  That logic is copied here.
+    try:
+        # Note that listdir and error are globals in this module due
+        # to earlier import-*.
+        names = os.listdir(top)
+    except error, err:
+        if onerror is not None:
+            onerror(err)
+        return
+
+    dirs, nondirs = [], []
+    for name in names:
+        if isdir(join(top, name)):
+            dirs.append(name)
+        else:
+            nondirs.append(name)
+
+    if topdown:
+        yield top, dirs, nondirs
+    for name in dirs:
+        path = join(top, name)
+        if followlinks or not islink(path):
+            for x in _walk(path, topdown, onerror, followlinks):
+                yield x
+    if not topdown:
+        yield top, dirs, nondirs
+
 
 def delete_unwanted_files(con):
     """Delete files found on disk that have no corresponding record in the
@@ -524,12 +617,15 @@
             return result[0]
 
     removed_count = 0
-    next_wanted_content_id = -1
+    content_id = next_wanted_content_id = -1
 
     hex_content_id_re = re.compile('^[0-9a-f]{8}$')
     ONE_DAY = 24 * 60 * 60
 
-    for dirpath, dirnames, filenames in os.walk(get_storage_root()):
+    # XXX gary 2010-04-22 bug=569217
+    # We should switch back to os.walk once we switch to Python 2.6.
+    for dirpath, dirnames, filenames in _walk(
+        get_storage_root(), followlinks=True):
 
         # Ignore known and harmless noise in the Librarian storage area.
         if 'incoming' in dirnames: