On Fri, Jan 28, 2011 at 7:58 PM, Daniel Shahaf <d...@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...@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').
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).
> 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....
Cheers,
--
Johan
[1]
http://svn.apache.org/repos/asf/subversion/trunk/notes/variance-adjusted-patching.html
