Johan Corveleyn wrote on Mon, Jan 31, 2011 at 02:47:50 +0100:
> On Fri, Jan 28, 2011 at 7:58 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> > Johan Corveleyn wrote on Fri, Jan 28, 2011 at 14:04:07 +0100:
> >> On Fri, Jan 28, 2011 at 9:29 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> >> > May I suggest that, if this code is to be released, then you validate
> >> > its correctnss? For example, a minimal regression test that is written
> >> > to record trunk's pre-branch behaviour might be sufficient.
> >>
> >> ... I'll accept your suggestion. I'll try to take some time for that
> >> this weekend. If anyone beats me to it, that would be fine as well of
> >> course :).
> >>
>
> Sorry, didn't get around to it yet. Maybe sometime during next week.
>
> > Thanks :-). I've looked into it, but it seems the output functions in
> > svn_diff.h are oriented to diff2/diff3 only (they don't have an
> > 'ancestor' enum or parameters); and in a quick test, I couldn't get
> > tools/diff/diff4 to output anything sensible:
> >
> > % for i in 1 2 3 4; do echo $i>$i; done
> > % ./tools/diff/diff4 1 2 3 4
> > 3
> > %
>
> I think it's more or less sensible, although I'm not completely sure.
> If you look at the "usage" of diff4, you see:
>
> Usage: /path/to/diff4 <mine> <older> <yours> <ancestor>
>
> I think what you did is: take the difference between 2 and 3 (older
> and yours), and apply that to 1 (mine), using 4 as the common
> ancestor. Apparently that yields "3" :-).
>
> Compare that with the use of diff3, with the files 1, 2 and 3:
>
> Usage: /path/to/diff3 <mine> <older> <yours>
>
> $ diff3 1 2 3
> <<<<<<< 1
> 1
> =======
> 3
> >>>>>>> 3
>
> Hmmm, that looks like a merge conflict, which seems logical (trying to
> apply the diff between 2 and 3, to the file 1 which doesn't contain a
> line '2').
>
Agreed.
> So now I also don't really understand why diff4 successfully merges
> it, given the ancestor 4. Maybe the reasoning is as follows, given
> that 4 is the common ancestor:
>
> - 1 has evolved out of 4 (so '1' is the mine's replacement for '4')
>
> - 2 has evolved out of 4 (so in the merge source, '2' is the
> replacement for '4')
>
> - So with "variance-adjusted patching", the diff between 2 and 3
> translates into: "replace whatever '4' was transformed into (in this
> case '1') into '3'". That would yield '3'.
>
> Not sure though.
>
> > How can we test diff4 then? Do we have to add public diff4 APIs in
> > order to be able to test svn_diff_diff4_2()?
>
> Maybe we should come up with a better example. In the meantime, I'm
> trying to understand diff4 (reading kfogel's article at [1], as
> referenced in diff4.c).
>
That article was very helpful --- thanks for the pointer!
> > Daniel
> > (on the one hand I'd much prefer to test the API changes before
> > releasing them; on the other hand, adding API's related to an API
> > that no one (to our knowledge) uses seems odd)
>
> Yes. I think we should give it some effort to come up with some
> minimal testing. But if it takes too long, we should probably not let
> this be blocking....
>
I've went ahead and made a patch out of the second example in that
article. However, I admit that I got it to work by fiddling with the
order of the four parameters until the test passed :-)
Could you have a look? (attached)
Thanks,
Daniel
> Cheers,
> --
> Johan
>
> [1] http://svn.apache.org/repos/asf/subversion/trunk/notes/variance-adjusted-patching.html
Received on 2011-02-04 08:01:02 CET