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

[Eric Wong <normalperson@yhbt.net>]

Peter Baumann <waste.manager@gmx.de> wrote:
> Therefore the easiest solution to clear the cache is to delete the files
> on disk in 'git svn reset'. Normally, deleting the files behind the back
> of the memoization module would be problematic, because the in-memory
> representation would still exist and contain wrong data. Fortunately, the
> memoization is active in memory only for a small portion of the code.
> Invalidating the cache by deleting the files on disk if it isn't active
> should be safe.

Thanks for the patch and explanation.  A few comments below:

> +	sub clear_memoized_mergeinfo_caches {
> +		die "Only call this method in non-memoized context" if ($memoized);
> +
> +		my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";
> +		return unless -d $cache_path;
> +
> +		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)) {
> +				unlink("$cache_file.$suffix");

Need to check for unlink() errors (and ignore ENOENT).

> @@ -2126,8 +2142,13 @@ sub rev_map_set {
>  
>  	sysopen(my $fh, $db_lock, O_RDWR | O_CREAT)
>  	     or croak "Couldn't open $db_lock: $!\n";
> -	$update_ref eq 'reset' ? _rev_map_reset($fh, $rev, $commit) :
> -				 _rev_map_set($fh, $rev, $commit);
> +	if ($update_ref eq 'reset') {
> +		_rev_map_reset($fh, $rev, $commit);
> +		clear_memoized_mergeinfo_caches();

Better to clear_memoized_mergeinfo_caches() before _rev_map_reset()
in case unlink() (or anything else) fails when clearing the cache.

> +test_expect_success 'initialize source svn repo' '
> +	svn_cmd mkdir -m "create trunk" "$svnrepo"/trunk &&
> +	svn_cmd mkdir -m "create branches" "$svnrepo/branches" &&
> +	svn_cmd co "$svnrepo"/trunk "$SVN_TREE" &&
> +	(
> +		cd "$SVN_TREE" &&
> +		touch foo &&
> +		svn add foo &&

svn_cmd here, too.

> +		svn commit -m "a" &&
> +		svn cp -m branch "$svnrepo"/trunk "$svnrepo"/branches/branch1 &&
> +		svn switch "$svnrepo"/branches/branch1 &&
> +		touch bar &&
> +		svn add bar &&
> +		svn commit -m b &&
> +		svn switch "$svnrepo"/trunk &&
> +		touch baz &&
> +		svn add baz &&
> +		svn commit -m c &&
> +		svn up &&
> +		svn merge "$svnrepo"/branches/branch1 &&
> +		svn commit -m "m"
> +	) &&