Mon, 26 Nov 2007 22:31:25 -0500
[svn] Minor clean-ups. The most important of these is that we now have a better
strategy for dealing with the pathological case of having many extraction
destinations taken: after trying various names with number suffixes, we'll
finally resort to mkstemp/mkdtemp to get what we need. This still isn't
guaranteed to work but it's much more reliable. Also print out a warning
for the user when we extract to some weird directory.
scripts/dtrx | file | annotate | diff | comparison | revisions | |
tests/tests.yml | file | annotate | diff | comparison | revisions |
--- a/scripts/dtrx Fri Nov 23 17:55:07 2007 -0500 +++ b/scripts/dtrx Mon Nov 26 22:31:25 2007 -0500 @@ -63,7 +63,7 @@ mimetypes.encodings_map.setdefault('.bz2', 'bzip2') mimetypes.encodings_map.setdefault('.lzma', 'lzma') -mimetypes.types_map.setdefault('.gem', 'x-ruby-gem') +mimetypes.types_map.setdefault('.gem', 'application/x-ruby-gem') logger = logging.getLogger('dtrx-log') @@ -88,14 +88,21 @@ def is_free(self, filename): return not os.path.exists(filename) + def create(self): + fd, filename = tempfile.mkstemp(prefix=self.original_name + '.', + dir='.') + os.close(fd) + return filename + def check(self): + if self.is_free(self.original_name): + return self.original_name for suffix in [''] + ['.%s' % (x,) for x in range(1, 10)]: filename = '%s%s' % (self.original_name, suffix) if self.is_free(filename): return filename - raise ValueError("all alternatives for name %s taken" % - (self.original_name,)) - + return self.create() + class DirectoryChecker(FilenameChecker): def is_free(self, filename): @@ -107,6 +114,9 @@ raise return True + def create(self): + return tempfile.mkdtemp(prefix=self.original_name + '.', dir='.') + class ExtractorError(Exception): pass @@ -141,11 +151,11 @@ self.pipes.append((command, description)) def run_pipes(self, final_stdout=None): - if final_stdout is None: + if not self.pipes: + return + elif final_stdout is None: # FIXME: Buffering this might be dumb. final_stdout = tempfile.TemporaryFile() - if not self.pipes: - return num_pipes = len(self.pipes) last_pipe = num_pipes - 1 processes = [] @@ -447,6 +457,12 @@ return "%s returned with exit status %s" % (command, status) return self.organize() + def set_target(self, target, checker): + self.target = checker(target).check() + if self.target != target: + logger.warning("extracting %s to %s" % + (self.extractor.filename, self.target)) + # The "where to extract" table, with options and archive types. # This dictates the contents of each can_handle method. @@ -513,7 +529,7 @@ destination = self.extractor.content_name.rstrip('/') else: destination = self.extractor.basename() - self.target = checker(destination).check() + self.set_target(destination, checker) if os.path.isdir(self.extractor.target): os.rename(source, self.target) os.rmdir(self.extractor.target) @@ -537,7 +553,7 @@ def organize(self): basename = self.extractor.basename() - self.target = self.extractor.name_checker(basename).check() + self.set_target(basename, self.extractor.name_checker) os.rename(self.extractor.target, self.target)
--- a/tests/tests.yml Fri Nov 23 17:55:07 2007 -0500 +++ b/tests/tests.yml Mon Nov 26 22:31:25 2007 -0500 @@ -472,3 +472,25 @@ prerun: chmod 500 . error: true grep: "cannot extract here: [Pp]ermission denied" + +- name: recursive listing is a no-op + options: -rl + filenames: test-recursive-badperms.tar.bz2 + grep: test-badperms.tar + antigrep: testdir/ + +- name: graceful coping when many extraction directories are taken + directory: busydir + prerun: | + mkdir test-1.23 + for i in $(seq 1 10); do mkdir test-1.23.$i; done + filenames: ../test-1.23.tar.gz + grep: "WARNING: extracting" + +- name: graceful coping when many decompression targets are taken + directory: busydir + prerun: | + touch test-text + for i in $(seq 1 10); do touch test-text.$i; done + filenames: ../test-text.gz + grep: "WARNING: extracting"