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
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
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
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 , as
referenced in diff4.c).
> (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....
Received on 2011-01-31 02:48:50 CET