Re: svn commit: r33597 - branches/diff-repos-wc/subversion/libsvn_client

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_at_tigris.org wrote:
> Author: neels
> Date: Fri Oct 10 21:52:20 2008
> New Revision: 33597
>
> Log:
+On the 'diff-repos-wc' branch:
+
> *** Needs testing. Error reports and debugs welcome. ***

So, I'm using attached test script, which runs diff --summarize in all the
cases repos<>repos, repos<>wc and wc<>wc (only the case of wc text-base vs.
wc local mods, because arbitrary wc<>wc diffs aren't supported by diff in
the first place). By running the script using both of trunk and my branch
and wildly comparing outputs, I get the results listed below.

I guess it's best to first try to fix the very wrong case that is so
important for dirs_same_p() (marked "*** NOTE"), hopefully fixing normal
diff on trunk in the process.

~Neels

===== Results =====
(...only using file:// URLs for now...)

===== repos-wc =====

Firstly, I diff the working copy against its own URL
  `svn diff --summarize --old=${URL} --new=.` ("." being URL's wc)
and compare with
  `svn status` (which does sort of the same).

Summarizing diff prints the complete URL while status only prints the path
relative to the working copy. (Removing the URL)

These summarizing diff output lines differ from status:
  M quux/added_empty_dir_with_props ** status says "A "
  M quux/added_dir_with_props ** status says "A "
 AM quux/added_file_with_props ** status says "A "
They should all say "AM", so the first two need fixing.

These lines are only in status and missing in summarizing diff:
 D quux/deleted_empty_dir
 A quux/added_dir
 D quux/deleted_dir
 A quux/added_empty_dir
The first and last should definitely appear in a summarizing diff. The two
in the middle *do* show the enclosed file in a summarizing diff (not shown
in this mail), so they don't necessarily need fixing (right?).

Looks pretty good so far.

---
Secondly, I diff the working copy against an unrelated URL (which
"coincidentally" is similar to the working copy's base)
  `svn diff --summarize --old=${URL}/foo_at_2 --new=quux`.
and again compare to status, which should show similar results in this
particular setup.
Summarizing diff prints the complete URL, but it mysteriously adds a "quux"
to it, resulting in
  ${URL}/foo/quux/...
which is wrong since foo and quux are siblings. (Ignoring that. Removing the
${URL}/foo part)
-svn status
+svn diff --summarize --old=${URL}/foo_at_2 --new=quux
These are the same in both:
 M       modified_file
 M       modified_dir_with_props/file_in_modified_dir_with_props
 M       modified_dir/file_in_modified_dir
These differ:
+D       added_file
-A       added_file
+M       modified_file_with_props
-MM      modified_file_with_props
+D       added_file_with_props
-A       added_file_with_props
These appear *twice* in summarizing diff:
+D       added_dir/file_in_added_dir
 A       added_dir/file_in_added_dir
+D       added_dir_with_props/file_in_added_dir_with_props
 A       added_dir_with_props/file_in_added_dir_with_props
+ M      added_dir_with_props
-A       added_dir_with_props
+ M      added_dir_with_props
+ M      added_empty_dir_with_props
-A       added_empty_dir_with_props
+ M      added_empty_dir_with_props
These are missing in summarizing diff:
- M      modified_empty_dir_only_props
-D       deleted_empty_dir
- M      modified_dir_with_props
-A       added_dir
- M      modified_dir_only_props
- M      modified_file_only_props
-D       deleted_dir
-D       deleted_dir/file_in_deleted_dir
-D       deleted_file
-A       added_empty_dir
So, something is obviously still very wrong with this one. Alas, this is
exactly the case that is so important for dirs_same_p(). Frown.
*** NOTE that the current `svn diff' (non-summarizing, on trunk) shows
exactly the same nonsensical output, just in long format. So, it's not me
who broke it, it's simply the same shmuh that normal diff on trunk does.
The only difference to normal diff is that normal diff completely omits
  MM      modified_file_with_props
which is very bad indeed (the file has *both* prop and content changes!).
Here's an extract of a normal trunk diff in this case which shows why some
nodes are listed twice; the same prop change is first undone (although it's
not there yet) and then added "again":
[[[
Property changes on: quux/added_dir_with_props
___________________________________________________________________
Deleted: changedprop
   - propchange
[...]
Property changes on: quux/added_dir_with_props
___________________________________________________________________
Added: changedprop
   + propchange
]]]
===== repos-repos =====
I compare the old implementation of repos<>repos summarizing diff with the
new one. Here are the differences:
File in added dir skips to after the added dir. I see it as an improvement:
 -A      file:///arch/hg/svn-tc/test/repos/quux/added_dir/file_in_added_dir
  A      file:///arch/hg/svn-tc/test/repos/quux/added_dir
 +A      file:///arch/hg/svn-tc/test/repos/quux/added_dir/file_in_added_dir
Added directory with prop changes is split into two lines; that's bad:
-AM     file:///arch/hg/svn-tc/test/repos/quux/added_empty_dir_with_props
+A      file:///arch/hg/svn-tc/test/repos/quux/added_empty_dir_with_props
+ M     file:///arch/hg/svn-tc/test/repos/quux/added_empty_dir_with_props
And here we see a combination of the two above:
+A      file:///arch/hg/svn-tc/test/repos/quux/added_dir_with_props
 A      file:///arch/hg/svn-tc/test/repos/quux/added_dir_with_props/file_...
-AM     file:///arch/hg/svn-tc/test/repos/quux/added_dir_with_props
+ M     file:///arch/hg/svn-tc/test/repos/quux/added_dir_with_props
The two "A " and " M" lines should be fixed back into a single "AM" line.
Looks good otherwise.
===== wc-wc =====
I compare
  `svn diff --summarize --old=quux --new=quux`
and
  `svn status` (which does sort of the same).
Summarizing diff prints quux twice "quux/quux". I ignore that for now and
get the exact same results as in the first part of repos-wc above, except
this time the working copy path is printed instead of the URL and no editing
from my side is needed to compare:
(---repeating---8<-----)
These summarizing diff output lines differ from status:
  M      quux/added_empty_dir_with_props  ** status says "A "
  M      quux/added_dir_with_props        ** status says "A "
 AM      quux/added_file_with_props       ** status says "A "
They should all say "AM", so the first two need fixing.
These lines are only in status and missing in summarizing diff:
 D       quux/deleted_empty_dir
 A       quux/added_dir
 D       quux/deleted_dir
 A       quux/added_empty_dir
The first and last should definitely appear in a summarizing diff. The two
in the middle *do* show the enclosed file in a summarizing diff (not shown
in this mail), so they don't necessarily need fixing (right?).
Looks pretty good so far.
(----->8---repeating-end---)
===== done. =====
-- 
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
    echo "modified_file" > $1/modified_file
    echo "modified_file_with_props" > $1/modified_file_with_props
    echo "modified_file_only_props" > $1/modified_file_only_props

    mkdir $1/modified_dir
    echo "file" > $1/modified_dir/file_in_modified_dir
    
    mkdir $1/modified_dir_with_props
    echo "file" > $1/modified_dir_with_props/file_in_modified_dir_with_props
    
    mkdir $1/modified_dir_only_props
    echo "file" > $1/modified_dir_only_props/file_in_modified_dir_only_props
    
    mkdir $1/modified_empty_dir_only_props
    
    echo "file" > $1/deleted_file

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

modify_stuff()
{
        if [ -d "$1" ]
        then
    echo "more" >> $1/modified_file

    echo "more" >> $1/modified_file_with_props
    ${SVN} propset changedprop propchange $1/modified_file_with_props

    ${SVN} propset changedprop propchange $1/modified_file_only_props

    echo "added file" > $1/added_file
    ${SVN} add $1/added_file

    echo "added file with props" > $1/added_file_with_props
    ${SVN} add $1/added_file_with_props
    ${SVN} propset addedprop propadd $1/added_file_with_props

    echo "more" >> $1/modified_dir/file_in_modified_dir

    ${SVN} propset changedprop propchange $1/modified_dir_with_props
    echo "more" >> $1/modified_dir_with_props/file_in_modified_dir_with_props

    ${SVN} propset changedprop propchange $1/modified_dir_only_props

    ${SVN} propset changedprop propchange $1/modified_empty_dir_only_props

    mkdir $1/added_dir
    echo "newfile" > $1/added_dir/file_in_added_dir
    ${SVN} add $1/added_dir

    mkdir $1/added_dir_with_props
    echo "newfile" > $1/added_dir_with_props/file_in_added_dir_with_props
    ${SVN} add $1/added_dir_with_props
    ${SVN} propset changedprop propchange $1/added_dir_with_props

    mkdir $1/added_empty_dir
    ${SVN} add $1/added_empty_dir

    mkdir $1/added_empty_dir_with_props
    ${SVN} add $1/added_empty_dir_with_props
    ${SVN} propset changedprop propchange $1/added_empty_dir_with_props

    ${SVN} rm $1/deleted_file $1/deleted_dir $1/deleted_empty_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} --new=."
${SVN} diff --summarize --old=${URL} --new=.
echo
echo "${SVN} status"
${SVN} status
echo
echo "${SVN} diff --summarize --old=${URL}/foo_at_2 --new=quux"
${SVN} diff --summarize --old=${URL}/foo_at_2 --new=quux
echo
echo "${SVN} diff --old=${URL}/foo_at_2 --new=quux"
${SVN} diff --old=${URL}/foo_at_2 --new=quux

echo
echo "===== repos-repos ====="
echo
echo "${SVN} diff --summarize --old=${URL}/quux_at_3 --new=${URL}/foo_at_3"
${SVN} diff --summarize --old=${URL}/quux_at_3 --new=${URL}/foo_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} status quux"
${SVN} status quux

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

./k