# HG changeset patch # User brett # Date 1195853122 18000 # Node ID 027fcd7ae00202c0731a1f080143d4c1001b5f4f # Parent f637b9d24c2199c222effdfb8e80ea04659044eb [svn] Improve the error reporting to be more user-friendly, at least in many of the really basic cases. diff -r f637b9d24c21 -r 027fcd7ae002 scripts/dtrx --- a/scripts/dtrx Fri Nov 23 15:22:34 2007 -0500 +++ b/scripts/dtrx Fri Nov 23 16:25:22 2007 -0500 @@ -353,12 +353,16 @@ class NoPipeExtractor(BaseExtractor): # Some extraction tools won't accept the archive from stdin. With # these, the piping infrastructure we normally set up generally doesn't - # work, at least at first. We can still use most of it; we just can't - # seed self.archive with the archive file. So instead we seed it with - # /dev/null, and specify the filename on the command line as necessary. - # This class doesn't do anything by itself; it's just meant to be a - # base class for extractors that rely on these dumb tools. + # work, at least at first. We can still use most of it; we just don't + # want to seed self.archive with the archive file, since that sucks up + # memory. So instead we seed it with /dev/null, and specify the + # filename on the command line as necessary. We also open the actual + # file with os.open, to make sure we can actually do it (permissions + # are good, etc.). This class doesn't do anything by itself; it's just + # meant to be a base class for extractors that rely on these dumb + # tools. def __init__(self, filename, encoding): + os.close(os.open(filename, os.O_RDONLY)) BaseExtractor.__init__(self, '/dev/null', None) self.filename = os.path.realpath(filename) @@ -758,10 +762,7 @@ except (ExtractorError, IOError, OSError), exception: error = str(exception) logger.debug(''.join(traceback.format_exception(*sys.exc_info()))) - if error: - logger.info("%s: %s", self.current_filename, error) - return False - return True + return error class ExtractionAction(BaseAction): @@ -780,12 +781,12 @@ def run(self, filename, extractor): self.current_filename = filename - success = (self.report(extractor.extract) and - self.report(self.get_handler, extractor) and - self.report(self.current_handler.handle)) - if success: + error = (self.report(extractor.extract) or + self.report(self.get_handler, extractor) or + self.report(self.current_handler.handle)) + if not error: self.target = self.current_handler.target - return success + return error class ListAction(BaseAction): @@ -809,7 +810,7 @@ def run(self, filename, extractor): self.current_filename = filename - return (self.report(self.get_list, extractor) and + return (self.report(self.get_list, extractor) or self.report(self.show_list, filename)) @@ -878,26 +879,49 @@ action.target, tail_path) self.archives.setdefault(directory, []).append(basename) + def check_file(self, filename): + try: + result = os.stat(filename) + except OSError, error: + return error.strerror + if stat.S_ISDIR(result.st_mode): + return "cannot extract a directory" + + def try_extractors(self, filename, builder): + last_error = "could not find a way to extract this" + while True: + try: + extractor = builder.next() + except StopIteration: + return last_error + except (IOError, OSError, ExtractorError), error: + return str(error) + error = self.action.run(filename, extractor) + if error: + logger.info("%s: %s" % (filename, error)) + last_error = error + else: + self.recurse(filename, extractor, self.action) + return + def run(self): if self.options.show_list: action = ListAction else: action = ExtractionAction - action = action(self.options, self.archives.values()[0]) + self.action = action(self.options, self.archives.values()[0]) while self.archives: self.current_directory, self.filenames = self.archives.popitem() os.chdir(self.current_directory) for filename in self.filenames: builder = ExtractorBuilder(filename, self.options) - for extractor in builder.get_extractor(): - if action.run(filename, extractor): - self.successes.append(filename) - self.recurse(filename, extractor, action) - break + error = (self.check_file(filename) or + self.try_extractors(filename, builder.get_extractor())) + if error: + logger.error("%s: %s" % (filename, error)) + self.failures.append(filename) else: - logger.error("%s: could not find a way to extract this" % - (filename,)) - self.failures.append(filename) + self.successes.append(filename) self.options.one_entry_policy.permanent_policy = EXTRACT_WRAP if self.failures: return 1 diff -r f637b9d24c21 -r 027fcd7ae002 tests/tests.yml --- a/tests/tests.yml Fri Nov 23 15:22:34 2007 -0500 +++ b/tests/tests.yml Fri Nov 23 16:25:22 2007 -0500 @@ -421,7 +421,49 @@ cd trickery unzip -q ../$1 -- name: get an error when extracting a non-archive +- name: non-archive error filenames: /dev/null error: true - grep: ERROR + grep: "could not find a way to extract this" + +- name: no such file error + filenames: nonexistent-file.tar.gz + error: true + grep: "[Nn]o such file" + +- name: no such file error with no extension + filenames: nonexistent-file + error: true + grep: "[Nn]o such file" + +- name: try to extract a directory error + filenames: test-directory + prerun: mkdir test-directory + error: true + grep: "cannot extract a directory" + +- name: permission denied error + filenames: unreadable-file.tar.gz + prerun: | + touch unreadable-file.tar.gz + chmod 000 unreadable-file.tar.gz + cleanup: rm -f unreadable-file.tar.gz + error: true + grep: "[Pp]ermission denied" + +- name: permission denied no-pipe file error + filenames: unreadable-file.zip + prerun: | + touch unreadable-file.zip + chmod 000 unreadable-file.zip + cleanup: rm -f unreadable-file.zip + error: true + grep: "[Pp]ermission denied" + +- name: bad file error + filenames: bogus-file.tar.gz + prerun: | + touch bogus-file.tar.gz + cleanup: rm -f bogus-file.tar.gz + error: true + grep: "returned status code [^0]"