Eric Wong <e...@80x24.org> writes:
> Johannes Schindelin <johannes.schinde...@gmx.de> wrote:
>> +++ b/perl/Git/SVN.pm
>> @@ -1658,6 +1658,11 @@ sub tie_for_persistent_memoization {
>> if ($memo_backend > 0) {
>> tie %$hash => 'Git::SVN::Memoize::YAML', "$path.yaml";
>> } else {
>> + # first verify that any existing file can actually be loaded
>> + # (it may have been saved by an incompatible version)
>> + if (-e "$path.db") {
>> + unlink "$path.db" unless eval { retrieve("$path.db"); 1
>> };
>> + }
>
> That retrieve() call is unlikely to work without "use Storable"
> to import it into the current package.
>
> I also favor setting "$path.db" once to detect typos and avoid
> going over 80 columns. Additionally, having error-checking for
> unlink might be useful.
>
> So perhaps squashing this on top:
>
> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
> index 025c894..b3c1460 100644
> --- a/perl/Git/SVN.pm
> +++ b/perl/Git/SVN.pm
> @@ -1660,10 +1660,15 @@ sub tie_for_persistent_memoization {
> } else {
> # first verify that any existing file can actually be loaded
> # (it may have been saved by an incompatible version)
> - if (-e "$path.db") {
> - unlink "$path.db" unless eval { retrieve("$path.db"); 1
> };
> + my $db = "$path.db";
> + if (-e $db) {
> + use Storable qw(retrieve);
> +
> + if (!eval { retrieve($db); 1 }) {
> + unlink $db or die "unlink $db failed: $!";
> + }
> }
> - tie %$hash => 'Memoize::Storable', "$path.db", 'nstore';
> + tie %$hash => 'Memoize::Storable', $db, 'nstore';
> }
> }
>
>
> Thoughts? Thanks.
Just peeking from the sideline, but the your squash looks like an
improvement to me.
Hopefully the final version after your interaction with Dscho can
come to me via another "pull this now"?
Thanks.
