[pacman-dev] [PATCH 1/2] contrib: Add verify-pacman-repo-db.pl
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- contrib/doc/verify-pacman-repo-db.1.txt | 50 ++++++ contrib/verify-pacman-repo-db.pl | 279 ++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+) create mode 100644 contrib/doc/verify-pacman-repo-db.1.txt create mode 100755 contrib/verify-pacman-repo-db.pl diff --git a/contrib/doc/verify-pacman-repo-db.1.txt b/contrib/doc/verify-pacman-repo-db.1.txt new file mode 100644 index 0000000..01eafbe --- /dev/null +++ b/contrib/doc/verify-pacman-repo-db.1.txt @@ -0,0 +1,50 @@ +///// +vim:set ts=4 sw=4 syntax=asciidoc noet spell spelllang=en_us: +///// +verify-pacman-repo-db(1) +======================= + +Name +---- +verify-pacman-repo-db - package repository verification utility + + +Synopsis +-------- +'verify-pacman-repo-db' [options] + + +Description +----------- +'verify-pacman-repo-db' looks at a pacman repo database and verifies its +content with the actual package files. The database is expected to be in +the same directory as the packages (or symlinks to the packages). + +The following properties are verified for each package in the database: + + - existence of the package file + - file size + - MD5 and SHA256 checksum (--checksum) + +Options +------- +*-h, \--help*:: + Output a short help message. + +*\--debug*:: + Enable debug output. + +*-c, \--checksum*:: + Verify checksums of packages. Note that this means all packages files will + be read from disk. Otherwise only metadata is compared which does not + require to read package file contents. + +*-t, \--threads* <number>:: + Use 'number' threads to verify packages. Note that each thread may use up + to approximately 128MiB of memory. Default: 1 + +See Also +-------- +linkman:repo-add[8] + +include::footer.txt[] diff --git a/contrib/verify-pacman-repo-db.pl b/contrib/verify-pacman-repo-db.pl new file mode 100755 index 0000000..71cfddf --- /dev/null +++ b/contrib/verify-pacman-repo-db.pl @@ -0,0 +1,279 @@ +#!/usr/bin/perl -T +use warnings; +use strict; + +=pod + +=head1 SYNOPSIS + +verify-pacman-repo-db.pl [options] <database file> ... + + Options: + --help, -h Show short help message + --debug Enable debug output + --checksum, -c Verify checksums of packages + --thread, -t <num> Use num threads to verify packages. Default: 1 + NOTE: Each thread uses up to approx. 128MiB of memory + +=head1 DESCRIPTION + +verify-pacman-repo-db.pl looks at a pacman repo database and verifies its content +with the actual package files. The database is expected to be in the same +directory as the packages (or symlinks to the packages). + +The following properties are verified for each package in the database: + + - existence of the package file + - file size + - MD5 and SHA256 checksum (--checksum) + +=head1 NOTES + +This script does intentionally not use any ALPM libraries. The format is simple +enough to be parsed and this way we might just detect more problems because the +libalpm parsing code might also have bugs. We also stay much more portable +which might be good for people that want to check a db, but don't actually have +pacman installed. + +=cut + +package main; +use Getopt::Long; +use Pod::Usage; + +exit main(); + +sub main { + my %opts = ( + threads => 1, + ); + + Getopt::Long::Configure ("bundling"); + pod2usage(-verbose => 0) if (@ARGV== 0); + GetOptions(\%opts, "help|h", "debug", "threads|t=i", "checksum|c") or pod2usage(2); + pod2usage(0) if $opts{help}; + + my $verifier = Verifier->new(\%opts); + + for my $repodb (@ARGV) { + $verifier->check_repodb($repodb); + } + + $verifier->finalize(); + return $verifier->get_error_status(); +} + +package Verifier; +use Archive::Tar; +use Digest::MD5; +use Digest::SHA; +use File::Basename; +use threads; +use threads::shared; +use Thread::Queue; + +sub new { + my $class = shift; + my $opts = shift; + + my $self :shared = shared_clone({ + opts => \%{$opts}, + package_queue => Thread::Queue->new(), + output_queue => Thread::Queue->new(), + workers => [], + errors => 0, + }); + + bless $self, $class; + $self->start_workers(); + return $self; +} + +sub start_workers { + my $self = shift; + + threads->new(\&_worker_output_queue, $self); + + for (my $i = 0; $i < $self->{opts}->{threads}; $i++) { + my $thr :shared = shared_clone(threads->new(\&_worker_package_queue, $self)); + push @{$self->{workers}}, $thr; + } +} + +sub _worker_package_queue { + my $self = shift; + while (my $workpack = $self->{package_queue}->dequeue()) { + my $dbdata = $self->_parse_db_entry($workpack->{db_desc_content}); + $self->{errors} += $self->_verify_db_entry($workpack->{dirname}, $dbdata); + } +} + +sub _worker_output_queue { + my $self = shift; + while (my $output = $self->{output_queue}->dequeue()) { + print STDERR $output; + } +} + +sub finalize { + my $self = shift; + + $self->{package_queue}->end(); + $self->_join_threads($self->{workers}); + + $self->{output_queue}->end(); + $self->_join_threads([threads->list]); +} + +sub _join_threads { + my $self = shift; + my $threads = shift; + + for my $thr (@{$threads}) { + if ($thr->tid && !threads::equal($thr, threads->self)) { + print "waiting for thread ".$thr->tid()." to finish\n" if $self->{opts}->{debug}; + $thr->join; + } + } +} + +sub get_error_status { + my $self = shift; + + return $self->{errors} > 0; +} + +sub check_repodb { + my $self = shift; + my $repodb = shift; + + my $db = Archive::Tar->new(); + $db->read($repodb); + + my $dirname = dirname($repodb); + my $pkgcount = 0; + + my @files = $db->list_files(); + for my $file_object ($db->get_files()) { + if ($file_object->name =~ m/^([^\/]+)\/desc$/) { + my $package = $1; + $self->{package_queue}->enqueue({ + package => $package, + db_desc_content => $file_object->get_content(), + dirname => $dirname, + }); + $pkgcount++; + } + } + + $self->_debug(sprintf("Queued %d package from database '%s'\n", $pkgcount, $repodb)); +} + +sub _parse_db_entry { + my $self = shift; + my $content = shift; + my %db; + my $key; + + for my $line (split /\n/, $content) { + if ($line eq '') { + $key = undef; + next; + } + if ($line =~ m/^%(.+)%$/) { + $key = $1; + } else { + push @{$db{$key}}, $line; + die "\$key not set. Is the db formated incorrectly?" unless $key; + } + } + return \%db; +} + +sub _output { + my $self = shift; + my $output = shift; + + return if $output eq ""; + + $output = sprintf("Thread %s: %s", threads->self->tid(), $output); + $self->{output_queue}->enqueue($output); +} + +sub _debug { + my $self = shift; + my $output = shift; + $self->_output($output) if $self->{opts}->{debug}; +} + +sub _verify_db_entry { + my $self = shift; + my $basedir = shift; + my $dbdata = shift; + my $ret = 0; + my $output = ""; + + # verify package exists + my $pkgfile = $basedir.'/'.$dbdata->{FILENAME}[0]; + $self->_debug(sprintf("Checking package %s\n", $dbdata->{FILENAME}[0])); + unless (-e $pkgfile) { + $self->_output(sprintf("Package file missing: %s\n", $pkgfile)); + return 1; + } + + $ret += $self->_verify_package_size($dbdata, $pkgfile); + $ret += $self->_verify_package_checksum($dbdata, $pkgfile) if $self->{opts}->{checksum}; + # TODO verify gpg sigs? + + return $ret; +} + +sub _verify_package_size { + my $self = shift; + my $dbdata = shift; + my $pkgfile = shift; + + my $csize = $dbdata->{CSIZE}[0]; + my $filesize = (stat($pkgfile))[7]; + unless ($csize == $filesize) { + $self->_output(sprintf("Package file has incorrect size: %d vs %d: %s\n", $csize, $filesize, $pkgfile)); + return 1; + } + return 0; +} + +sub _verify_package_checksum { + my $self = shift; + my $dbdata = shift; + my $pkgfile = shift; + + my $md5 = Digest::MD5->new; + my $sha = Digest::SHA->new(256); + + my $content; + # 128MiB to keep random IO low when using multiple threads (only works for large packages though) + my $chunksize = 1024*1024*128; + open my $fh, "<", $pkgfile; + while (read($fh, $content, $chunksize)) { + $md5->add($content); + $sha->add($content); + } + + my $expected_sha = $dbdata->{SHA256SUM}[0]; + my $expected_md5 = $dbdata->{MD5SUM}[0]; + my $got_md5 = $md5->hexdigest; + my $got_sha = $sha->hexdigest; + + unless ($expected_sha eq $got_sha and $expected_md5 eq $got_md5) { + my $output; + $output .= sprintf "Package file has incorrect checksum: %s\n", $pkgfile; + $output .= sprintf "expected: SHA %s\n", $expected_sha; + $output .= sprintf "got: SHA %s\n", $got_sha; + $output .= sprintf "expected: MD5 %s\n", $expected_md5; + $output .= sprintf "got: MD5 %s\n", $got_md5; + $self->_output($output); + return 1; + } + return 0; +} + -- 2.9.0
Makefile.am is mostly copied from ./doc/Makefile.am Signed-off-by: Florian Pritz <bluewind@xinu.at> --- configure.ac | 1 + contrib/Makefile.am | 6 +++++ contrib/doc/.gitignore | 1 + contrib/doc/Makefile.am | 57 +++++++++++++++++++++++++++++++++++++++++++++++ contrib/doc/asciidoc.conf | 1 + contrib/doc/footer.txt | 1 + 6 files changed, 67 insertions(+) create mode 100644 contrib/doc/.gitignore create mode 100644 contrib/doc/Makefile.am create mode 120000 contrib/doc/asciidoc.conf create mode 120000 contrib/doc/footer.txt diff --git a/configure.ac b/configure.ac index dd4ac04..c20d9ba 100644 --- a/configure.ac +++ b/configure.ac @@ -520,6 +520,7 @@ test/pacman/tests/Makefile test/scripts/Makefile test/util/Makefile contrib/Makefile +contrib/doc/Makefile Makefile ]) AC_OUTPUT diff --git a/contrib/Makefile.am b/contrib/Makefile.am index e34b43e..897cd85 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -1,5 +1,11 @@ # enforce that all scripts have a --help and --version option AUTOMAKE_OPTIONS = std-options +SUBDIRS= +if WANT_DOC +SUBDIRS += doc +endif + +DIST_SUBDIRS = $(SUBDIRS) bin_SCRIPTS = \ $(OURSCRIPTS) diff --git a/contrib/doc/.gitignore b/contrib/doc/.gitignore new file mode 100644 index 0000000..c5612bc --- /dev/null +++ b/contrib/doc/.gitignore @@ -0,0 +1 @@ +verify-pacman-repo-db.1 diff --git a/contrib/doc/Makefile.am b/contrib/doc/Makefile.am new file mode 100644 index 0000000..df132d8 --- /dev/null +++ b/contrib/doc/Makefile.am @@ -0,0 +1,57 @@ +# We have to do some funny stuff here with the manpages. In order to ensure +# a dist tarball doesn't get put out there without manpages, we keep those +# files listed in EXTRA_DIST no matter what. However, we only add them to +# man_MANS if --enable-asciidoc and/or --enable-doxygen are used. + +ASCIIDOC_MANS = \ + verify-pacman-repo-db.1 + +EXTRA_DIST = \ + asciidoc.conf \ + footer.txt \ + verify-pacman-repo-db.1.txt \ + $(ASCIIDOC_MANS) + +# Files that should be removed, but which Automake does not know. +MOSTLYCLEANFILES = $(ASCIIDOC_MANS) + +# Ensure manpages are fresh when building a dist tarball +dist-hook: + $(MAKE) $(AM_MAKEFLAGS) clean + $(MAKE) $(AM_MAKEFLAGS) all + +if USE_GIT_VERSION +GIT_VERSION := $(shell sh -c 'git describe --abbrev=4 --dirty | sed s/^v//') +REAL_PACKAGE_VERSION = $(GIT_VERSION) +else +REAL_PACKAGE_VERSION = $(PACKAGE_VERSION) +endif + +man_MANS = +dist_man_MANS = $(ASCIIDOC_MANS) + +pkgdatadir = ${datadir}/${PACKAGE} + +ASCIIDOC_OPTS = \ + -f $(srcdir)/asciidoc.conf \ + -a pacman_version="$(REAL_PACKAGE_VERSION)" \ + -a pacman_date="`date +%Y-%m-%d`" \ + -a pkgdatadir=$(pkgdatadir) \ + -a localstatedir=$(localstatedir) \ + -a sysconfdir=$(sysconfdir) \ + -a datarootdir=$(datarootdir) + +A2X_OPTS = \ + --no-xmllint \ + -d manpage \ + -f manpage \ + --xsltproc-opts='-param man.endnotes.list.enabled 0 -param man.endnotes.are.numbered 0' + +# These rules are due to the includes and files of the asciidoc text +$(ASCIIDOC_MANS): asciidoc.conf footer.txt Makefile.am + $(AM_V_GEN)a2x $(A2X_OPTS) --asciidoc-opts="$(ASCIIDOC_OPTS) --out-file=./$@" $(srcdir)/$@.txt + +# Dependency rules +verify-pacman-repo-db.1: verify-pacman-repo-db.1.txt + +# vim:set noet: diff --git a/contrib/doc/asciidoc.conf b/contrib/doc/asciidoc.conf new file mode 120000 index 0000000..ff9653d --- /dev/null +++ b/contrib/doc/asciidoc.conf @@ -0,0 +1 @@ +../../doc/asciidoc.conf \ No newline at end of file diff --git a/contrib/doc/footer.txt b/contrib/doc/footer.txt new file mode 120000 index 0000000..9dd4bae --- /dev/null +++ b/contrib/doc/footer.txt @@ -0,0 +1 @@ +../../doc/footer.txt \ No newline at end of file -- 2.9.0
On 19/07/16 00:28, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at>
A commit message would be nice... Is there any reason PGP checksums are not checked?
--- contrib/doc/verify-pacman-repo-db.1.txt | 50 ++++++ contrib/verify-pacman-repo-db.pl | 279 ++++++++++++++++++++++++++++++++ 2 files changed, 329 insertions(+) create mode 100644 contrib/doc/verify-pacman-repo-db.1.txt create mode 100755 contrib/verify-pacman-repo-db.pl
diff --git a/contrib/doc/verify-pacman-repo-db.1.txt b/contrib/doc/verify-pacman-repo-db.1.txt new file mode 100644 index 0000000..01eafbe --- /dev/null +++ b/contrib/doc/verify-pacman-repo-db.1.txt @@ -0,0 +1,50 @@ +///// +vim:set ts=4 sw=4 syntax=asciidoc noet spell spelllang=en_us: +///// +verify-pacman-repo-db(1) +======================= + +Name +---- +verify-pacman-repo-db - package repository verification utility + + +Synopsis +-------- +'verify-pacman-repo-db' [options] + + +Description +----------- +'verify-pacman-repo-db' looks at a pacman repo database and verifies its +content with the actual package files. The database is expected to be in +the same directory as the packages (or symlinks to the packages). + +The following properties are verified for each package in the database: + + - existence of the package file + - file size + - MD5 and SHA256 checksum (--checksum) + +Options +------- +*-h, \--help*:: + Output a short help message. + +*\--debug*:: + Enable debug output. + +*-c, \--checksum*:: + Verify checksums of packages. Note that this means all packages files will + be read from disk. Otherwise only metadata is compared which does not + require to read package file contents. + +*-t, \--threads* <number>:: + Use 'number' threads to verify packages. Note that each thread may use up + to approximately 128MiB of memory. Default: 1 + +See Also +-------- +linkman:repo-add[8] + +include::footer.txt[] diff --git a/contrib/verify-pacman-repo-db.pl b/contrib/verify-pacman-repo-db.pl new file mode 100755 index 0000000..71cfddf --- /dev/null +++ b/contrib/verify-pacman-repo-db.pl @@ -0,0 +1,279 @@ +#!/usr/bin/perl -T +use warnings; +use strict; + +=pod + +=head1 SYNOPSIS + +verify-pacman-repo-db.pl [options] <database file> ... + + Options: + --help, -h Show short help message + --debug Enable debug output + --checksum, -c Verify checksums of packages + --thread, -t <num> Use num threads to verify packages. Default: 1 + NOTE: Each thread uses up to approx. 128MiB of memory + +=head1 DESCRIPTION + +verify-pacman-repo-db.pl looks at a pacman repo database and verifies its content +with the actual package files. The database is expected to be in the same +directory as the packages (or symlinks to the packages). + +The following properties are verified for each package in the database: + + - existence of the package file + - file size + - MD5 and SHA256 checksum (--checksum) + +=head1 NOTES + +This script does intentionally not use any ALPM libraries. The format is simple +enough to be parsed and this way we might just detect more problems because the +libalpm parsing code might also have bugs. We also stay much more portable +which might be good for people that want to check a db, but don't actually have +pacman installed. + +=cut + +package main; +use Getopt::Long; +use Pod::Usage; + +exit main(); + +sub main { + my %opts = ( + threads => 1, + ); + + Getopt::Long::Configure ("bundling"); + pod2usage(-verbose => 0) if (@ARGV== 0); + GetOptions(\%opts, "help|h", "debug", "threads|t=i", "checksum|c") or pod2usage(2); + pod2usage(0) if $opts{help}; + + my $verifier = Verifier->new(\%opts); + + for my $repodb (@ARGV) { + $verifier->check_repodb($repodb); + } + + $verifier->finalize(); + return $verifier->get_error_status(); +} + +package Verifier; +use Archive::Tar; +use Digest::MD5; +use Digest::SHA; +use File::Basename; +use threads; +use threads::shared; +use Thread::Queue; + +sub new { + my $class = shift; + my $opts = shift; + + my $self :shared = shared_clone({ + opts => \%{$opts}, + package_queue => Thread::Queue->new(), + output_queue => Thread::Queue->new(), + workers => [], + errors => 0, + }); + + bless $self, $class; + $self->start_workers(); + return $self; +} + +sub start_workers { + my $self = shift; + + threads->new(\&_worker_output_queue, $self); + + for (my $i = 0; $i < $self->{opts}->{threads}; $i++) { + my $thr :shared = shared_clone(threads->new(\&_worker_package_queue, $self)); + push @{$self->{workers}}, $thr; + } +} + +sub _worker_package_queue { + my $self = shift; + while (my $workpack = $self->{package_queue}->dequeue()) { + my $dbdata = $self->_parse_db_entry($workpack->{db_desc_content}); + $self->{errors} += $self->_verify_db_entry($workpack->{dirname}, $dbdata); + } +} + +sub _worker_output_queue { + my $self = shift; + while (my $output = $self->{output_queue}->dequeue()) { + print STDERR $output; + } +} + +sub finalize { + my $self = shift; + + $self->{package_queue}->end(); + $self->_join_threads($self->{workers}); + + $self->{output_queue}->end(); + $self->_join_threads([threads->list]); +} + +sub _join_threads { + my $self = shift; + my $threads = shift; + + for my $thr (@{$threads}) { + if ($thr->tid && !threads::equal($thr, threads->self)) { + print "waiting for thread ".$thr->tid()." to finish\n" if $self->{opts}->{debug}; + $thr->join; + } + } +} + +sub get_error_status { + my $self = shift; + + return $self->{errors} > 0; +} + +sub check_repodb { + my $self = shift; + my $repodb = shift; + + my $db = Archive::Tar->new(); + $db->read($repodb); + + my $dirname = dirname($repodb); + my $pkgcount = 0; + + my @files = $db->list_files(); + for my $file_object ($db->get_files()) { + if ($file_object->name =~ m/^([^\/]+)\/desc$/) { + my $package = $1; + $self->{package_queue}->enqueue({ + package => $package, + db_desc_content => $file_object->get_content(), + dirname => $dirname, + }); + $pkgcount++; + } + } + + $self->_debug(sprintf("Queued %d package from database '%s'\n", $pkgcount, $repodb)); +} + +sub _parse_db_entry { + my $self = shift; + my $content = shift; + my %db; + my $key; + + for my $line (split /\n/, $content) { + if ($line eq '') { + $key = undef; + next; + } + if ($line =~ m/^%(.+)%$/) { + $key = $1; + } else { + push @{$db{$key}}, $line; + die "\$key not set. Is the db formated incorrectly?" unless $key; + } + } + return \%db; +} + +sub _output { + my $self = shift; + my $output = shift; + + return if $output eq ""; + + $output = sprintf("Thread %s: %s", threads->self->tid(), $output); + $self->{output_queue}->enqueue($output); +} + +sub _debug { + my $self = shift; + my $output = shift; + $self->_output($output) if $self->{opts}->{debug}; +} + +sub _verify_db_entry { + my $self = shift; + my $basedir = shift; + my $dbdata = shift; + my $ret = 0; + my $output = ""; + + # verify package exists + my $pkgfile = $basedir.'/'.$dbdata->{FILENAME}[0]; + $self->_debug(sprintf("Checking package %s\n", $dbdata->{FILENAME}[0])); + unless (-e $pkgfile) { + $self->_output(sprintf("Package file missing: %s\n", $pkgfile)); + return 1; + } + + $ret += $self->_verify_package_size($dbdata, $pkgfile); + $ret += $self->_verify_package_checksum($dbdata, $pkgfile) if $self->{opts}->{checksum}; + # TODO verify gpg sigs? + + return $ret; +} + +sub _verify_package_size { + my $self = shift; + my $dbdata = shift; + my $pkgfile = shift; + + my $csize = $dbdata->{CSIZE}[0]; + my $filesize = (stat($pkgfile))[7]; + unless ($csize == $filesize) { + $self->_output(sprintf("Package file has incorrect size: %d vs %d: %s\n", $csize, $filesize, $pkgfile)); + return 1; + } + return 0; +} + +sub _verify_package_checksum { + my $self = shift; + my $dbdata = shift; + my $pkgfile = shift; + + my $md5 = Digest::MD5->new; + my $sha = Digest::SHA->new(256); + + my $content; + # 128MiB to keep random IO low when using multiple threads (only works for large packages though) + my $chunksize = 1024*1024*128; + open my $fh, "<", $pkgfile; + while (read($fh, $content, $chunksize)) { + $md5->add($content); + $sha->add($content); + } + + my $expected_sha = $dbdata->{SHA256SUM}[0]; + my $expected_md5 = $dbdata->{MD5SUM}[0]; + my $got_md5 = $md5->hexdigest; + my $got_sha = $sha->hexdigest; + + unless ($expected_sha eq $got_sha and $expected_md5 eq $got_md5) { + my $output; + $output .= sprintf "Package file has incorrect checksum: %s\n", $pkgfile; + $output .= sprintf "expected: SHA %s\n", $expected_sha; + $output .= sprintf "got: SHA %s\n", $got_sha; + $output .= sprintf "expected: MD5 %s\n", $expected_md5; + $output .= sprintf "got: MD5 %s\n", $got_md5; + $self->_output($output); + return 1; + } + return 0; +} +
On 07.08.2016 08:28, Allan McRae wrote:
A commit message would be nice...
Would a copy of the manpage description be fine or do you have something else in mind?
Is there any reason PGP checksums are not checked?
I don't see a mention of checksum-only verification in the gpg manpage so I'll assume you mean signatures here. The main reason is that I'm not sure if it is really necessary. If we want to catch obvious problems (missing or broken package file), checking the sha256 and md5 hashes is enough. PGP opens a whole can of worms starting with the simple issue that this script should also be useful to mirror admins that want to check if their mirror is good. Those servers may not run the distro for which they provide a mirror and they probably don't have the keys in their keyring so verifying the signatures is not easily possible. I currently don't consider the feature worth adding, but I haven't thought about it too much, which is why the TODO has a question mark at the end. If you want, I can remove that line entirely given I've thought about it some more now and still don't see a huge value in having it. Florian
On 07/08/16 23:13, Florian Pritz wrote:
On 07.08.2016 08:28, Allan McRae wrote:
A commit message would be nice...
Would a copy of the manpage description be fine or do you have something else in mind?
Is there any reason PGP checksums are not checked?
I don't see a mention of checksum-only verification in the gpg manpage so I'll assume you mean signatures here.
The main reason is that I'm not sure if it is really necessary. If we want to catch obvious problems (missing or broken package file), checking the sha256 and md5 hashes is enough. PGP opens a whole can of worms starting with the simple issue that this script should also be useful to mirror admins that want to check if their mirror is good. Those servers may not run the distro for which they provide a mirror and they probably don't have the keys in their keyring so verifying the signatures is not easily possible.
I currently don't consider the feature worth adding, but I haven't thought about it too much, which is why the TODO has a question mark at the end. If you want, I can remove that line entirely given I've thought about it some more now and still don't see a huge value in having it.
I didn't actually see the TODO - I was purely commenting based on the man page. Get rid of the TODO, and put a very brief description in the commit message and I will apply. (I am assuming it is tested due to not knowing perl that well...) A
From the documentation:
verify-pacman-repo-db looks at a pacman repo database and verifies its content with the actual package files. The database is expected to be in the same directory as the packages (or symlinks to the packages). The following properties are verified for each package in the database: - existence of the package file - file size - MD5 and SHA256 checksum (--checksum) Signed-off-by: Florian Pritz <bluewind@xinu.at> --- v2: - Remove GPG TODO - Add commit message - Remove duplicate documentation from script - Move actual documentation to dedicated commit contrib/verify-pacman-repo-db.pl | 260 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 260 insertions(+) create mode 100755 contrib/verify-pacman-repo-db.pl diff --git a/contrib/verify-pacman-repo-db.pl b/contrib/verify-pacman-repo-db.pl new file mode 100755 index 0000000..1d02c26 --- /dev/null +++ b/contrib/verify-pacman-repo-db.pl @@ -0,0 +1,260 @@ +#!/usr/bin/perl -T +use warnings; +use strict; + + +# This is used for the usage output +=pod + +=head1 SYNOPSIS + +verify-pacman-repo-db.pl [options] <database file> ... + + Options: + --help, -h Show short help message + --debug Enable debug output + --checksum, -c Verify checksums of packages + --thread, -t <num> Use num threads to verify packages. Default: 1 + NOTE: Each thread uses up to approx. 128MiB of memory + +=cut + +package main; +use Getopt::Long; +use Pod::Usage; + +exit main(); + +sub main { + my %opts = ( + threads => 1, + ); + + Getopt::Long::Configure ("bundling"); + pod2usage(-verbose => 0) if (@ARGV== 0); + GetOptions(\%opts, "help|h", "debug", "threads|t=i", "checksum|c") or pod2usage(2); + pod2usage(0) if $opts{help}; + + my $verifier = Verifier->new(\%opts); + + for my $repodb (@ARGV) { + $verifier->check_repodb($repodb); + } + + $verifier->finalize(); + return $verifier->get_error_status(); +} + +package Verifier; +use Archive::Tar; +use Digest::MD5; +use Digest::SHA; +use File::Basename; +use threads; +use threads::shared; +use Thread::Queue; + +sub new { + my $class = shift; + my $opts = shift; + + my $self :shared = shared_clone({ + opts => \%{$opts}, + package_queue => Thread::Queue->new(), + output_queue => Thread::Queue->new(), + workers => [], + errors => 0, + }); + + bless $self, $class; + $self->start_workers(); + return $self; +} + +sub start_workers { + my $self = shift; + + threads->new(\&_worker_output_queue, $self); + + for (my $i = 0; $i < $self->{opts}->{threads}; $i++) { + my $thr :shared = shared_clone(threads->new(\&_worker_package_queue, $self)); + push @{$self->{workers}}, $thr; + } +} + +sub _worker_package_queue { + my $self = shift; + while (my $workpack = $self->{package_queue}->dequeue()) { + my $dbdata = $self->_parse_db_entry($workpack->{db_desc_content}); + $self->{errors} += $self->_verify_db_entry($workpack->{dirname}, $dbdata); + } +} + +sub _worker_output_queue { + my $self = shift; + while (my $output = $self->{output_queue}->dequeue()) { + print STDERR $output; + } +} + +sub finalize { + my $self = shift; + + $self->{package_queue}->end(); + $self->_join_threads($self->{workers}); + + $self->{output_queue}->end(); + $self->_join_threads([threads->list]); +} + +sub _join_threads { + my $self = shift; + my $threads = shift; + + for my $thr (@{$threads}) { + if ($thr->tid && !threads::equal($thr, threads->self)) { + print "waiting for thread ".$thr->tid()." to finish\n" if $self->{opts}->{debug}; + $thr->join; + } + } +} + +sub get_error_status { + my $self = shift; + + return $self->{errors} > 0; +} + +sub check_repodb { + my $self = shift; + my $repodb = shift; + + my $db = Archive::Tar->new(); + $db->read($repodb); + + my $dirname = dirname($repodb); + my $pkgcount = 0; + + my @files = $db->list_files(); + for my $file_object ($db->get_files()) { + if ($file_object->name =~ m/^([^\/]+)\/desc$/) { + my $package = $1; + $self->{package_queue}->enqueue({ + package => $package, + db_desc_content => $file_object->get_content(), + dirname => $dirname, + }); + $pkgcount++; + } + } + + $self->_debug(sprintf("Queued %d package from database '%s'\n", $pkgcount, $repodb)); +} + +sub _parse_db_entry { + my $self = shift; + my $content = shift; + my %db; + my $key; + + for my $line (split /\n/, $content) { + if ($line eq '') { + $key = undef; + next; + } + if ($line =~ m/^%(.+)%$/) { + $key = $1; + } else { + push @{$db{$key}}, $line; + die "\$key not set. Is the db formated incorrectly?" unless $key; + } + } + return \%db; +} + +sub _output { + my $self = shift; + my $output = shift; + + return if $output eq ""; + + $output = sprintf("Thread %s: %s", threads->self->tid(), $output); + $self->{output_queue}->enqueue($output); +} + +sub _debug { + my $self = shift; + my $output = shift; + $self->_output($output) if $self->{opts}->{debug}; +} + +sub _verify_db_entry { + my $self = shift; + my $basedir = shift; + my $dbdata = shift; + my $ret = 0; + my $output = ""; + + # verify package exists + my $pkgfile = $basedir.'/'.$dbdata->{FILENAME}[0]; + $self->_debug(sprintf("Checking package %s\n", $dbdata->{FILENAME}[0])); + unless (-e $pkgfile) { + $self->_output(sprintf("Package file missing: %s\n", $pkgfile)); + return 1; + } + + $ret += $self->_verify_package_size($dbdata, $pkgfile); + $ret += $self->_verify_package_checksum($dbdata, $pkgfile) if $self->{opts}->{checksum}; + + return $ret; +} + +sub _verify_package_size { + my $self = shift; + my $dbdata = shift; + my $pkgfile = shift; + + my $csize = $dbdata->{CSIZE}[0]; + my $filesize = (stat($pkgfile))[7]; + unless ($csize == $filesize) { + $self->_output(sprintf("Package file has incorrect size: %d vs %d: %s\n", $csize, $filesize, $pkgfile)); + return 1; + } + return 0; +} + +sub _verify_package_checksum { + my $self = shift; + my $dbdata = shift; + my $pkgfile = shift; + + my $md5 = Digest::MD5->new; + my $sha = Digest::SHA->new(256); + + my $content; + # 128MiB to keep random IO low when using multiple threads (only works for large packages though) + my $chunksize = 1024*1024*128; + open my $fh, "<", $pkgfile; + while (read($fh, $content, $chunksize)) { + $md5->add($content); + $sha->add($content); + } + + my $expected_sha = $dbdata->{SHA256SUM}[0]; + my $expected_md5 = $dbdata->{MD5SUM}[0]; + my $got_md5 = $md5->hexdigest; + my $got_sha = $sha->hexdigest; + + unless ($expected_sha eq $got_sha and $expected_md5 eq $got_md5) { + my $output; + $output .= sprintf "Package file has incorrect checksum: %s\n", $pkgfile; + $output .= sprintf "expected: SHA %s\n", $expected_sha; + $output .= sprintf "got: SHA %s\n", $got_sha; + $output .= sprintf "expected: MD5 %s\n", $expected_md5; + $output .= sprintf "got: MD5 %s\n", $got_md5; + $self->_output($output); + return 1; + } + return 0; +} + -- 2.9.0
Makefile.am is mostly copied from ./doc/Makefile.am Signed-off-by: Florian Pritz <bluewind@xinu.at> --- v2: - Move actual documentation to dedicated commit configure.ac | 1 + contrib/Makefile.am | 6 ++++++ contrib/doc/Makefile.am | 54 +++++++++++++++++++++++++++++++++++++++++++++++ contrib/doc/asciidoc.conf | 1 + contrib/doc/footer.txt | 1 + 5 files changed, 63 insertions(+) create mode 100644 contrib/doc/Makefile.am create mode 120000 contrib/doc/asciidoc.conf create mode 120000 contrib/doc/footer.txt diff --git a/configure.ac b/configure.ac index dd4ac04..c20d9ba 100644 --- a/configure.ac +++ b/configure.ac @@ -520,6 +520,7 @@ test/pacman/tests/Makefile test/scripts/Makefile test/util/Makefile contrib/Makefile +contrib/doc/Makefile Makefile ]) AC_OUTPUT diff --git a/contrib/Makefile.am b/contrib/Makefile.am index e34b43e..897cd85 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -1,5 +1,11 @@ # enforce that all scripts have a --help and --version option AUTOMAKE_OPTIONS = std-options +SUBDIRS= +if WANT_DOC +SUBDIRS += doc +endif + +DIST_SUBDIRS = $(SUBDIRS) bin_SCRIPTS = \ $(OURSCRIPTS) diff --git a/contrib/doc/Makefile.am b/contrib/doc/Makefile.am new file mode 100644 index 0000000..7060270 --- /dev/null +++ b/contrib/doc/Makefile.am @@ -0,0 +1,54 @@ +# We have to do some funny stuff here with the manpages. In order to ensure +# a dist tarball doesn't get put out there without manpages, we keep those +# files listed in EXTRA_DIST no matter what. However, we only add them to +# man_MANS if --enable-asciidoc and/or --enable-doxygen are used. + +ASCIIDOC_MANS = + +EXTRA_DIST = \ + asciidoc.conf \ + footer.txt \ + $(ASCIIDOC_MANS) + +# Files that should be removed, but which Automake does not know. +MOSTLYCLEANFILES = $(ASCIIDOC_MANS) + +# Ensure manpages are fresh when building a dist tarball +dist-hook: + $(MAKE) $(AM_MAKEFLAGS) clean + $(MAKE) $(AM_MAKEFLAGS) all + +if USE_GIT_VERSION +GIT_VERSION := $(shell sh -c 'git describe --abbrev=4 --dirty | sed s/^v//') +REAL_PACKAGE_VERSION = $(GIT_VERSION) +else +REAL_PACKAGE_VERSION = $(PACKAGE_VERSION) +endif + +man_MANS = +dist_man_MANS = $(ASCIIDOC_MANS) + +pkgdatadir = ${datadir}/${PACKAGE} + +ASCIIDOC_OPTS = \ + -f $(srcdir)/asciidoc.conf \ + -a pacman_version="$(REAL_PACKAGE_VERSION)" \ + -a pacman_date="`date +%Y-%m-%d`" \ + -a pkgdatadir=$(pkgdatadir) \ + -a localstatedir=$(localstatedir) \ + -a sysconfdir=$(sysconfdir) \ + -a datarootdir=$(datarootdir) + +A2X_OPTS = \ + --no-xmllint \ + -d manpage \ + -f manpage \ + --xsltproc-opts='-param man.endnotes.list.enabled 0 -param man.endnotes.are.numbered 0' + +# These rules are due to the includes and files of the asciidoc text +$(ASCIIDOC_MANS): asciidoc.conf footer.txt Makefile.am + $(AM_V_GEN)a2x $(A2X_OPTS) --asciidoc-opts="$(ASCIIDOC_OPTS) --out-file=./$@" $(srcdir)/$@.txt + +# Dependency rules + +# vim:set noet: diff --git a/contrib/doc/asciidoc.conf b/contrib/doc/asciidoc.conf new file mode 120000 index 0000000..ff9653d --- /dev/null +++ b/contrib/doc/asciidoc.conf @@ -0,0 +1 @@ +../../doc/asciidoc.conf \ No newline at end of file diff --git a/contrib/doc/footer.txt b/contrib/doc/footer.txt new file mode 120000 index 0000000..9dd4bae --- /dev/null +++ b/contrib/doc/footer.txt @@ -0,0 +1 @@ +../../doc/footer.txt \ No newline at end of file -- 2.9.0
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- v2: - new commit contrib/doc/.gitignore | 1 + contrib/doc/Makefile.am | 5 ++- contrib/doc/verify-pacman-repo-db.1.txt | 60 +++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 contrib/doc/.gitignore create mode 100644 contrib/doc/verify-pacman-repo-db.1.txt diff --git a/contrib/doc/.gitignore b/contrib/doc/.gitignore new file mode 100644 index 0000000..c5612bc --- /dev/null +++ b/contrib/doc/.gitignore @@ -0,0 +1 @@ +verify-pacman-repo-db.1 diff --git a/contrib/doc/Makefile.am b/contrib/doc/Makefile.am index 7060270..df132d8 100644 --- a/contrib/doc/Makefile.am +++ b/contrib/doc/Makefile.am @@ -3,11 +3,13 @@ # files listed in EXTRA_DIST no matter what. However, we only add them to # man_MANS if --enable-asciidoc and/or --enable-doxygen are used. -ASCIIDOC_MANS = +ASCIIDOC_MANS = \ + verify-pacman-repo-db.1 EXTRA_DIST = \ asciidoc.conf \ footer.txt \ + verify-pacman-repo-db.1.txt \ $(ASCIIDOC_MANS) # Files that should be removed, but which Automake does not know. @@ -50,5 +52,6 @@ $(ASCIIDOC_MANS): asciidoc.conf footer.txt Makefile.am $(AM_V_GEN)a2x $(A2X_OPTS) --asciidoc-opts="$(ASCIIDOC_OPTS) --out-file=./$@" $(srcdir)/$@.txt # Dependency rules +verify-pacman-repo-db.1: verify-pacman-repo-db.1.txt # vim:set noet: diff --git a/contrib/doc/verify-pacman-repo-db.1.txt b/contrib/doc/verify-pacman-repo-db.1.txt new file mode 100644 index 0000000..fde45fd --- /dev/null +++ b/contrib/doc/verify-pacman-repo-db.1.txt @@ -0,0 +1,60 @@ +///// +vim:set ts=4 sw=4 syntax=asciidoc noet spell spelllang=en_us: +///// +verify-pacman-repo-db(1) +======================= + +Name +---- +verify-pacman-repo-db - package repository verification utility + + +Synopsis +-------- +'verify-pacman-repo-db' [options] + + +Description +----------- +'verify-pacman-repo-db' looks at a pacman repo database and verifies its +content with the actual package files. The database is expected to be in +the same directory as the packages (or symlinks to the packages). + +The following properties are verified for each package in the database: + + - existence of the package file + - file size + - MD5 and SHA256 checksum (--checksum) + +Options +------- +*-h, \--help*:: + Output a short help message. + +*\--debug*:: + Enable debug output. + +*-c, \--checksum*:: + Verify checksums of packages. Note that this means all packages files will + be read from disk. Otherwise only metadata is compared which does not + require to read package file contents. + +*-t, \--threads* <number>:: + Use 'number' threads to verify packages. Note that each thread may use up + to approximately 128MiB of memory. Default: 1 + +Notes +===== + +This script does intentionally not use any ALPM libraries. The format is simple +enough to be parsed and this way we might just detect more problems because the +libalpm parsing code might also have bugs. We also stay much more portable +which might be good for people that want to check a db, but don't actually have +pacman installed. + + +See Also +-------- +linkman:repo-add[8] + +include::footer.txt[] -- 2.9.0
On 08/07/16 at 03:46pm, Florian Pritz wrote:
From the documentation:
verify-pacman-repo-db looks at a pacman repo database and verifies its content with the actual package files. The database is expected to be in the same directory as the packages (or symlinks to the packages).
The following properties are verified for each package in the database:
- existence of the package file - file size - MD5 and SHA256 checksum (--checksum)
Signed-off-by: Florian Pritz <bluewind@xinu.at> ---
v2:
- Remove GPG TODO - Add commit message - Remove duplicate documentation from script - Move actual documentation to dedicated commit
contrib/verify-pacman-repo-db.pl | 260 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 260 insertions(+) create mode 100755 contrib/verify-pacman-repo-db.pl
diff --git a/contrib/verify-pacman-repo-db.pl b/contrib/verify-pacman-repo-db.pl new file mode 100755 index 0000000..1d02c26 --- /dev/null +++ b/contrib/verify-pacman-repo-db.pl @@ -0,0 +1,260 @@ +#!/usr/bin/perl -T +use warnings; +use strict; + + +# This is used for the usage output +=pod + +=head1 SYNOPSIS + +verify-pacman-repo-db.pl [options] <database file> ... + + Options: + --help, -h Show short help message + --debug Enable debug output + --checksum, -c Verify checksums of packages + --thread, -t <num> Use num threads to verify packages. Default: 1 + NOTE: Each thread uses up to approx. 128MiB of memory + +=cut + +package main; +use Getopt::Long; +use Pod::Usage; + +exit main(); + +sub main { + my %opts = ( + threads => 1, + ); + + Getopt::Long::Configure ("bundling"); + pod2usage(-verbose => 0) if (@ARGV== 0); + GetOptions(\%opts, "help|h", "debug", "threads|t=i", "checksum|c") or pod2usage(2); + pod2usage(0) if $opts{help}; + + my $verifier = Verifier->new(\%opts); + + for my $repodb (@ARGV) { + $verifier->check_repodb($repodb); + } + + $verifier->finalize(); + return $verifier->get_error_status(); +} + +package Verifier; +use Archive::Tar; +use Digest::MD5; +use Digest::SHA; +use File::Basename; +use threads; +use threads::shared; +use Thread::Queue; + +sub new { + my $class = shift; + my $opts = shift; + + my $self :shared = shared_clone({ + opts => \%{$opts}, + package_queue => Thread::Queue->new(), + output_queue => Thread::Queue->new(), + workers => [], + errors => 0, + }); + + bless $self, $class; + $self->start_workers(); + return $self; +} + +sub start_workers { + my $self = shift; + + threads->new(\&_worker_output_queue, $self); + + for (my $i = 0; $i < $self->{opts}->{threads}; $i++) { + my $thr :shared = shared_clone(threads->new(\&_worker_package_queue, $self)); + push @{$self->{workers}}, $thr; + } +} + +sub _worker_package_queue { + my $self = shift; + while (my $workpack = $self->{package_queue}->dequeue()) { + my $dbdata = $self->_parse_db_entry($workpack->{db_desc_content}); + $self->{errors} += $self->_verify_db_entry($workpack->{dirname}, $dbdata); + } +} + +sub _worker_output_queue { + my $self = shift; + while (my $output = $self->{output_queue}->dequeue()) { + print STDERR $output; + } +} + +sub finalize { + my $self = shift; + + $self->{package_queue}->end(); + $self->_join_threads($self->{workers}); + + $self->{output_queue}->end(); + $self->_join_threads([threads->list]); +} + +sub _join_threads { + my $self = shift; + my $threads = shift; + + for my $thr (@{$threads}) { + if ($thr->tid && !threads::equal($thr, threads->self)) { + print "waiting for thread ".$thr->tid()." to finish\n" if $self->{opts}->{debug}; + $thr->join; + } + } +} + +sub get_error_status { + my $self = shift; + + return $self->{errors} > 0; +} + +sub check_repodb { + my $self = shift; + my $repodb = shift; + + my $db = Archive::Tar->new(); + $db->read($repodb); + + my $dirname = dirname($repodb); + my $pkgcount = 0; + + my @files = $db->list_files(); + for my $file_object ($db->get_files()) { + if ($file_object->name =~ m/^([^\/]+)\/desc$/) { + my $package = $1; + $self->{package_queue}->enqueue({ + package => $package, + db_desc_content => $file_object->get_content(), + dirname => $dirname, + }); + $pkgcount++; + } + } + + $self->_debug(sprintf("Queued %d package from database '%s'\n", $pkgcount, $repodb)); +} + +sub _parse_db_entry { + my $self = shift; + my $content = shift; + my %db; + my $key; + + for my $line (split /\n/, $content) { + if ($line eq '') { + $key = undef; + next; + } + if ($line =~ m/^%(.+)%$/) { + $key = $1; + } else { + push @{$db{$key}}, $line; + die "\$key not set. Is the db formated incorrectly?" unless $key; + }
This if/else chain is wrong, values can match /^%.+%$/. It should be: if($line eq '') ... elsif($key)... elsif($line =~ /^%(.+)%$/) ... else ... Also, s/formated/formatted/.
+ } + return \%db; +} + +sub _output { + my $self = shift; + my $output = shift; + + return if $output eq ""; + + $output = sprintf("Thread %s: %s", threads->self->tid(), $output); + $self->{output_queue}->enqueue($output); +} + +sub _debug { + my $self = shift; + my $output = shift; + $self->_output($output) if $self->{opts}->{debug}; +} + +sub _verify_db_entry { + my $self = shift; + my $basedir = shift; + my $dbdata = shift; + my $ret = 0; + my $output = ""; + + # verify package exists + my $pkgfile = $basedir.'/'.$dbdata->{FILENAME}[0]; + $self->_debug(sprintf("Checking package %s\n", $dbdata->{FILENAME}[0])); + unless (-e $pkgfile) { + $self->_output(sprintf("Package file missing: %s\n", $pkgfile)); + return 1; + } + + $ret += $self->_verify_package_size($dbdata, $pkgfile); + $ret += $self->_verify_package_checksum($dbdata, $pkgfile) if $self->{opts}->{checksum}; + + return $ret; +} + +sub _verify_package_size { + my $self = shift; + my $dbdata = shift; + my $pkgfile = shift; + + my $csize = $dbdata->{CSIZE}[0]; + my $filesize = (stat($pkgfile))[7]; + unless ($csize == $filesize) { + $self->_output(sprintf("Package file has incorrect size: %d vs %d: %s\n", $csize, $filesize, $pkgfile)); + return 1; + } + return 0; +} + +sub _verify_package_checksum { + my $self = shift; + my $dbdata = shift; + my $pkgfile = shift; + + my $md5 = Digest::MD5->new; + my $sha = Digest::SHA->new(256); + + my $content; + # 128MiB to keep random IO low when using multiple threads (only works for large packages though) + my $chunksize = 1024*1024*128; + open my $fh, "<", $pkgfile; + while (read($fh, $content, $chunksize)) { + $md5->add($content); + $sha->add($content); + } + + my $expected_sha = $dbdata->{SHA256SUM}[0]; + my $expected_md5 = $dbdata->{MD5SUM}[0]; + my $got_md5 = $md5->hexdigest; + my $got_sha = $sha->hexdigest; + + unless ($expected_sha eq $got_sha and $expected_md5 eq $got_md5) { + my $output; + $output .= sprintf "Package file has incorrect checksum: %s\n", $pkgfile; + $output .= sprintf "expected: SHA %s\n", $expected_sha; + $output .= sprintf "got: SHA %s\n", $got_sha; + $output .= sprintf "expected: MD5 %s\n", $expected_md5; + $output .= sprintf "got: MD5 %s\n", $got_md5; + $self->_output($output); + return 1; + } + return 0; +} + -- 2.9.0
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- Thanks for the review Andrew! Does this change look correct? I've also fixed another minor spelling issue. If Andrew signs off on this I'll squash it my branch. contrib/verify-pacman-repo-db.pl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contrib/verify-pacman-repo-db.pl b/contrib/verify-pacman-repo-db.pl index 1d02c26..147f71e 100755 --- a/contrib/verify-pacman-repo-db.pl +++ b/contrib/verify-pacman-repo-db.pl @@ -141,39 +141,39 @@ sub check_repodb { my $package = $1; $self->{package_queue}->enqueue({ package => $package, db_desc_content => $file_object->get_content(), dirname => $dirname, }); $pkgcount++; } } - $self->_debug(sprintf("Queued %d package from database '%s'\n", $pkgcount, $repodb)); + $self->_debug(sprintf("Queued %d package(s) from database '%s'\n", $pkgcount, $repodb)); } sub _parse_db_entry { my $self = shift; my $content = shift; my %db; my $key; for my $line (split /\n/, $content) { if ($line eq '') { $key = undef; next; - } - if ($line =~ m/^%(.+)%$/) { + } elsif ($key) { + push @{$db{$key}}, $line; + } elsif ($line =~ m/^%(.+)%$/) { $key = $1; } else { - push @{$db{$key}}, $line; - die "\$key not set. Is the db formated incorrectly?" unless $key; + die "\$key not set. Is the db formatted incorrectly?" unless $key; } } return \%db; } sub _output { my $self = shift; my $output = shift; return if $output eq ""; -- 2.9.0
On 08/09/16 at 07:15pm, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> ---
Thanks for the review Andrew! Does this change look correct?
I've also fixed another minor spelling issue.
If Andrew signs off on this I'll squash it my branch.
Yes, this looks correct. The 'next' in the first branch is redundant now, though.
contrib/verify-pacman-repo-db.pl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/contrib/verify-pacman-repo-db.pl b/contrib/verify-pacman-repo-db.pl index 1d02c26..147f71e 100755 --- a/contrib/verify-pacman-repo-db.pl +++ b/contrib/verify-pacman-repo-db.pl @@ -141,39 +141,39 @@ sub check_repodb { my $package = $1; $self->{package_queue}->enqueue({ package => $package, db_desc_content => $file_object->get_content(), dirname => $dirname, }); $pkgcount++; } }
- $self->_debug(sprintf("Queued %d package from database '%s'\n", $pkgcount, $repodb)); + $self->_debug(sprintf("Queued %d package(s) from database '%s'\n", $pkgcount, $repodb)); }
sub _parse_db_entry { my $self = shift; my $content = shift; my %db; my $key;
for my $line (split /\n/, $content) { if ($line eq '') { $key = undef; next; - } - if ($line =~ m/^%(.+)%$/) { + } elsif ($key) { + push @{$db{$key}}, $line; + } elsif ($line =~ m/^%(.+)%$/) { $key = $1; } else { - push @{$db{$key}}, $line; - die "\$key not set. Is the db formated incorrectly?" unless $key; + die "\$key not set. Is the db formatted incorrectly?" unless $key; } } return \%db; }
sub _output { my $self = shift; my $output = shift;
return if $output eq ""; -- 2.9.0
On Saturday, August 20, 2016 10:54:55 AM CEST Andrew Gregory wrote:
If Andrew signs off on this I'll squash it my branch.
Yes, this looks correct. The 'next' in the first branch is redundant now, though.
Right, thanks. I've rebased the patches on my working branch now. @Allan: Feel free to merge them now. Florian
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Florian Pritz