# HG changeset patch # User brett # Date 1163288539 18000 # Node ID 1f3cb3845dfd77cce92fd85ae935d29f71ca527b # Parent 77043f4e6a9fb25d99c6c6367bdcf475d3b8ba08 [svn] Add a test for recursive extraction which also makes sure that we fix permissions after we extract the archive, and DTRT when an archive contains one file. Add code to handle the latter two cases. ExtractorApplication is a total mess at this point. I already am having a hard time following how the pieces fit together. Cleaning it up is my next task; that'll be easier now that I test most of the functionality again. diff -r 77043f4e6a9f -r 1f3cb3845dfd TODO --- a/TODO Mon Nov 06 22:36:47 2006 -0500 +++ b/TODO Sat Nov 11 18:42:19 2006 -0500 @@ -1,10 +1,9 @@ Things which I have a use case/anti-use case for: +* Clean up ExtractorApplication, ugh. +* Reconsider the current extraction policy for the ONE_DIRECTORY case. * Decompress non-archive files. -* DTRT in the "archive contains one file" case. -* Fix permissions as you extract. Things that are generally good: -* Usage information. * Better tests. * Better error messages. diff -r 77043f4e6a9f -r 1f3cb3845dfd scripts/x --- a/scripts/x Mon Nov 06 22:36:47 2006 -0500 +++ b/scripts/x Sat Nov 11 18:42:19 2006 -0500 @@ -312,10 +312,18 @@ self.show_error("all good names for an extraction directory taken") return directory + def move_to_directory(self, filename, target): + if not os.path.isdir(filename): + filename = os.path.split(filename)[0] + target = os.path.join(target, filename) + os.rename(filename, target) + def prepare_extraction(self): self.current_path = '.' contents = self.current_extractor.check_contents() - if contents not in (MATCHING_DIRECTORY, EMPTY): + if contents == MATCHING_DIRECTORY: + self.target_directory = self.current_filename + elif contents != EMPTY: self.target_directory = self.prepare_target_directory() if self.target_directory is None: return False @@ -323,10 +331,10 @@ os.chdir(self.target_directory) self.current_path = '..' else: - self.cleanup_actions.append((os.rename, contents, + self.cleanup_actions.append((self.move_to_directory, contents, self.target_directory)) else: - self.target_directory = os.curdir + self.target_directory = None return True def extract(self): @@ -340,15 +348,27 @@ def recurse(self): if not self.options.recursive: return True - print "wow", self.current_extractor.included_archives for filename in self.current_extractor.included_archives: tail_path, basename = os.path.split(filename) directory = os.path.join(self.current_directory, self.target_directory, tail_path) self.archives.setdefault(directory, []).append(basename) - print self.archives return True + def fix_perms(self): + if self.target_directory is None: + return True + status = subprocess.call(['chmod', '-R', 'u+rw', + os.path.join(self.current_directory, + self.target_directory)]) + if status == 0: + status = subprocess.call(['find', + os.path.join(self.current_directory, + self.target_directory), + '-type', 'd', + '-exec', 'chmod', 'u+x', '{}', ';']) + return status == 0 + def run(self): while self.archives: self.current_directory, filenames = self.archives.popitem() @@ -362,6 +382,7 @@ running = actions.pop(0)() for action in self.cleanup_actions: action[0](*action[1:]) + running = self.fix_perms() if running: self.successes.append(self.current_filename) else: diff -r 77043f4e6a9f -r 1f3cb3845dfd tests/compare.py --- a/tests/compare.py Mon Nov 06 22:36:47 2006 -0500 +++ b/tests/compare.py Sat Nov 11 18:42:19 2006 -0500 @@ -28,23 +28,34 @@ set -e """ -tests = {'test-1.23.tar': ['tar -xf test-1.23.tar'], - 'test-1.23.tar.gz': ['tar -xzf test-1.23.tar.gz'], - 'test-1.23.tar.bz2': ['mkdir test-1.23', - 'cd test-1.23', - 'tar -jxf ../test-1.23.tar.bz2'], - 'test-1.23.zip': ['mkdir test-1.23', - 'cd test-1.23', - 'unzip -q ../test-1.23.zip'], - 'test-1.23.cpio': ['cpio -i --make-directories \ - /dev/null'], - 'test-1.23_all.deb': ['TD=$PWD', - 'mkdir test-1.23', - 'cd /tmp', - 'ar x $TD/test-1.23_all.deb data.tar.gz', - 'cd $TD/test-1.23', - 'tar -zxf /tmp/data.tar.gz', - 'rm /tmp/data.tar.gz']} +tests = {'test-1.23.tar': ([], ['tar -xf test-1.23.tar'], []), + 'test-1.23.tar.gz': ([], ['tar -xzf test-1.23.tar.gz'], []), + 'test-1.23.tar.bz2': ([], ['mkdir test-1.23', + 'cd test-1.23', + 'tar -jxf ../test-1.23.tar.bz2'], []), + 'test-1.23.zip': ([], ['mkdir test-1.23', + 'cd test-1.23', + 'unzip -q ../test-1.23.zip'], []), + 'test-1.23.cpio': ([], ['cpio -i --make-directories \ + /dev/null'], []), + 'test-1.23_all.deb': ([], ['TD=$PWD', + 'mkdir test-1.23', + 'cd /tmp', + 'ar x $TD/test-1.23_all.deb data.tar.gz', + 'cd $TD/test-1.23', + 'tar -zxf /tmp/data.tar.gz', + 'rm /tmp/data.tar.gz'], []), + 'test-recursive-badperms.tar.bz2': ( + ['-r'], + ['mkdir test-recursive-badperms', + 'cd test-recursive-badperms', + 'tar -jxf ../test-recursive-badperms.tar.bz2', + 'tar -xf test-badperms.tar', + 'mv testdir test-badperms', + 'chmod 755 test-badperms'], + ['if [ "x`cat test-recursive-badperms/test-badperms/testfile`" = "xhey" ]', + 'then exit 0; else exit 1; fi'] + )} if os.path.exists('scripts/x') and os.path.exists('tests'): os.chdir('tests') @@ -59,9 +70,9 @@ class ExtractorTest(object): - def __init__(self, archive_filename, commands): + def __init__(self, archive_filename, info): self.archive_filename = archive_filename - self.shell_commands = commands + self.arguments, self.shell_commands, self.shell_test = info def get_results(self, commands): status = subprocess.call(commands) @@ -73,17 +84,26 @@ process.stdout.close() return set(output.split('\n')) - def get_shell_results(self): + def write_script(self, commands): script = open(TESTSCRIPT_NAME, 'w') - script.write("%s%s\n" % (SCRIPT_PROLOGUE, - '\n'.join(self.shell_commands))) + script.write("%s%s\n" % (SCRIPT_PROLOGUE, '\n'.join(commands))) script.close() subprocess.call(['chmod', 'u+w', TESTSCRIPT_NAME]) + + def get_shell_results(self): + self.write_script(self.shell_commands) return self.get_results(['sh', TESTSCRIPT_NAME]) def get_extractor_results(self): - return self.get_results(['../scripts/x', self.archive_filename]) + return self.get_results(['../scripts/x'] + self.arguments + + [self.archive_filename]) + def get_posttest_result(self): + if not self.shell_test: + return 0 + self.write_script(self.shell_test) + return subprocess.call(['sh', TESTSCRIPT_NAME]) + def clean(self): status = subprocess.call(['find', '-mindepth', '1', '-maxdepth', '1', '-type', 'd', @@ -98,6 +118,7 @@ expected = self.get_shell_results() self.clean() actual = self.get_extractor_results() + posttest_result = self.get_posttest_result() self.clean() if expected is None: raise ExtractorTestError("could not get baseline results") @@ -110,6 +131,11 @@ print "Only in actual results:" print '\n'.join(actual.difference(expected)) return False + elif posttest_result != 0: + print "FAILED:", self.archive_filename + print "Posttest returned status code", posttest_result + print actual + return False else: print "Passed:", self.archive_filename return True diff -r 77043f4e6a9f -r 1f3cb3845dfd tests/test-recursive-badperms.tar.bz2 Binary file tests/test-recursive-badperms.tar.bz2 has changed