On 12/11/20 5:10 PM, meganomic via arch-projects wrote:
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.
Nice, thanks! Haven't taken more than a cursory look at the implementation, but one thing stands out for cleanup, see below.
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':
if pkgpath.suffix in ('.gz', '.z', '.Z', '.bz2', '.bz', '.xz', '.zst') will be much better for readability than repeating `pkgpath.suffix ==` once for each suffix.
+ 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)
-- Eli Schwartz Bug Wrangler and Trusted User