[PATCH] Investigating/testing diff --summarize

[Julian Foad <ju...@btopenworld.com>]

On Sun, 2008-10-12 at 05:12 +0200, Neels J. Hofmeyr wrote: 
> This is a summary for those interested in discovering scores of broken diff
> cases (on trunk, marked "*** NOTE") or in the current state of summarizing
> diff re- implementation, and otherwise just a note to myself.

Neels, thanks for adding support for "diff --summarize" repos-wc,
wc-repos and wc-wc cases.

I'm looking at the current state of the "diff-repos-wc" branch.

I have extended diff-tests 37, "basic diff summarize", to also
incorporate:

  - dir add, dir delete (as well as the other cases it already did)

  - repos-wc, wc-wc (as well as repos-repos that it already did)

This shows up some problems - e.g. it fails to call the "dir deleted"
and "dir added" callbacks except in repos-repos mode.

I have added "printf" debugging lines to the callbacks to show this.

Posting so anyone can investigate. I'll have more of a look tomorrow.

- Julian




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

Re: [PATCH] Investigating/testing diff --summarize

[Julian Foad <ju...@btopenworld.com>]

On Thu, 2008-10-16 at 00:29 +0200, Julian Foad wrote:
> On Sun, 2008-10-12 at 05:12 +0200, Neels J. Hofmeyr wrote: 
> > This is a summary for those interested in discovering scores of broken diff
> > cases (on trunk, marked "*** NOTE") or in the current state of summarizing
> > diff re- implementation, and otherwise just a note to myself.
> 
> Neels, thanks for adding support for "diff --summarize" repos-wc,
> wc-repos and wc-wc cases.
> 
> I'm looking at the current state of the "diff-repos-wc" branch.
> 
> I have extended diff-tests 37, "basic diff summarize", to also
> incorporate:
> 
>   - dir add, dir delete (as well as the other cases it already did)
> 
>   - repos-wc, wc-wc (as well as repos-repos that it already did)
> 
> This shows up some problems - e.g. it fails to call the "dir deleted"
> and "dir added" callbacks except in repos-repos mode.
> 
> I have added "printf" debugging lines to the callbacks to show this.
> 
> Posting so anyone can investigate. I'll have more of a look tomorrow.
> 
> - Julian

Here's the patch... attached.

- Julian

Re: [PATCH] Investigating/testing diff --summarize

["Neels J. Hofmeyr" <ne...@elego.de>]

Hi Julian,

we decided to merge the diff-repos-wc branch to trunk even though
summarizing diff isn't working properly for the newly introduced cases yet,
because we can't be bothered right now. At least it does *something*.

But, we thought it would be best to first match the new implementation of
repos<>repos diff_summarize to the old output of diff_summarize. But it
turns out not to be easy at all.

The old repos<>repos prints the complete URL for each node, and that for the
`old' side of the diff. What we have is `old' and `new' targets, and the
path passed in by the diff_callbacks, which is loosely connected to the
`new' target. Now, the problem is, depending on how diff --summarize was
invoked, the path passed in by the callbacks is either the current filename
that is already in the target URL, or the relative path and filename,
repeating some path elements already found in the URL, or, stupidly enough,
might also be *just* the part of the path relative to the target without
repetitions involved. I tried to use svn_path_is_child(), but it fails at
some while managing other cases. So, it's really cumbersome to check for all
these cases just to stay compliant with previous diff --summarize output.

Instead, I'd suggest taking the normal repos<>repos diff as an example.
Let's see:

[[[
svn diff --old=file:////arch/hg/svn-tc/test/repos/foo@2
--new=file:////arch/hg/svn-tc/test/repos/quux@3
Index: modified_dir/file_in_modified_dir
===================================================================
--- modified_dir/file_in_modified_dir	(.../foo)	(revision 2)
+++ modified_dir/file_in_modified_dir	(.../quux)	(revision 3)
@@ -1,2 +1 @@
 file
-more
]]]

[[[
svn diff
--old=file:////arch/hg/svn-tc/test/repos/foo/modified_dir/file_in_modified_dir@2
--new=file:////arch/hg/svn-tc/test/repos/quux/modified_dir/file_in_modified_dir@3
Index: file_in_modified_dir
===================================================================
--- file_in_modified_dir	(.../foo/modified_dir/file_in_modified_dir)
(revision 2)
+++ file_in_modified_dir	(.../quux/modified_dir/file_in_modified_dir)
(revision 3)
@@ -1,2 +1 @@
 file
-more
]]]

Reflecting on what I would *want* to see in summarizing diff, I think I'd
never want to see the complete URLs repeated over and over. I'd only want
the really interesting path section.

So, aaafter all, I actually like your patch best. It is also the simplest
solution of them all, it just takes the path that the diff callbacks are
given for each node, and doesn't muck about with appending stuff.

Normal diff tries to explicitly print the differing URL sections and
revisions, because it needs to identify which part of the diff is coming
from where. Since summarizing diff just has one piece of information on each
node, I think we can omit this information. Let's throw that in and let
other people make us change it, and let's finally stop wasting precious time
on this useless nonesense for now... :P

For reference, attaching my most recent attempt at fixing the output for
modified files and a script that generates some diff --summarize and some
normal diff cases. (patch is against branches/diff-repos-wc/)

~Neels



Julian Foad wrote:
> On Sun, 2008-10-12 at 05:12 +0200, Neels J. Hofmeyr wrote: 
>> This is a summary for those interested in discovering scores of broken diff
>> cases (on trunk, marked "*** NOTE") or in the current state of summarizing
>> diff re- implementation, and otherwise just a note to myself.
> 
> Neels, thanks for adding support for "diff --summarize" repos-wc,
> wc-repos and wc-wc cases.
> 
> I'm looking at the current state of the "diff-repos-wc" branch.
> 
> I have extended diff-tests 37, "basic diff summarize", to also
> incorporate:
> 
>   - dir add, dir delete (as well as the other cases it already did)
> 
>   - repos-wc, wc-wc (as well as repos-repos that it already did)
> 
> This shows up some problems - e.g. it fails to call the "dir deleted"
> and "dir added" callbacks except in repos-repos mode.
> 
> I have added "printf" debugging lines to the callbacks to show this.
> 
> Posting so anyone can investigate. I'll have more of a look tomorrow.
> 
> - Julian
> 
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
> 

-- 
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696  mobile: +49 177 2345869  fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194