From dab94e1f7009fce04f87c15df6e9e7da99c63ada Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 3 Dec 2015 15:02:50 +0900 Subject: [PATCH] Bug 1230060 - Allow to restrict what unaccounted files to remove when copying from a FileCopier. r=gps The default behavior for a FileCopier's copy is to remove all the files and directories in the destination that aren't in its registry. The remove_unaccounted argument can be passed as False to disable this behavior. This change adds another possibility, where remove_unaccounted may be a FileRegistry, in which case only the files in that registry are removed. This allows to e.g. only remove files that were copied from a previous FileCopier.copy, leaving aside files that were in the destination for some other reason. --- python/mozbuild/mozpack/copier.py | 92 +++++++++++++-------- python/mozbuild/mozpack/test/test_copier.py | 44 ++++++++++ 2 files changed, 102 insertions(+), 34 deletions(-) diff --git a/python/mozbuild/mozpack/copier.py b/python/mozbuild/mozpack/copier.py index 73439c08f812..5ad9d31c99ae 100644 --- a/python/mozbuild/mozpack/copier.py +++ b/python/mozbuild/mozpack/copier.py @@ -331,41 +331,50 @@ class FileCopier(FileRegistry): os.umask(umask) os.chmod(d, 0777 & ~umask) - # While we have remove_unaccounted, it doesn't apply to empty - # directories because it wouldn't make sense: an empty directory - # is empty, so removing it should have no effect. - existing_dirs = set() - existing_files = set() - for root, dirs, files in os.walk(destination): - # We need to perform the same symlink detection as above. os.walk() - # doesn't follow symlinks into directories by default, so we need - # to check dirs (we can't wait for root). - if have_symlinks: - filtered = [] - for d in dirs: - full = os.path.join(root, d) - st = os.lstat(full) - if stat.S_ISLNK(st.st_mode): - # This directory symlink is not a required - # directory: any such symlink would have been - # removed and a directory created above. - if remove_all_directory_symlinks: - os.remove(full) - result.removed_files.add(os.path.normpath(full)) + if isinstance(remove_unaccounted, FileRegistry): + existing_files = set(os.path.normpath(os.path.join(destination, p)) + for p in remove_unaccounted.paths()) + existing_dirs = set(os.path.normpath(os.path.join(destination, p)) + for p in remove_unaccounted + .required_directories()) + existing_dirs |= {os.path.normpath(destination)} + else: + # While we have remove_unaccounted, it doesn't apply to empty + # directories because it wouldn't make sense: an empty directory + # is empty, so removing it should have no effect. + existing_dirs = set() + existing_files = set() + for root, dirs, files in os.walk(destination): + # We need to perform the same symlink detection as above. + # os.walk() doesn't follow symlinks into directories by + # default, so we need to check dirs (we can't wait for root). + if have_symlinks: + filtered = [] + for d in dirs: + full = os.path.join(root, d) + st = os.lstat(full) + if stat.S_ISLNK(st.st_mode): + # This directory symlink is not a required + # directory: any such symlink would have been + # removed and a directory created above. + if remove_all_directory_symlinks: + os.remove(full) + result.removed_files.add( + os.path.normpath(full)) + else: + existing_files.add(os.path.normpath(full)) else: - existing_files.add(os.path.normpath(full)) - else: - filtered.append(d) + filtered.append(d) - dirs[:] = filtered + dirs[:] = filtered - existing_dirs.add(os.path.normpath(root)) + existing_dirs.add(os.path.normpath(root)) - for d in dirs: - existing_dirs.add(os.path.normpath(os.path.join(root, d))) + for d in dirs: + existing_dirs.add(os.path.normpath(os.path.join(root, d))) - for f in files: - existing_files.add(os.path.normpath(os.path.join(root, f))) + for f in files: + existing_files.add(os.path.normpath(os.path.join(root, f))) # Now we reconcile the state of the world against what we want. @@ -420,10 +429,25 @@ class FileCopier(FileRegistry): # Remove empty directories that aren't required. for d in sorted(remove_dirs, key=len, reverse=True): - # Permissions may not allow deletion. So ensure write access is - # in place before attempting delete. - os.chmod(d, 0700) - os.rmdir(d) + try: + try: + os.rmdir(d) + except OSError as e: + if e.errno in (errno.EPERM, errno.EACCES): + # Permissions may not allow deletion. So ensure write + # access is in place before attempting to rmdir again. + os.chmod(d, 0700) + os.rmdir(d) + else: + raise + except OSError as e: + # If remove_unaccounted is a # FileRegistry, then we have a + # list of directories that may not be empty, so ignore rmdir + # ENOTEMPTY errors for them. + if (isinstance(remove_unaccounted, FileRegistry) and + e.errno == errno.ENOTEMPTY): + continue + raise result.removed_directories.add(d) return result diff --git a/python/mozbuild/mozpack/test/test_copier.py b/python/mozbuild/mozpack/test/test_copier.py index 53caf09d1832..7c3b80f2cdc7 100644 --- a/python/mozbuild/mozpack/test/test_copier.py +++ b/python/mozbuild/mozpack/test/test_copier.py @@ -421,6 +421,50 @@ class TestFileCopier(TestWithTmpDir): # existing when it does not. self.assertIn(self.tmppath('dest/foo/bar'), result.existing_files) + def test_remove_unaccounted_file_registry(self): + """Test FileCopier.copy(remove_unaccounted=FileRegistry())""" + + dest = self.tmppath('dest') + + copier = FileCopier() + copier.add('foo/bar/baz', GeneratedFile('foobarbaz')) + copier.add('foo/bar/qux', GeneratedFile('foobarqux')) + copier.add('foo/hoge/fuga', GeneratedFile('foohogefuga')) + copier.add('foo/toto/tata', GeneratedFile('footototata')) + + os.makedirs(os.path.join(dest, 'bar')) + with open(os.path.join(dest, 'bar', 'bar'), 'w') as fh: + fh.write('barbar'); + os.makedirs(os.path.join(dest, 'foo', 'toto')) + with open(os.path.join(dest, 'foo', 'toto', 'toto'), 'w') as fh: + fh.write('foototototo'); + + result = copier.copy(dest, remove_unaccounted=False) + + self.assertEqual(self.all_files(dest), + set(copier.paths()) | { 'foo/toto/toto', 'bar/bar'}) + self.assertEqual(self.all_dirs(dest), + {'foo/bar', 'foo/hoge', 'foo/toto', 'bar'}) + + copier2 = FileCopier() + copier2.add('foo/hoge/fuga', GeneratedFile('foohogefuga')) + + # We expect only files copied from the first copier to be removed, + # not the extra file that was there beforehand. + result = copier2.copy(dest, remove_unaccounted=copier) + + self.assertEqual(self.all_files(dest), + set(copier2.paths()) | { 'foo/toto/toto', 'bar/bar'}) + self.assertEqual(self.all_dirs(dest), + {'foo/hoge', 'foo/toto', 'bar'}) + self.assertEqual(result.updated_files, + {self.tmppath('dest/foo/hoge/fuga')}) + self.assertEqual(result.existing_files, set()) + self.assertEqual(result.removed_files, {self.tmppath(p) for p in + ('dest/foo/bar/baz', 'dest/foo/bar/qux', 'dest/foo/toto/tata')}) + self.assertEqual(result.removed_directories, + {self.tmppath('dest/foo/bar')}) + class TestFilePurger(TestWithTmpDir): def test_file_purger(self):