Re: [PATCH] git svn: reset invalidates the memoized mergeinfo caches

From: Eric Wong <normalperson@yhbt.net>
Date: Aug 8, 2012, 22:52 UTC
Message-ID: <20120808225258.GA24956@dcvr.yhbt.net>
In-Reply-To: <20120808054129.GB10899@m62s10.vlinux.de>

Peter Baumann <waste.manager@gmx.de> wrote:

Show 19 quoted lines

> On Tue, Aug 07, 2012 at 08:45:10PM +0000, Eric Wong wrote:
> > Peter Baumann <waste.manager@gmx.de> wrote:
> > > +			for my $suffix (qw(yaml db)) {
> > > +				unlink("$cache_file.$suffix");
> > 
> > Need to check for unlink() errors (and ignore ENOENT).
> 
> I'm not sure what you mean here: Aren't we screwed either way if unlinking
> the file failed? There is nothhing we can do about it if e.g. the user doesn't
> have the permissions to delete the file, besides terminating, e.g.
> 
> 	for my $cache_file (("$cache_path/lookup_svn_merge",
> 			     "$cache_path/check_cherry_pick",
> 			     "$cache_path/has_no_changes")) {
> 		for my $suffix (qw(yaml db)) {
> 			next unless (-e "$cache_file.$suffix");
> 			unlink("$cache_file.$suffix") or 
> 				die "Failed to delete $cache_file.$suffix";
> 		}

Yes we're screwed, but silent failure is the worst way to fail, especially if it can lead us back to the problems your patch is meant to address.

Perhaps something like this (with $! to show the error):

			my $file = "$cache_file.$suffix";
			next unless -e $file;
			unlink($file) or die "unlink($file) failed: $!\n";