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

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_at_2
--new=file:////arch/hg/svn-tc/test/repos/quux_at_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_at_2
--new=file:////arch/hg/svn-tc/test/repos/quux/modified_dir/file_in_modified_dir_at_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_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_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

#!/bin/sh

# The next line is the only line you should need to adjust.
SVNDIR=/arch/elego/svn/trunk

## GENERIC PREPARATIONS

SVN=${SVNDIR}/subversion/svn/svn
SVNSERVE=${SVNDIR}/subversion/svnserve/svnserve
SVNADMIN=${SVNDIR}/subversion/svnadmin/svnadmin

# use my local way if my script is there (neels).
if [ -f /usr/local/bin/superpower ]; then
  echo "##### REMEMBER TO SETUP YOUR ENVIRONMENT #####"
  SVN=svn
  SVNSERVE=svnserve
  SVNADMIN=svnadmin
fi

${SVN} --version

# Select an access method. If svn://, the svnserve setup is
# handled automagically by this script; but if http://, then
# you'll have to configure it yourself first.
#
# URL=http://localhost/neels/repos
# URL=svn://localhost/repos
URL=file:///`pwd`/repos

rm -rf repos wc

${SVNADMIN} create repos

# These are for svnserve only.
echo "[general]" > repos/conf/svnserve.conf
echo "anon-access = write" >> repos/conf/svnserve.conf
echo "auth-access = write" >> repos/conf/svnserve.conf

# The server will only be contacted if $URL is svn://foo, of course.
${SVNSERVE} --pid-file svnserve-pid -d -r `pwd`
# And put the kill command in a file, in case need to run it manually.
echo "kill -9 `cat svnserve-pid`" > k
chmod a+rwx k

${SVN} co -q ${URL} wc

cd wc

## ACTUAL TEST

create_stuff()
{
        if [ -n "$1" -a ! -e "$1" ]
        then
    mkdir $1

    mkdir $1/modified_dir
    echo "file" > $1/modified_dir/file_in_modified_dir
    
    ${SVN} add $1
        else
                echo "create_stuff needs a nonexistent directory argument"
        fi
}

modify_stuff()
{
        if [ -d "$1" ]
        then
    echo "more" >> $1/modified_dir/file_in_modified_dir
        else
                echo "modify_stuff needs a directory argument"
        fi
}

echo "===== creating rev 1"
create_stuff foo
${SVN} ci -m "rev 1"

echo "===== creating rev 2"
modify_stuff foo
${SVN} ci -m "rev 2"

# Do the same again in a different dir, but leave mods
# uncommitted.

echo "===== creating rev 3"
create_stuff quux
${SVN} ci -m "rev 3"

echo "===== creating local changes"
modify_stuff quux
${SVN} up

# let's see what diff has to say.
echo
echo "===== Results ====="
echo "===== repos-wc ====="
echo
echo "${SVN} diff --summarize --old=${URL}/foo --new=quux"
${SVN} diff --summarize --old=${URL}/foo --new=quux
echo
echo "${SVN} diff --summarize --old=${URL}/foo_at_1 --new=quux"
${SVN} diff --summarize --old=${URL}/foo_at_1 --new=quux

echo
echo "${SVN} diff --summarize --old=${URL}/foo/modified_dir/file_in_modified_dir --new=quux/modified_dir/file_in_modified_dir"
${SVN} diff --summarize --old=${URL}/foo/modified_dir/file_in_modified_dir --new=quux/modified_dir/file_in_modified_dir
echo
echo "${SVN} diff --summarize --old=${URL}/foo/modified_dir/file_in_modified_dir_at_1 --new=quux/modified_dir/file_in_modified_dir"
${SVN} diff --summarize --old=${URL}/foo/modified_dir/file_in_modified_dir_at_1 --new=quux/modified_dir/file_in_modified_dir

echo
echo "===== repos-repos ====="
echo
echo "${SVN} diff --summarize --old=${URL}/foo_at_2 --new=${URL}/quux_at_3"
${SVN} diff --summarize --old=${URL}/foo_at_2 --new=${URL}/quux_at_3

echo
echo "${SVN} diff --old=${URL}/foo_at_2 --new=${URL}/quux_at_3"
${SVN} diff --old=${URL}/foo_at_2 --new=${URL}/quux_at_3

echo
echo "-----"

echo
echo "${SVN} diff --summarize --old=${URL}/foo/modified_dir/file_in_modified_dir_at_2 --new=${URL}/quux/modified_dir/file_in_modified_dir_at_3"
${SVN} diff --summarize --old=${URL}/foo/modified_dir/file_in_modified_dir_at_2 --new=${URL}/quux/modified_dir/file_in_modified_dir_at_3

echo
echo "${SVN} diff --old=${URL}/foo/modified_dir/file_in_modified_dir_at_2 --new=${URL}/quux/modified_dir/file_in_modified_dir_at_3"
${SVN} diff --old=${URL}/foo/modified_dir/file_in_modified_dir_at_2 --new=${URL}/quux/modified_dir/file_in_modified_dir_at_3

echo
echo "===== wc-wc ====="
echo
echo "${SVN} diff --summarize --old=quux --new=quux"
${SVN} diff --summarize --old=quux --new=quux

echo
echo "${SVN} diff --summarize --old=quux/modified_dir/file_in_modified_dir --new=quux/modified_dir/file_in_modified_dir"
${SVN} diff --summarize --old=quux/modified_dir/file_in_modified_dir --new=quux/modified_dir/file_in_modified_dir

## ACTUAL TEST ENDS
echo "====="
cd ..

./k