On Tue, Jun 8, 2010 at 6:37 AM, Allan McRae <allan@archlinux.org> wrote:
Check every checksum that makepkg supports rather than only md5sums. Fixes FS#17168.
Improved by: Kazuo Teramoto <kaz.rag@gmail.com> Signed-off-by: Allan McRae <allan@archlinux.org> ---
Big thanks to Kazuo Teramoto who helped me remove that ugly code duplication!
Namcap/__init__.py | 2 +- Namcap/arrays.py | 3 +- Namcap/checksums.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++ Namcap/extravars.py | 3 +- Namcap/md5sums.py | 44 ------------------------------------------ namcap-tags | 8 +++--- namcap.1 | 6 ++-- parsepkgbuild | 21 ++++++++++++++++++++ 8 files changed, 86 insertions(+), 54 deletions(-) create mode 100644 Namcap/checksums.py delete mode 100644 Namcap/md5sums.py
diff --git a/Namcap/__init__.py b/Namcap/__init__.py index 77aa63b..457902a 100644 --- a/Namcap/__init__.py +++ b/Namcap/__init__.py @@ -47,9 +47,9 @@ __pkgbuild__ = """ badbackups capsnames carch + checksums invalidstartdir license - md5sums pkgname rpath sfurl diff --git a/Namcap/arrays.py b/Namcap/arrays.py index e3111aa..3ecb1ed 100644 --- a/Namcap/arrays.py +++ b/Namcap/arrays.py @@ -29,7 +29,8 @@ class package: def analyze(self, pkginfo, tar): arrayvars = ['arch', 'license', 'depends', 'makedepends', 'optdepends', 'provides', 'conflicts' , 'replaces', - 'backup', 'source', 'noextract', 'md5sums'] + 'backup', 'source', 'noextract', 'md5sums', + 'sha1sums', 'sha256sums', 'sha384sums', 'sha512sums'] Added trailing spaces here, bad! But I fixed it.
ret = [[], [], []] for i in pkginfo.pkgbuild: m = re.match('\s*(.*)\s*=\s*(.*)\n', i) diff --git a/Namcap/checksums.py b/Namcap/checksums.py new file mode 100644 index 0000000..04f887a --- /dev/null +++ b/Namcap/checksums.py Use -M -B- C next time as args to git-format-patch so this comes across as a file update (with a different name) rather than a completely new file. That way we actually get a diff.
@@ -0,0 +1,53 @@ +# +# namcap rules - checksums +# Copyright (C) 2003-2009 Jason Chu <jason@archlinux.org> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +# + +class package: + def short_name(self): + return "checksums" + def long_name(self): + return "Verifies checksums are included in a PKGBUILD" + def prereq(self): + return "" + def analyze(self, pkginfo, tar): + ret = [[],[],[]] You undid here what I did last night in c7a1c6d1e7ab7. Fixed it.
+ checksums=[('md5', 32), ('sha1', 40), ('sha256', 63), ('sha384', 96), ('sha512', 128)] + + if hasattr(pkginfo, 'source'): + haschecksums=False + for i,j in checksums: + if hasattr(pkginfo, i+'sums'): + haschecksums=True + if not haschecksums: + ret[0].append(("missing-checksums", ())) + + for sumname,sumlen in checksums: + sumname+='sums' + if hasattr(pkginfo, sumname): + if len(pkginfo.source) > len(getattr(pkginfo, sumname)): + ret[0].append(("not-enough-checksums %s %i needed", (sumname, len(pkginfo.source)))) + elif len(pkginfo.source) < len(getattr(pkginfo, sumname)): + ret[0].append(("too-many-checksums %s %i needed", (sumname, len(pkginfo.source)))) + for sum in getattr(pkginfo, sumname): + if len(sum) != sumlen: + ret[0].append(("improper-checksum %s %s", (sumname, sum)))
I can tell you are used to bash. Python convention likes spaces a bit more than what you were doing (although you also had several spots where you mixed having spaces and not having them). Instead of "sumname+='sums'", try "sumname<space>+=<space>'sums'". I've fixed this. Also, why use sumname/sumlen in one place and i/j in the other? Be consistent.
+ + return ret + def type(self): + return "pkgbuild" +# vim: set ts=4 sw=4 noet: diff --git a/Namcap/extravars.py b/Namcap/extravars.py index aebeb64..ac5a2ab 100644 --- a/Namcap/extravars.py +++ b/Namcap/extravars.py @@ -29,7 +29,8 @@ class package: def analyze(self, pkginfo, tar): stdvars = ['arch', 'license', 'depends', 'makedepends', 'provides', 'conflicts' , 'replaces', 'backup', - 'source', 'noextract', 'md5sums', 'pkgname', + 'source', 'noextract', 'md5sums', 'sha1sums', + 'sha256sums', 'sha384sums', 'sha512sums', 'pkgname', 'pkgver', 'pkgrel', 'pkgdesc', 'url', 'install'] ret = [[], [], []] for i in pkginfo.pkgbuild: diff --git a/Namcap/md5sums.py b/Namcap/md5sums.py deleted file mode 100644 index 0d60cd7..0000000 --- a/Namcap/md5sums.py +++ /dev/null @@ -1,44 +0,0 @@ -# -# namcap rules - md5sums -# Copyright (C) 2003-2009 Jason Chu <jason@archlinux.org> -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -# - -class package: - def short_name(self): - return "md5sums" - def long_name(self): - return "Verifies md5sums are included in a PKGBUILD" - def prereq(self): - return "" - def analyze(self, pkginfo, tar): - ret = [[], [], []] - if hasattr(pkginfo, 'source'): - if not hasattr(pkginfo, 'md5sums'): - ret[0].append(("missing-md5sums", ())) - else: - if len(pkginfo.source) > len(pkginfo.md5sums): - ret[0].append(("not-enough-md5sums %i needed", len(pkginfo.source))) - elif len(pkginfo.source) < len(pkginfo.md5sums): - ret[0].append(("too-many-md5sums %i needed", len(pkginfo.source))) - if hasattr(pkginfo, 'md5sums'): - for sum in pkginfo.md5sums: - if len(sum) != 32: - ret[0].append(("improper-md5sum %s", sum)) - return ret - def type(self): - return "pkgbuild" -# vim: set ts=4 sw=4 noet: diff --git a/namcap-tags b/namcap-tags index c4b1b3d..acb8e9c 100644 --- a/namcap-tags +++ b/namcap-tags @@ -30,7 +30,7 @@ file-world-writable %s :: File (%s) has the world writable bit set. gnome-mime-file %s :: File (%s) is an auto-generated GNOME mime file hardlink-found %s points to %s :: Hard link (%s) found that points to %s hicolor-icon-cache-not-updated :: Files in /usr/share/icons/hicolor but no call to gtk-update-icon-cache or xdg-icon-resource to update the icon cache -improper-md5sum %s :: Improper md5sum: '%s' +improper-checksum %s %s :: Improper %s: '%s' incorrect-library-permissions %s :: Library (%s) does not have permission set to 644 or 444 incorrect-permissions %s (%s/%s) :: File (%s) has %s/%s permissions info-dir-file-present %s :: Info directory file (%s) should not be present @@ -45,12 +45,12 @@ missing-custom-license-dir usr/share/licenses/%s :: Missing custom license direc missing-custom-license-file usr/share/licenses/%s/* :: Missing custom license file in package (usr/share/licenses/%s/*) missing-license :: Missing license missing-maintainer :: Missing Maintainer tag -missing-md5sums :: Missing md5sums +missing-checksums :: Missing checksums missing-url :: Missing url non-fhs-info-page %s :: Non-FHS info page (%s) found. Use /usr/share/info instead non-fhs-man-page %s :: Non-FHS man page (%s) found. Use /usr/share/man instead not-a-common-license %s :: %s is not a common license (it's not in /usr/share/licenses/common/) -not-enough-md5sums %i needed :: Not enough md5sums: %i needed +not-enough-checksums %s %i needed :: Not enough %s: %i needed package-name-in-uppercase :: No upper case letters in package names perllocal-pod-present %s :: perllocal.pod found in %s. pkgname-in-description :: Description should not contain the package name. @@ -61,7 +61,7 @@ scrollkeeper-dir-exists %s :: Scrollkeeper directory exists (%s). Remember to no specific-host-type-used %s :: Reference to one of %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-md5sums %i needed :: Too Many md5sums: %i needed +too-many-checksums %s %i needed :: Too many %s: %i needed use-pkgdir :: Use $pkgdir instead of $startdir/pkg use-srcdir :: Use $srcdir instead of $startdir/src using-dl-sourceforge :: Attempting to use dl sourceforge domain, use downloads.sourceforge.net instead diff --git a/namcap.1 b/namcap.1 index ea0a65a..3ee9075 100644 --- a/namcap.1 +++ b/namcap.1 @@ -40,6 +40,9 @@ Checks a PKGBUILD to verify that the package name has no upper case characters .B capsnamespkg Checks a package to verify that the package name has no upper case characters .TP +.B checksums +Makes sure that a PKGBUILD includes valid checksums +.TP .B depends This module runs ldd on all executables, gets the link-level dependencies, finds the smallest subset of dependencies that cover the link-level dependencies, and compares that list to the depends of the package. It returns messages in three cases: dependency detected and not included, dependency included but already satisfied, and dependency included and not needed. These suggestions are just guidelines and all package builders should take this into account (i.e. you're smarter than namcap is)
@@ -90,9 +93,6 @@ Verifies that the licenses variable has been filled in in a package. For package checks whether the license file has been installed in /usr/share/licenses/$pkgname/ .TP -.B md5sums -Makes sure that a PKGBUILD includes the md5sums -.TP .B mimefiles Checks whether update-mime-database is called when the package installs files in /usr/share/mime .TP diff --git a/parsepkgbuild b/parsepkgbuild index 295416b..f9b5a91 100755 --- a/parsepkgbuild +++ b/parsepkgbuild @@ -94,6 +94,27 @@ if [ -n "\$md5sums" ]; then for i in \${md5sums[@]}; do echo \$i; done echo "" fi +if [ -n "\$sha1sums" ]; then + echo "%SHA1SUMS%" + for i in \${sha1sums[@]}; do echo \$i; done + echo "" +fi +if [ -n "\$sha256sums" ]; then + echo "%SHA256SUMS%" + for i in \${sha256sums[@]}; do echo \$i; done + echo "" +fi +if [ -n "\$sha384sums" ]; then + echo "%SHA384SUMS%" + for i in \${sha384sums[@]}; do echo \$i; done + echo "" +fi +if [ -n "\$sha512sums" ]; then + echo "%SHA512SUMS%" + for i in \${sha512sums[@]}; do echo \$i; done + echo "" +fi +
[ -n "\$install" ] && echo -e "%INSTALL%\n\$install\n"
-- 1.7.1
Thanks, Allan. -Dan