From: Eric Wong <normalperson@yhbt.net>
Date: Tue, 29 May 2012 00:25:37 GMT
Subject: Re: [RFC/PATCH] git-svn: use YAML format for mergeinfo cache when possible
Message-ID: <20120529002537.GA4575@dcvr.yhbt.net>
In-Reply-To: <20120528003901.GA11103@burratino>

Jonathan Nieder <jrnieder@gmail.com> wrote:
> Eric Wong wrote:
> > Jonathan Nieder <jrnieder@gmail.com> wrote:
> 
> >> +package Git::SVN::Memoize::YAML;
> >
> > Can we use this as an opportunity to start splitting git-svn.perl into
> > multiple .pm files?
> 
> Not a bad idea.  I've included an example patch to sanity-check the
> approach below.

Thank you.  I've signed-off and pushed that and your other two out to
"master" on git://bogomips.org/git-svn

> >> +	my $truehash = (-e $filename) ? YAML::Any::LoadFile($filename) : {};
> >
> >> +	YAML::Any::DumpFile($self->{FILENAME}, $self->{H});
> >
> > These should die on errors, right?
> 
> At least in YAML::Old, they use Carp::croak.  Maybe something like
> 
> 	local @CARP_NOT = qw(YAML::Any);
> 
> to blame the caller for the error would bring sanity.

Interesting, didn't know about @CARP_NOT.  It's barely documented in
on my Debian stable install(s), but it seems to be available since 5.8.
We should be able to use it without breaking compatibility.

> >> +=head1 BUGS
> >
> >> +Error handling is awkward.
> >
> > How so?
> 
> I mostly meant that it's not obvious what the state of %hash is at the
> point marked with (*) below:
> 
> 	if (not eval {
> 		tie my %hash => 'Foo::Bar', @params;
> 		1;
> 	}) {
> 		my $err = $@ ||
> 			# a destructor might have clobbered $@
> 			"Zombie error";
> 		die $err if worth_dying($err);
> 
> 		(*) ... try to recover ...
> 	}
> 
> That's not specific to Memoize::YAML, though.  It probably is not
> awkward for wizards who know the details. :)

OK, so we're no better or worse off than we were before :)