I did a quick patch using your idea and it definitely seems like a good way to do it. Can probably clean it up and make it look prettier with some refactoring. I'll look into it later. diff --git a/namcap.py b/namcap.py index a7f532a..0e78b89 100755 --- a/namcap.py +++ b/namcap.py @@ -22,7 +22,10 @@ import getopt import os import sys -import tarfile +import subprocess +import tempfile +import pathlib +import xtarfile import Namcap.depends import Namcap.tags @@ -49,18 +52,6 @@ def usage(): sys.exit(2) -def open_package(filename): - try: - tar = tarfile.open(filename, "r") - if '.PKGINFO' not in tar.getnames(): - tar.close() - return None - except IOError: - if tar: - tar.close() - return None - return tar - def check_rules_exclude(optlist): '''Check if the -r (--rules) and the -r (--exclude) options are being used at same time''' @@ -79,39 +70,38 @@ def show_messages(name, key, messages): def process_realpackage(package, modules): """Runs namcap checks over a package tarball""" extracted = 0 - pkgtar = open_package(package) - - if not pkgtar: - print("Error: %s is empty or is not a valid package" % package) - return 1 - - pkginfo = Namcap.package.load_from_tarball(package) - # Loop through each one, load them apply if possible - for i in modules: - rule = get_modules()[i]() - - if isinstance(rule, Namcap.ruleclass.PkgInfoRule): - rule.analyze(pkginfo, None) - elif isinstance(rule, Namcap.ruleclass.PkgbuildRule): - pass - elif isinstance(rule, Namcap.ruleclass.TarballRule): - rule.analyze(pkginfo, pkgtar) - else: - show_messages(pkginfo["name"], 'E', - [('error-running-rule %s', i)]) - - # Output the three types of messages - show_messages(pkginfo["name"], 'E', rule.errors) - show_messages(pkginfo["name"], 'W', rule.warnings) + with xtarfile.open(package, "r") as pkgtar: + if '.PKGINFO' not in pkgtar.getnames(): + print("Error: %s is not a valid package" % package) + return 1 + + pkginfo = Namcap.package.load_from_tarball(package) + # Loop through each one, load them apply if possible + for i in modules: + rule = get_modules()[i]() + + if isinstance(rule, Namcap.ruleclass.PkgInfoRule): + rule.analyze(pkginfo, None) + elif isinstance(rule, Namcap.ruleclass.PkgbuildRule): + pass + elif isinstance(rule, Namcap.ruleclass.TarballRule): + rule.analyze(pkginfo, pkgtar) + else: + show_messages(pkginfo["name"], 'E', + [('error-running-rule %s', i)]) + + # Output the three types of messages + show_messages(pkginfo["name"], 'E', rule.errors) + show_messages(pkginfo["name"], 'W', rule.warnings) + if info_reporting: + show_messages(pkginfo["name"], 'I', rule.infos) + + # dependency analysis + errs, warns, infos = Namcap.depends.analyze_depends(pkginfo) + show_messages(pkginfo["name"], 'E', errs) + show_messages(pkginfo["name"], 'W', warns) if info_reporting: - show_messages(pkginfo["name"], 'I', rule.infos) - - # dependency analysis - errs, warns, infos = Namcap.depends.analyze_depends(pkginfo) - show_messages(pkginfo["name"], 'E', errs) - show_messages(pkginfo["name"], 'W', warns) - if info_reporting: - show_messages(pkginfo["name"], 'I', infos) + show_messages(pkginfo["name"], 'I', infos) def process_pkginfo(pkginfo, modules): """Runs namcap checks of a single, non-split PacmanPackage object""" @@ -237,14 +227,45 @@ if len(active_modules) == 0: # Go through each package, get the info, and apply the rules for package in packages: - if not os.access(package, os.R_OK): + pkgpath = pathlib.Path(package) + + if not pkgpath.is_file(): print("Error: Problem reading %s" % package) - usage() + parser.print_usage() + continue # Skip to next package if any + + if pkgpath.with_suffix('').suffix == '.tar': + # What compression is used? + extra_opts = '' + if pkgpath.suffix == '.gz' or pkgpath.suffix == '.z' or pkgpath.suffix == '.Z' or pkgpath.suffix == '.bz2' or pkgpath.suffix == '.bz' or pkgpath.suffix == '.xz' or pkgpath.suffix == '.zst': + process_realpackage(package, active_modules) + continue # Skip to next package if any + elif pkgpath.suffix == '.lzo': + cmd = 'lzop' + elif pkgpath.suffix == '.lrz': + cmd = 'lrzip' + extra_opts="-q -o -" + elif pkgpath.suffix == '.lz4': + cmd = 'lz4' + extra_opts="-q" + elif pkgpath.suffix == '.lz': + cmd = 'lzip' + extra_opts="-q" + else: + print("Unsupported compression!") + continue # Skip to next package if any + + # Run decompression and put the .tar file in a temporary directory + tmpdir = tempfile.TemporaryDirectory(prefix='namcap.') + tmpfilepath = pathlib.Path(tmpdir.name).joinpath(pkgpath.with_suffix('').name) + subprocess.run(cmd + ' -dcf ' + extra_opts + pkgpath.as_posix() + ' > ' + tmpfilepath.as_posix(), shell=True) + + if tmpfilepath.is_file() and xtarfile.is_tarfile(tmpfilepath): + process_realpackage(package, active_modules) - if os.path.isfile(package) and tarfile.is_tarfile(package): - process_realpackage(package, active_modules) elif 'PKGBUILD' in package: process_pkgbuild(package, active_modules) + else: print("Error: %s not package or PKGBUILD" % package) ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Friday, December 11, 2020 9:29 PM, Eli Schwartz via arch-projects <arch-projects@archlinux.org> wrote:
On 12/11/20 2:25 PM, meganomic via arch-projects wrote:
Adds handling of the compression and temporary storage into namcap.py so it can be removed from the bash script. https://bugs.archlinux.org/task/59844 mentions "use setuptools entry points." instead of the bash script but I don't know how to do that. I just removed it from the bash script for now.
The subprocess to decompression tools is still (with your patch) just as annoying to me as the existence of the bash script. Python's builtin tarfile.open can handle gz, bz2, or xz. Using the python-xtarfile module you can use xtarfile.open to also handle zst. We should prefer the native code instead of basically inlining bash into python.
True, in order to support lzo, lrz, lz4, lz, we might still need to do subprocess out and create a temporary file with the decompressed package, but this should really be a last resort (and users in practice should rarely hit this code).
What do you think about trying xtarfile first?
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Eli Schwartz Bug Wrangler and Trusted User