[arch-dev-public] [PATCH] Check all checksum types

Dan McGee dpmcgee at gmail.com
Tue Jun 8 09:52:12 EDT 2010


On Tue, Jun 8, 2010 at 6:37 AM, Allan McRae <allan at archlinux.org> wrote:
> Check every checksum that makepkg supports rather than only md5sums.
> Fixes FS#17168.
>
> Improved by: Kazuo Teramoto <kaz.rag at gmail.com>
> Signed-off-by: Allan McRae <allan at 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 at 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 at 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


More information about the arch-dev-public mailing list