From b0af986ec7f49089e9cbe1db787b9f42de98a979 Mon Sep 17 00:00:00 2001 From: Oula Kuuva Date: Tue, 3 Jul 2018 14:56:44 +0300 Subject: [PATCH 1/6] Use os.path functions from path directly More pythonic, plus outdir option needs join() which would be very ambiguous without context. --- bin/pysapcar | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/bin/pysapcar b/bin/pysapcar index 21e2cd8..2537427 100644 --- a/bin/pysapcar +++ b/bin/pysapcar @@ -21,8 +21,7 @@ # Standard imports import logging from sys import stdin -from os import mkdir, utime -from os.path import dirname, exists, normpath +from os import mkdir, utime, path from optparse import OptionParser, OptionGroup # Custom imports import pysap @@ -241,11 +240,11 @@ class PySAPCAR(object): for filename in self.target_files(sapcar.files_names, args): flag = CONTINUE fil = sapcar.files[filename] - filename = normpath(filename.replace("\x00", "")) # Take out null bytes if found + filename = path.normpath(filename.replace("\x00", "")) # Take out null bytes if found if fil.is_directory(): # If the directory doesn't exist, create it and set permissions and timestamp - if not exists(filename): + if not path.exists(filename): mkdir(filename) if chmod: chmod(filename, fil.perm_mode) @@ -255,8 +254,8 @@ class PySAPCAR(object): elif fil.is_file(): # If the file references a directory that is not there, create it first - file_dirname = dirname(filename) - if file_dirname and file_dirname != "" and not exists(file_dirname): + file_dirname = path.dirname(filename) + if file_dirname and file_dirname != "" and not path.exists(file_dirname): mkdir(file_dirname) self.logger.info("d %s", file_dirname) From bc64316d5aa587ed90057edd7c3a65f9b38ea291 Mon Sep 17 00:00:00 2001 From: Oula Kuuva Date: Tue, 3 Jul 2018 15:05:24 +0300 Subject: [PATCH 2/6] Fix some cosmetic issues to silence the linter Methods to static, redundant dirname check, print statement to logger.debug(). --- bin/pysapcar | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/bin/pysapcar b/bin/pysapcar index 2537427..9a2e062 100644 --- a/bin/pysapcar +++ b/bin/pysapcar @@ -63,7 +63,8 @@ class PySAPCAR(object): log_level = None archive_fd = None - def parse_options(self): + @staticmethod + def parse_options(): """Parses command-line options. """ @@ -131,8 +132,7 @@ class PySAPCAR(object): try: self.archive_fd = open(options.filename, self.mode) except IOError as e: - self.logger.error("pysapcar: error opening '%s' (%s)" % (options.filename, - e.strerror)) + self.logger.error("pysapcar: error opening '%s' (%s)" % (options.filename, e.strerror)) return else: self.archive_fd = stdin @@ -160,7 +160,8 @@ class PySAPCAR(object): return None return sapcar - def target_files(self, filenames, target_filenames=None): + @staticmethod + def target_files(filenames, target_filenames=None): """Generates the list of files to work on. It calculates the intersection between the file names selected in command-line and the ones in the archive to work on. @@ -193,7 +194,7 @@ class PySAPCAR(object): while len(args): filename = filename_in_archive = args.pop(0) - print args + self.logger.debug(args) if len(args) >= 2 and args[0] == "/n": args.pop(0) filename_in_archive = args.pop(0) @@ -255,7 +256,7 @@ class PySAPCAR(object): elif fil.is_file(): # If the file references a directory that is not there, create it first file_dirname = path.dirname(filename) - if file_dirname and file_dirname != "" and not path.exists(file_dirname): + if file_dirname and not path.exists(file_dirname): mkdir(file_dirname) self.logger.info("d %s", file_dirname) @@ -268,7 +269,7 @@ class PySAPCAR(object): flag = STOP else: flag = SKIP - except SAPCARInvalidChecksumException as e: + except SAPCARInvalidChecksumException: self.logger.error("pysapcar: Invalid checksum found for file '%s'", fil.filename) if options.enforce_checksum: flag = STOP From 94d4f17c8779d8590d64a4aac180e399d0af79ea Mon Sep 17 00:00:00 2001 From: Oula Kuuva Date: Tue, 3 Jul 2018 15:22:52 +0300 Subject: [PATCH 3/6] Replace deprecated optparse with argparse No changes to functionality, just future proofing. --- bin/pysapcar | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/bin/pysapcar b/bin/pysapcar index 9a2e062..5827043 100644 --- a/bin/pysapcar +++ b/bin/pysapcar @@ -22,7 +22,7 @@ import logging from sys import stdin from os import mkdir, utime, path -from optparse import OptionParser, OptionGroup +from argparse import ArgumentParser # Custom imports import pysap from pysapcompress import DecompressError @@ -74,25 +74,24 @@ class PySAPCAR(object): "url": pysap.__url__, "repo": pysap.__repo__} - parser = OptionParser(usage=pysapcar_usage, description=description, epilog=epilog) + parser = ArgumentParser(usage=pysapcar_usage, description=description, epilog=epilog) # Commands - parser.add_option("-c", dest="create", action="store_true", help="Create archive with specified files") - parser.add_option("-x", dest="extract", action="store_true", help="Extract files from an archive") - parser.add_option("-t", dest="list", action="store_true", help="List the contents of an archive") - parser.add_option("-a", dest="append", action="store_true", help="Append files to an archive") - parser.add_option("-f", dest="filename", help="Archive filename", metavar="FILE") - - misc = OptionGroup(parser, "Misc options") - misc.add_option("-v", dest="verbose", action="count", help="Verbose output") - misc.add_option("-e", "--enforce-checksum", dest="enforce_checksum", action="store_true", - help="Whether the checksum validation is enforced. A file with an invalid checksum won't be " - "extracted. When not set, only a warning would be thrown if checksum is invalid.") - misc.add_option("-b", "--break-on-error", dest="break_on_error", action="store_true", - help="Whether the extraction would continue if an error is identified.") - parser.add_option_group(misc) - - (options, args) = parser.parse_args() + parser.add_argument("-c", dest="create", action="store_true", help="Create archive with specified files") + parser.add_argument("-x", dest="extract", action="store_true", help="Extract files from an archive") + parser.add_argument("-t", dest="list", action="store_true", help="List the contents of an archive") + parser.add_argument("-a", dest="append", action="store_true", help="Append files to an archive") + parser.add_argument("-f", dest="filename", help="Archive filename", metavar="FILE") + + misc = parser.add_argument_group("Misc options") + misc.add_argument("-v", dest="verbose", action="count", help="Verbose output") + misc.add_argument("-e", "--enforce-checksum", dest="enforce_checksum", action="store_true", + help="Whether the checksum validation is enforced. A file with an invalid checksum won't be " + "extracted. When not set, only a warning would be thrown if checksum is invalid.") + misc.add_argument("-b", "--break-on-error", dest="break_on_error", action="store_true", + help="Whether the extraction would continue if an error is identified.") + + (options, args) = parser.parse_known_args() return options, args From 8018a217de0cab51a03ae8a644bb3239c6efe0c3 Mon Sep 17 00:00:00 2001 From: Oula Kuuva Date: Tue, 3 Jul 2018 15:42:28 +0300 Subject: [PATCH 4/6] Add option for specifying outdir for extraction --- bin/pysapcar | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bin/pysapcar b/bin/pysapcar index 5827043..68e4c20 100644 --- a/bin/pysapcar +++ b/bin/pysapcar @@ -22,6 +22,7 @@ import logging from sys import stdin from os import mkdir, utime, path +from os import sep as dir_separator from argparse import ArgumentParser # Custom imports import pysap @@ -45,7 +46,7 @@ list the contents of an archive: pysapcar -t[v][f archive] [file1 file2 ...] extract files from an archive: -pysapcar -x[v][f archive] [file1 file2 ...] +pysapcar -x[v][f archive] [-o outdir] [file1 file2 ...] append files to an archive: pysapcar -a[v][f archive] [file1 file2 [/n filename] ...] @@ -82,6 +83,7 @@ class PySAPCAR(object): parser.add_argument("-t", dest="list", action="store_true", help="List the contents of an archive") parser.add_argument("-a", dest="append", action="store_true", help="Append files to an archive") parser.add_argument("-f", dest="filename", help="Archive filename", metavar="FILE") + parser.add_argument("-o", dest="outdir", help="Path to directory where to extract files") misc = parser.add_argument_group("Misc options") misc.add_argument("-v", dest="verbose", action="count", help="Verbose output") @@ -241,6 +243,10 @@ class PySAPCAR(object): flag = CONTINUE fil = sapcar.files[filename] filename = path.normpath(filename.replace("\x00", "")) # Take out null bytes if found + if options.outdir: + # Have to strip directory separator from the beginning of the file name, because path.join disregards + # all previous components if any of the following components is an absolute path + filename = path.join(path.normpath(options.outdir), filename.lstrip(dir_separator)) if fil.is_directory(): # If the directory doesn't exist, create it and set permissions and timestamp From 9f53a2f4a9752b3049fad3f844c94361eb88bce5 Mon Sep 17 00:00:00 2001 From: Oula Kuuva Date: Tue, 3 Jul 2018 15:50:01 +0300 Subject: [PATCH 5/6] Use makedirs instead of mkdir when extracting mkdir barfs if archive contains /foo/bar/bash but not the intermediate directory /foo/bar. makedirs creates intermediate directories as well as the leaf dir. --- bin/pysapcar | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bin/pysapcar b/bin/pysapcar index 68e4c20..db62c56 100644 --- a/bin/pysapcar +++ b/bin/pysapcar @@ -21,7 +21,7 @@ # Standard imports import logging from sys import stdin -from os import mkdir, utime, path +from os import makedirs, utime, path from os import sep as dir_separator from argparse import ArgumentParser # Custom imports @@ -251,7 +251,7 @@ class PySAPCAR(object): if fil.is_directory(): # If the directory doesn't exist, create it and set permissions and timestamp if not path.exists(filename): - mkdir(filename) + makedirs(filename) if chmod: chmod(filename, fil.perm_mode) utime(filename, (fil.timestamp_raw, fil.timestamp_raw)) @@ -262,7 +262,9 @@ class PySAPCAR(object): # If the file references a directory that is not there, create it first file_dirname = path.dirname(filename) if file_dirname and not path.exists(file_dirname): - mkdir(file_dirname) + # mkdir barfs if archive contains /foo/bar/bash but not /foo/bar directory. + # makedirs creates intermediate directories as well + makedirs(file_dirname) self.logger.info("d %s", file_dirname) # Try to extract the file and handle potential errors From 12ac48c08d081a1fd41606409ed02c42e9eec7d1 Mon Sep 17 00:00:00 2001 From: Oula Kuuva Date: Tue, 3 Jul 2018 16:00:21 +0300 Subject: [PATCH 6/6] Handle OSErrors gracefully while extracting --- bin/pysapcar | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/bin/pysapcar b/bin/pysapcar index db62c56..0237e01 100644 --- a/bin/pysapcar +++ b/bin/pysapcar @@ -290,10 +290,15 @@ class PySAPCAR(object): break # Write the new file and set permissions - with open(filename, "wb") as new_file: - new_file.write(data) - if fchmod: - fchmod(new_file.fileno(), fil.perm_mode) + try: + with open(filename, "wb") as new_file: + new_file.write(data) + if fchmod: + fchmod(new_file.fileno(), fil.perm_mode) + except IOError as e: + self.logger.error("pysapcar: Failed to extract file '%s', reason: %s", filename, e.strerror) + if options.break_on_error: + break # Set the timestamp utime(filename, (fil.timestamp_raw, fil.timestamp_raw))