Re: [PATCH] Issue 2496 history following algorithm

Philip Martin <philip@codematters.co.uk> writes:

> Philip Martin <philip@codematters.co.uk> writes:
>
>> I've never really looked at the peg code before, but as far as I can
>> tell the problem occurs in svn_repos_trace_locations. I don't fully
>> understand the algorithm but it gets called with fs_path "/bar",
>> peg_revision 3, and location_revisions_orig of "2" and it identifies
>> the foo->bar copy at r3 but it doesn't put anything in the locations
>> array.
>
> Deleting this bit of code seems to fix the problem, does anyone know
> why it is present? [If you read the patch closely it's only the two
> linw while loop that is deleted.]
>
> Index: subversion/libsvn_repos/rev_hunt.c
> ===================================================================
> --- subversion/libsvn_repos/rev_hunt.c (revision 19420)
> +++ subversion/libsvn_repos/rev_hunt.c (working copy)
> @@ -497,12 +497,6 @@
> revision_ptr++;
> }
>
> - /* Follow the copy to its source. Ignore all revs between the
> - copy target rev and the copy source rev (non-inclusive). */
> - SVN_ERR(svn_fs_copied_from(&srev, &spath, croot, cpath, currpool));
> - while ((revision_ptr < revision_ptr_end) && (*revision_ptr > srev))
> - revision_ptr++;
> -
> /* Ultimately, it's not the path of the closest copy's source
> that we care about -- it's our own path's location in the
> copy source revision. So we'll tack the relative path that
> @@ -517,6 +511,7 @@
> the copy source path tells us that our path was located at
> "/trunk/foo/bar" before the copy.
> */
> + SVN_ERR(svn_fs_copied_from(&srev, &spath, croot, cpath, currpool));
> remainder = (strcmp(cpath, path) == 0) ? "" :
> svn_path_is_child(cpath, path, currpool);
> path = svn_path_join(spath, remainder, currpool);
>

I'm beginning to understand the problem and I think this patch is
incorrect because the source path might not be the same object, or
might not even exist, in revisions greater than srev.

   r1 /foo created
   r2 /foo deleted
   r3 ... more or less anything
   r4 /bar created by copying /foo@1

I think this is the original issue 1970 problem, although I am a
little surprised that non of the regression tests failed with my
patch applied.

It seems that svn_repos_trace_locations is not the place to fix this.
Perhaps svn_client__repos_locations could try all revisions between
start_revnum and end_revnum when looking for start_path? Would that
be acceptable for all commands, or would we need a flag to enable this
sort of serching?

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org