Merge changes to fix bugs removing stuff from new generation.

6 files changed, 106 insertions, 23 deletions

diff --git a/blackboxtest b/blackboxtest
index c397b47c..431f99c3 100755
--- a/blackboxtest
+++ b/blackboxtest
@@ -102,6 +102,7 @@ class ObnamTestCase(unittest.TestCase):
         return self.runcmd(['./obnam', 
                             '--quiet',
                             '--log', 'blackboxtest-obnam.log',
+                            '--log-level', 'debug',
                             '--client-name', self.client_name] +
                            args, stderr_ignore=stderr_ignore)
 
@@ -454,7 +455,7 @@ class ForgetTests(ObnamTestCase):
                 st = fs.lstat(os.path.join(dirname, filename))
                 usage += st.st_blocks * 512
         return usage
-        
+    
     def random_string(self, size):
         return ''.join(chr(random.randint(0, 255)) for i in xrange(size))
     
@@ -464,6 +465,18 @@ class ForgetTests(ObnamTestCase):
         self.forget(genids=self.generations())
         self.assertEqual(self.disk_usage(os.path.join(self.store, 'chunks')),
                          0)
+    
+    def test_removes_unwanted_data_with_empty_generation_remaining(self):
+        self.create_file(self.data, 'big', self.random_string(1024**2))
+        self.backup()
+        shutil.rmtree(self.data)
+        os.mkdir(self.data)
+        self.backup()
+        genids = self.generations()
+        forgettable = genids[:-1]
+        self.forget(genids=forgettable)
+        chunks = os.path.join(self.store, 'chunks')
+        self.assertEqual(self.disk_usage(chunks), 0)
 
 
 if __name__ == '__main__':
diff --git a/obnamlib/app.py b/obnamlib/app.py
index 1815add2..d420c2e1 100644
--- a/obnamlib/app.py
+++ b/obnamlib/app.py
@@ -108,6 +108,7 @@ class App(object):
         self.setup_logging()
         logging.info('Obnam %s starts' % obnamlib.version)
         if self.config.args:
+            logging.info('Executing command: %s' % self.config.args[0])
             self.interp.execute(self.config.args[0], self.config.args[1:])
         else:
             raise obnamlib.AppException('Usage error: '
diff --git a/obnamlib/clientmetadatatree.py b/obnamlib/clientmetadatatree.py
index d6303a6e..087d80fe 100644
--- a/obnamlib/clientmetadatatree.py
+++ b/obnamlib/clientmetadatatree.py
@@ -215,11 +215,10 @@ class ClientMetadataTree(obnamlib.StoreTree):
                 self._lookup_time(tree, self.GEN_ENDED))
 
     def create(self, filename, encoded_metadata):
-        namehash = self.hash_name(filename)
         file_id = self.get_file_id(self.curgen, filename)
-        key = self.fskey(namehash, self.FILE_NAME, file_id)
+        gen_id = self.get_generation_id(self.curgen)
         try:
-            old_metadata = self.curgen.lookup(key)
+            old_metadata = self.get_metadata(gen_id, filename)
         except KeyError:
             old_metadata = None
         if encoded_metadata != old_metadata:
@@ -246,9 +245,8 @@ class ClientMetadataTree(obnamlib.StoreTree):
         return tree.lookup(key)
 
     def set_metadata(self, filename, encoded_metadata):
-        namehash = self.hash_name(filename)
         file_id = self.get_file_id(self.curgen, filename)
-        key1 = self.fskey(namehash, self.FILE_NAME, file_id)
+        key1 = self.fskey(file_id, self.FILE_NAME, file_id)
         self.curgen.insert(key1, filename)
         
         key2 = self.fskey(file_id, self.FILE_METADATA, 
@@ -259,11 +257,18 @@ class ClientMetadataTree(obnamlib.StoreTree):
         file_id = self.get_file_id(self.curgen, filename)
         genid = self.get_generation_id(self.curgen)
 
+        # Remove any children.
+        minkey = self.fskey(file_id, self.DIR_CONTENTS, 0)
+        maxkey = self.fskey(file_id, self.DIR_CONTENTS, obnamlib.MAX_ID)
+        for key, basename in self.curgen.lookup_range(minkey, maxkey):
+            self.remove(os.path.join(filename, basename))
+
         # Remove chunk refs.
         for chunkid in self.get_file_chunks(genid, filename):
             key = self.chunk_key(chunkid, file_id)
             self.curgen.remove_range(key, key)
-
+            
+        # Remove this file's metadata.
         minkey = self.fskey(file_id, 0, 0)
         maxkey = self.fskey(file_id, self.TYPE_MAX, self.SUBKEY_MAX)
         self.curgen.remove_range(minkey, maxkey)
diff --git a/obnamlib/clientmetadatatree_tests.py b/obnamlib/clientmetadatatree_tests.py
index ca95e6be..11ec2612 100644
--- a/obnamlib/clientmetadatatree_tests.py
+++ b/obnamlib/clientmetadatatree_tests.py
@@ -156,6 +156,8 @@ class ClientMetadataTreeFileOpsTests(unittest.TestCase):
         self.clientid = self.client.get_generation_id(self.client.curgen)
         self.file_metadata = obnamlib.Metadata(st_mode=stat.S_IFREG | 0666)
         self.file_encoded = obnamlib.store.encode_metadata(self.file_metadata)
+        self.dir_metadata = obnamlib.Metadata(st_mode=stat.S_IFDIR | 0777)
+        self.dir_encoded = obnamlib.store.encode_metadata(self.dir_metadata)
         
     def tearDown(self):
         shutil.rmtree(self.tempdir)
@@ -174,11 +176,64 @@ class ClientMetadataTreeFileOpsTests(unittest.TestCase):
     def test_creates_file_at_root(self):
         self.client.create('/foo', self.file_encoded)
         self.assertEqual(self.client.listdir(self.clientid, '/'), ['foo'])
+        self.assertEqual(self.client.get_metadata(self.clientid, '/foo'),
+                         self.file_encoded)
 
     def test_removes_file_at_root(self):
         self.client.create('/foo', self.file_encoded)
         self.client.remove('/foo')
         self.assertEqual(self.client.listdir(self.clientid, '/'), [])
+        self.assertRaises(KeyError, self.client.get_metadata, 
+                          self.clientid, '/foo')
+
+    def test_creates_directory_at_root(self):
+        self.client.create('/foo', self.dir_encoded)
+        self.assertEqual(self.client.listdir(self.clientid, '/'), ['foo'])
+        self.assertEqual(self.client.get_metadata(self.clientid, '/foo'), 
+                         self.dir_encoded)
+
+    def test_removes_directory_at_root(self):
+        self.client.create('/foo', self.dir_encoded)
+        self.client.remove('/foo')
+        self.assertEqual(self.client.listdir(self.clientid, '/'), [])
+        self.assertRaises(KeyError, self.client.get_metadata, 
+                          self.clientid, '/foo')
+
+    def test_creates_directory_and_files_and_subdirs(self):
+        self.client.create('/foo', self.dir_encoded)
+        self.client.create('/foo/foobar', self.file_encoded)
+        self.client.create('/foo/bar', self.dir_encoded)
+        self.client.create('/foo/bar/baz', self.file_encoded)
+        self.assertEqual(self.client.listdir(self.clientid, '/'), ['foo'])
+        self.assertEqual(sorted(self.client.listdir(self.clientid, '/foo')), 
+                         ['bar', 'foobar'])
+        self.assertEqual(self.client.listdir(self.clientid, '/foo/bar'), 
+                         ['baz'])
+        self.assertEqual(self.client.get_metadata(self.clientid, '/foo'), 
+                         self.dir_encoded)
+        self.assertEqual(self.client.get_metadata(self.clientid, '/foo/bar'), 
+                         self.dir_encoded)
+        self.assertEqual(self.client.get_metadata(self.clientid, '/foo/foobar'), 
+                         self.file_encoded)
+        self.assertEqual(self.client.get_metadata(self.clientid, 
+                                                  '/foo/bar/baz'), 
+                         self.file_encoded)
+
+    def test_removes_directory_and_files_and_subdirs(self):
+        self.client.create('/foo', self.dir_encoded)
+        self.client.create('/foo/foobar', self.file_encoded)
+        self.client.create('/foo/bar', self.dir_encoded)
+        self.client.create('/foo/bar/baz', self.file_encoded)
+        self.client.remove('/foo')
+        self.assertEqual(self.client.listdir(self.clientid, '/'), [])
+        self.assertRaises(KeyError, self.client.get_metadata, 
+                          self.clientid, '/foo')
+        self.assertRaises(KeyError, self.client.get_metadata, 
+                          self.clientid, '/foo/foobar')
+        self.assertRaises(KeyError, self.client.get_metadata, 
+                          self.clientid, '/foo/bar')
+        self.assertRaises(KeyError, self.client.get_metadata, 
+                          self.clientid, '/foo/bar/baz')
 
     def test_has_no_file_chunks_initially(self):
         self.assertEqual(self.client.get_file_chunks(self.clientid, '/foo'), [])
diff --git a/obnamlib/plugins/backup_plugin.py b/obnamlib/plugins/backup_plugin.py
index 7072fd19..a30a5244 100644
--- a/obnamlib/plugins/backup_plugin.py
+++ b/obnamlib/plugins/backup_plugin.py
@@ -90,9 +90,9 @@ class BackupPlugin(obnamlib.ObnamPlugin):
             logging.debug('absolute roots: %s' % absroots)
             self.remove_old_roots(absroots)
 
-            for root in roots:
-                self.fs.reinit(root)
-                absroot = self.fs.abspath('.')
+            for absroot in absroots:
+                logging.debug('Backing up root %s' % absroot)
+                self.fs.reinit(absroot)
                 for pathname, metadata in self.find_files(absroot):
                     logging.debug('backing up %s' % pathname)
                     try:
@@ -146,7 +146,9 @@ class BackupPlugin(obnamlib.ObnamPlugin):
                     yield pathname, metadata
                     needed = True
             metadata = obnamlib.read_metadata(self.fs, dirname)
-            if needed or self.needs_backup(dirname, metadata):
+            if not needed:
+                needed = self.needs_backup(dirname, metadata)
+            if needed:
                 yield dirname, metadata
 
     def prune(self, dirname, subdirs, filenames):
@@ -169,6 +171,10 @@ class BackupPlugin(obnamlib.ObnamPlugin):
     def needs_backup(self, pathname, current):
         '''Does a given file need to be backed up?'''
         
+        # Directories always require backing up so that backup_dir_contents
+        # can remove stuff that no longer exists from them.
+        if current.isdir():
+            return True
         try:
             old = self.store.get_metadata(self.store.new_generation, pathname)
         except obnamlib.Error:
@@ -239,8 +245,9 @@ class BackupPlugin(obnamlib.ObnamPlugin):
             old_basenames = []
 
         for old in old_basenames:
+            pathname = os.path.join(root, old)
             if old not in new_basenames:
-                self.store.remove(os.path.join(root, old))
+                self.store.remove(pathname)
         # Files that are created after the previous generation will be
         # added to the directory when they are backed up, so we don't
         # need to worry about them here.
@@ -257,30 +264,22 @@ class BackupPlugin(obnamlib.ObnamPlugin):
         
         def is_parent(pathname):
             x = pathname + os.sep
-            logging.debug('is_parent: x is %s' % x)
             for new_root in new_roots:
                 if new_root.startswith(x):
-                    logging.debug('is_parent: starts with x: %s' % new_root)
                     return True
-            logging.debug('is_parent: is not %s' % pathname)
             return False
 
         def helper(dirname):
-            logging.debug('helper: %s' % dirname)
             gen_id = self.store.new_generation
             basenames = self.store.listdir(gen_id, dirname)
             for basename in basenames:
                 pathname = os.path.join(dirname, basename)
                 if is_parent(pathname):
-                    logging.debug('helper: is parent: %s' % pathname)
                     metadata = self.store.get_metadata(gen_id, pathname)
                     if metadata.isdir():
                         helper(pathname)
                 elif pathname not in new_roots:
-                    logging.debug('helper: removing %s' % pathname)
                     self.store.remove(pathname)
-                else:
-                    logging.debug('helper: keeping %s' % pathname)
 
         helper('/')
 
diff --git a/obnamlib/store.py b/obnamlib/store.py
index 72010ff8..93ae867e 100644
--- a/obnamlib/store.py
+++ b/obnamlib/store.py
@@ -17,6 +17,7 @@
 import btree
 import errno
 import hashlib
+import logging
 import os
 import random
 import struct
@@ -381,17 +382,26 @@ class Store(object):
         
         '''
 
+        logging.debug('_really_remove_generation: %d' % gen_id)
         chunk_ids = self.client.list_chunks_in_generation(gen_id)
+        logging.debug('_r_r_g: chunk ids in gen: %s' % chunk_ids)
+        for chunk_id in chunk_ids:
+            logging.debug('_r_r_g: removing chunk %d from client' % chunk_id)
+            checksum = self.chunklist.get_checksum(chunk_id)
+            self.chunksums.remove(checksum, chunk_id, self.current_client_id)
         for other_id in self.list_generations():
             if other_id != gen_id:
+                logging.debug('_r_r_g: other_id is %d' % other_id)
+                other_chunks = self.client.list_chunks_in_generation(other_id)
+                logging.debug('_r_r_g: other_chunks: %s' % other_chunks)
                 chunk_ids = [chunk_id
                              for chunk_id in chunk_ids
-                             if not self.client.chunk_in_use(other_id, 
-                                                             chunk_id)]
+                             if chunk_id not in other_chunks]
+        logging.debug('_r_r_g: chunk ids only in gen: %s' % chunk_ids)
         for chunk_id in chunk_ids:
             checksum = self.chunklist.get_checksum(chunk_id)
-            self.chunksums.remove(checksum, chunk_id, self.current_client_id)
             if not self.chunksums.chunk_is_used(checksum, chunk_id):
+                logging.debug('_r_r_g: removing chunk %d' % chunk_id)
                 self.remove_chunk(chunk_id)
         self.client.remove_generation(gen_id)