(+cc: Eric Wong) Hi Jason, Jason Gross wrote: [This patch teaches "git svn" to invalidate caches when they fail to load, for example because the endianness or size of some type changed, which is common in the perl 5.6 -> 5.8 upgrade.] > http://lists.debian.org/debian-perl/2011/05/msg00023.html and > http://lists.debian.org/debian-perl/2011/05/msg00026.html). [...] > --- a/git-svn.perl > +++ b/git-svn.perl > @@ -1680,7 +1680,7 @@ use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent > $_use_svnsync_props $no_reuse_existing $_minimize_url > $_use_log_author $_add_author_from $_localtime/; > use Carp qw/croak/; > -use File::Path qw/mkpath/; > +use File::Path qw/mkpath rmtree/; > use File::Copy qw/copy/; > use IPC::Open3; > use Memoize; # core since 5.8.0, Jul 2002 > @@ -3198,28 +3198,41 @@ sub has_no_changes { > $memoized = 1; > > my $cache_path = "$ENV{GIT_DIR}/svn/.caches/"; > - mkpath([$cache_path]) unless -d $cache_path; > - > - tie my %lookup_svn_merge_cache => 'Memoize::Storable', [...] > - ; > + my $do_memoization = sub { > + mkpath([$cache_path]) unless -d $cache_path; [...] > + ; > + }; > + > + if (not eval { > + $do_memoization->(); > + 1; > + }) { > + my $err = $@ || "Zombie error"; # "Zombie error" to catch clobbered $@ in buggy destructors > + die $err unless -d $cache_path; > + print STDERR "Discarding cache and trying again ($@)\n"; > + rmtree([$cache_path]); > + $do_memoization->(); > + } Thank you thank you thank you. Okay, time for nitpicks: 1) Would it be possible to lift this do_memoization() to a toplevel sub? I suspect that could make the code a little easier to read. 2) Is it important to discard the cache for _all_ errors, instead of just corruption and "is not compatible" errors? Rebuilding the cache is not cheap, and I am afraid of effects like repeatedly discarding the cache only to rebuild it again due to a typo in git-svn.perl or an out-of-memory condition. 3) The line with "Zombie error" is very long --- I guess putting the comment on the line before would help. 4) The series would be clearer imho as a single patch that includes both the fix and tests. Eric, what do you think? Thanks again, :) Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html