We need to use re.search() rather than re.match() most of the time in this rule. Also simplify it, add some more architectures to look for, and update the tests accordingly so they are actually correct and match what the rule is trying to do. Signed-off-by: Dan McGee --- Namcap/rules/carch.py | 18 ++++++++-------- Namcap/tests/pkgbuild/test_carch.py | 38 +++++++++++++++++++++++----------- namcap-tags | 2 +- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/Namcap/rules/carch.py b/Namcap/rules/carch.py index 0469460..7cb3319 100644 --- a/Namcap/rules/carch.py +++ b/Namcap/rules/carch.py @@ -26,15 +26,15 @@ class package(PkgbuildRule): name = "carch" description = "Verifies that no specific host type is used" def analyze(self, pkginfo, tar): - arches = ['i686', 'i586', 'x86_64'] - archmatch = re.compile('%s' % '|'.join(arches)) + arches = ['arm', 'i586', 'i686', 'ppc', 'x86_64'] + archmatch = re.compile(r'\b(%s)\b' % '|'.join(arches)) # Match an arch=(i686) line - archline = re.compile('arch=\w*(%s)' % '|'.join(arches)) - # Match a [ "$CARCH" = "x86_64" ] line - archif = re.compile('''\[\s*("|')\$CARCH("|').*("|')(%s)("|')\s*\]''' % '|'.join(arches)) - for i in pkginfo.pkgbuild: - if archmatch.match(i): - if not archline.match(i) and not archif.match(i): - self.warnings.append(("specific-host-type-used %s", ",".join(arches))) + archline = re.compile(r'\w*arch=') + for line in pkginfo.pkgbuild: + # strip any comments + line = line.split('#')[0] + match = archmatch.search(line) + if match and not archline.match(line) and not '$CARCH' in line: + self.warnings.append(("specific-host-type-used %s", match.group(1))) # vim: set ts=4 sw=4 noet: diff --git a/Namcap/tests/pkgbuild/test_carch.py b/Namcap/tests/pkgbuild/test_carch.py index 1ec8597..2d95b81 100644 --- a/Namcap/tests/pkgbuild/test_carch.py +++ b/Namcap/tests/pkgbuild/test_carch.py @@ -25,24 +25,32 @@ import Namcap.rules.carch as module class NamcapSpecialArchTest(PkgbuildTest): pkgbuild1 = """ -# Maintainer: Arch Linux -# Contributor: Arch Linux +pkgname=mypackage +pkgver=1.0 +pkgrel=1 +pkgdesc="A package" +arch=('i686') + +build() { + cd "${srcdir}"/${pkgname}-${pkgver} + [[ $CARCH == x86_64 ]] && CFLAGS+="-m64" + [ '$CARCH' = 'i686' ] && CFLAGS+="-m32" + ./configure --prefix=/usr + make +} +""" + pkgbuild2 = """ pkgname=mypackage pkgver=1.0 pkgrel=1 pkgdesc="A package" arch=('i686') -url="http://www.example.com/" -license=('GPL') -depends=('glibc') -options=('!libtool') -source=(ftp://ftp.example.com/pub/mypackage-0.1.tar.gz) -md5sums=('abcdefabcdef12345678901234567890') build() { cd "${srcdir}"/${pkgname}-${pkgver} - [[ $CARCH == x86_64 ]] && CFLAGS+="-m32" + [[ $CARCH == x86_64 ]] && CFLAGS+="-m64" + [ '$CARCH' = 'i686' ] && CFLAGS+="-m32" ./configure --prefix=/usr make } @@ -51,6 +59,7 @@ package() { cd "${srcdir}"/${pkgname}-${pkgver} ./configure --prefix=/usr make DESTDIR="${pkgdir}" install + cp foobar /usr/lib/i686/pkg/ } """ @@ -59,12 +68,17 @@ package() { def preSetUp(self): self.rule = module.package - @unittest.expectedFailure def test_example1(self): - # Example 1 r = self.run_on_pkg(self.pkgbuild1) self.assertEqual(r.errors, []) - self.assertEqual(r.warnings, ("specific-host-type-used %s", "i686")) + self.assertEqual(r.warnings, []) self.assertEqual(r.infos, []) + def test_example2(self): + r = self.run_on_pkg(self.pkgbuild2) + self.assertEqual(r.errors, []) + self.assertEqual(r.warnings, [("specific-host-type-used %s", "i686")]) + self.assertEqual(r.infos, []) + + # vim: set ts=4 sw=4 noet: diff --git a/namcap-tags b/namcap-tags index 46ef2a0..a342590 100644 --- a/namcap-tags +++ b/namcap-tags @@ -66,7 +66,7 @@ potential-non-fhs-info-page %s :: Potential non-FHS info page (%s) found. potential-non-fhs-man-page %s :: Potential non-FHS man page (%s) found. script-link-detected %s in %s :: Script link detected (%s) in file %s scrollkeeper-dir-exists %s :: Scrollkeeper directory exists (%s). Remember to not run scrollkeeper till post_{install,upgrade,remove}. -specific-host-type-used %s :: Reference to one of %s should be changed to $CARCH +specific-host-type-used %s :: Reference to %s should be changed to $CARCH specific-sourceforge-mirror :: Attempting to use specific sourceforge mirror, use downloads.sourceforge.net instead symlink-found %s points to %s :: Symlink (%s) found that points to %s too-many-checksums %s %i needed :: Too many %s: %i needed -- 1.7.5.2

* Do each import separate as is normal python convention * Fix some comment grammar and word usage * Rename verify_package() to open_package() to signify what it actually does, and remove a lot of unnecessary code in the process Signed-off-by: Dan McGee --- Namcap/ruleclass.py | 2 +- namcap.py | 37 +++++++++++++++---------------------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/Namcap/ruleclass.py b/Namcap/ruleclass.py index 17bc2b2..f34c657 100644 --- a/Namcap/ruleclass.py +++ b/Namcap/ruleclass.py @@ -24,7 +24,7 @@ This module defines the base classes from which Namcap rules are derived and how they are meant to be used. """ -# pacman 3 does not need classes to derive from object +# python 3 does not need classes to derive from object class AbstractRule(object): "The parent class of all rules" def __init__(self): diff --git a/namcap.py b/namcap.py index a53879b..dcebc55 100755 --- a/namcap.py +++ b/namcap.py @@ -19,13 +19,16 @@ # # -# System wide global stuff -import sys -import os -import imp import getopt -import types, tarfile, re, string, Namcap +import imp +import os +import re import shutil +import string +import sys +import tarfile +import types + import Namcap.depends import Namcap.tags @@ -51,26 +54,16 @@ def usage(): sys.exit(2) -def verify_package(filename): - """Checks if a tarball is a valid package""" - if not os.path.isfile(filename): - return 0 - if not tarfile.is_tarfile(filename): - return 0 +def open_package(filename): try: - tar = tarfile.open(package, "r") - if not tar: - return 0 - if not len(tar.getmembers()) > 0: - tar.close() - return 0 - if not '.PKGINFO' in tar.getnames(): + tar = tarfile.open(filename, "r") + if '.PKGINFO' not in tar.getnames(): tar.close() - return 0 + return None except IOError: if tar: tar.close() - return 0 + return None return tar def check_rules_exclude(optlist): @@ -91,7 +84,7 @@ def show_messages(name, key, messages): def process_realpackage(package, modules): """Runs namcap checks over a package tarball""" extracted = 0 - pkgtar = verify_package(package) + pkgtar = open_package(package) if not pkgtar: print("Error: %s is empty or is not a valid package" % package) @@ -105,7 +98,7 @@ def process_realpackage(package, modules): if isinstance(rule, Namcap.ruleclass.PkgInfoRule): rule.analyze(pkginfo, None) elif isinstance(rule, Namcap.ruleclass.PkgdirRule): - # If it's not extracted, then extract it and then analyze the package + # If it's not extracted, extract it and then analyze the package if not extracted: os.mkdir(sandbox_directory) for j in pkgtar.getmembers(): -- 1.7.5.2