Skip to content

Commit cff0645

Browse files
rjbsrjbs
committed
indexer: try to use "return early" instead of $ok more
The indexer, here, kept doing this: if ($ok) { ... } if ($ok) { ... } ...and any given block might set $ok to 0, but never return early. That meant that it was hard to know whether $ok might get set back later. It never would. Instead, the code now returns early, because $ok can only become true early on, and from then on is really a way of saying "abort, but keep running through code anyway". Better to just return.
1 parent ee65987 commit cff0645

1 file changed

Lines changed: 89 additions & 90 deletions

File tree

‎lib/PAUSE/package.pm‎

Lines changed: 89 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -569,111 +569,110 @@ has the same version number and the distro has a more recent modification time.}
569569
}
570570
}
571571

572-
# sanity check
573-
if ($ok) {
574-
575-
if ($self->{FIO}{DIO}{VERSION_FROM_META_OK}) {
576-
# nothing to argue at the moment, e.g. lib_pm.PL
577-
} elsif (
578-
! $pp->{basename_matches_package}
579-
&&
580-
PAUSE->basename_matches_package($ofile,$package)
581-
) {
582-
583-
$Logger->log([
584-
"warning: basename does not match package, but it used to: %s", {
585-
package => $package,
586-
old_file => $ofile,
587-
new_file => $pp->{infile},
588-
}
589-
]);
590-
591-
$ok = 0;
592-
}
572+
# If we're not okay yet, we're not going to become okay going forward.
573+
return unless $ok;
574+
575+
if ($self->{FIO}{DIO}{VERSION_FROM_META_OK}) {
576+
# nothing to argue at the moment, e.g. lib_pm.PL
577+
} elsif (
578+
! $pp->{basename_matches_package}
579+
&&
580+
PAUSE->basename_matches_package($ofile,$package)
581+
) {
582+
$Logger->log([
583+
"warning: basename does not match package, but it used to: %s", {
584+
package => $package,
585+
old_file => $ofile,
586+
new_file => $pp->{infile},
587+
}
588+
]);
589+
590+
return;
593591
}
594592

595-
if ($ok) {
596-
my $query = qq{SELECT package, version, dist from packages WHERE LOWER(package) = LOWER(?)};
597-
my($pkg_recs) = $dbh->selectall_arrayref($query,{ Slice => {} },$package);
598-
if (@$pkg_recs > 1) {
599-
$Logger->log([
600-
"conflicting records exist in packages table, won't index: %s",
601-
[ @$pkg_recs ],
602-
]);
603-
604-
$self->index_status(
605-
$ctx,
606-
$package,
607-
"undef",
608-
$pp->{infile},
609-
PAUSE::mldistwatch::Constants::EDBCONFLICT,
610-
qq{Indexing failed because of conflicting records for $package.
593+
my ($pkg_recs) = $dbh->selectall_arrayref(
594+
qq{
595+
SELECT package, version, dist
596+
FROM packages
597+
WHERE LOWER(package) = LOWER(?)
598+
},
599+
{ Slice => {} },
600+
$package,
601+
);
602+
603+
if (@$pkg_recs > 1) {
604+
$Logger->log([
605+
"conflicting records exist in packages table, won't index: %s",
606+
[ @$pkg_recs ],
607+
]);
608+
609+
$self->index_status(
610+
$ctx,
611+
$package,
612+
"undef",
613+
$pp->{infile},
614+
PAUSE::mldistwatch::Constants::EDBCONFLICT,
615+
qq{Indexing failed because of conflicting records for $package.
611616
Please report the case to the PAUSE admins at modules\@perl.org.},
612-
);
613-
$ok = 0;
614-
}
617+
);
618+
619+
return; # XXX Obsolete when the index_status above becomes an
620+
# ->abort_indexing_package!
615621
}
616622

617623
return unless $self->_version_ok($ctx, $pp, $package, $dist);
618624

619625

620-
if ($ok) {
621-
my $query = qq{
622-
UPDATE packages
623-
SET package = ?, version = ?, dist = ?, file = ?,
624-
filemtime = ?, pause_reg = ?
625-
WHERE LOWER(package) = LOWER(?)
626-
};
626+
$Logger->log([
627+
"updating packages: %s", {
628+
package => $package,
629+
version => $pp->{version},
630+
dist => $dist,
631+
infile => $pp->{infile},
632+
filetime => $pp->{filemtime},
633+
disttime => $self->dist->{TIME},
634+
},
635+
]);
627636

628-
$Logger->log([
629-
"updating packages: %s", {
630-
package => $package,
631-
version => $pp->{version},
632-
dist => $dist,
633-
infile => $pp->{infile},
634-
filetime => $pp->{filemtime},
635-
disttime => $self->dist->{TIME},
636-
},
637-
]);
637+
my $rows_affected = eval {
638+
$dbh->do(
639+
q{
640+
UPDATE packages
641+
SET package = ?, version = ?, dist = ?, file = ?,
642+
filemtime = ?, pause_reg = ?
643+
WHERE LOWER(package) = LOWER(?)
644+
},
645+
undef,
646+
$package, $pp->{version}, $dist, $pp->{infile},
647+
$pp->{filemtime}, $self->dist->{TIME},
648+
$package,
649+
);
650+
};
638651

639-
my $rows_affected = eval { $dbh->do
640-
($query,
641-
undef,
642-
$package,
643-
$pp->{version},
644-
$dist,
645-
$pp->{infile},
646-
$pp->{filemtime},
647-
$self->dist->{TIME},
648-
$package,
649-
);
650-
};
651-
652-
if ($rows_affected) { # expecting only "1" can happen
653-
$self->index_status(
654-
$ctx,
655-
$package,
656-
$pp->{version},
657-
$pp->{infile},
658-
PAUSE::mldistwatch::Constants::OK,
659-
"indexed",
660-
);
661-
} else {
662-
my $dbherrstr = $dbh->errstr;
663-
$self->index_status(
664-
$ctx,
665-
$package,
666-
"undef",
667-
$pp->{infile},
668-
PAUSE::mldistwatch::Constants::EDBERR,
669-
qq{The PAUSE indexer could not store the indexing
652+
unless ($rows_affected) {
653+
my $dbherrstr = $dbh->errstr;
654+
$self->index_status(
655+
$ctx,
656+
$package,
657+
"undef",
658+
$pp->{infile},
659+
PAUSE::mldistwatch::Constants::EDBERR,
660+
qq{The PAUSE indexer could not store the indexing
670661
result in the DB due the following error: C< $dbherrstr >.
671662
Please report the case to the PAUSE admins at modules\@perl.org.},
672-
);
673-
}
663+
);
674664

665+
return; # XXX this return obsolete when ->abort_indexing_package is here
675666
}
676667

668+
$self->index_status(
669+
$ctx,
670+
$package,
671+
$pp->{version},
672+
$pp->{infile},
673+
PAUSE::mldistwatch::Constants::OK,
674+
"indexed",
675+
);
677676
}
678677

679678
sub __do_regular_perl_update {

0 commit comments

Comments
 (0)