You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Neels J Hofmeyr <ne...@elego.de> on 2008/10/03 02:34:15 UTC
`svn diff' paths abbreviated wrongly
Hi dev,
When I diff two paths that begin similarly like this:
/A/foo/baz
/A/foobar/baz
, then svn diff's output puts a path separator where there shouldn't be one:
[[[
$ svn diff svn://localhost/diffpaths/A/foo/baz \
svn://localhost/diffpaths/A/foobar/baz
Index: baz
===================================================================
--- baz (.../baz) (revision 1)
+++ baz (.../bar/baz) (revision 1)
@@ -1 +1 @@
-baz
+quux
]]]
It should say something like
.../foo/baz
.../foobar/baz
but it says
.../baz
.../bar/baz
which is wrong since there is no directory called "bar".
I couldn't find an issue for it. (Maybe this is related, although it talks
about the "Index:" line further above: #1498 "svn diff should mention full
URL to file")
What's the general stance on this?
~Neels
--
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
Re: `svn diff' paths abbreviated wrongly
Posted by Neels J Hofmeyr <ne...@elego.de>.
Looks good to me.
Thanks!
~Neels
Karl Fogel wrote:
> Karl Fogel <kf...@red-bean.com> writes:
>> Debugging now.
>
> Got a fix. I'll commit when it passes "make *check", but it's this:
>
> [[[
> Fix bug whereby 2-URL diff inserted a path separator into a path component.
>
> * subversion/libsvn_client/diff.c
> (diff_content_changed): Just call svn_path_get_longest_ancestor,
> instead of trying (and failing) to duplicate it inline.
>
> Found by: neels
> (But this follows up to r9693, and is really part of issue #1771.)
>
> See original report:
>
> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=143771
> From: Neels J Hofmeyr <ne...@elego.de>
> To: Subversion Developers <de...@subversion.tigris.org>
> Subject: Re: `svn diff' paths abbreviated wrongly
> Date: Fri, 03 Oct 2008 05:05:03 +0200
> Message-ID: <48...@elego.de>
> ]]]
>
> Index: subversion/libsvn_client/diff.c
> ===================================================================
> --- subversion/libsvn_client/diff.c (revision 33405)
> +++ subversion/libsvn_client/diff.c (working copy)
> @@ -422,7 +422,7 @@
> const char *label1, *label2;
> svn_boolean_t mt1_binary = FALSE, mt2_binary = FALSE;
> const char *path1, *path2;
> - int i;
> + apr_size_t len;
>
> /* Get a stream from our output file. */
> os = svn_stream_from_aprfile2(diff_cmd_baton->outfile, TRUE, subpool);
> @@ -444,20 +444,10 @@
>
> path1 = diff_cmd_baton->orig_path_1;
> path2 = diff_cmd_baton->orig_path_2;
> + len = strlen(svn_path_get_longest_ancestor(path1, path2, subpool));
> + path1 = path1 + len;
> + path2 = path2 + len;
>
> - for (i = 0; path1[i] && path2[i] && (path1[i] == path2[i]); i++)
> - ;
> -
> - /* Make sure the prefix is made of whole components. (Issue #1771) */
> - if (path1[i] || path2[i])
> - {
> - for ( ; (i > 0) && (path1[i] != '/'); i--)
> - ;
> - }
> -
> - path1 = path1 + i;
> - path2 = path2 + i;
> -
> /* ### Should diff labels print paths in local style? Is there
> already a standard for this? In any case, this code depends on
> a particular style, so not calling svn_path_local_style() on the
>
> ---------------------------------------------------------------------
> 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
Re: `svn diff' paths abbreviated wrongly
Posted by Karl Fogel <kf...@red-bean.com>.
Karl Fogel <kf...@red-bean.com> writes:
> Debugging now.
Got a fix. I'll commit when it passes "make *check", but it's this:
[[[
Fix bug whereby 2-URL diff inserted a path separator into a path component.
* subversion/libsvn_client/diff.c
(diff_content_changed): Just call svn_path_get_longest_ancestor,
instead of trying (and failing) to duplicate it inline.
Found by: neels
(But this follows up to r9693, and is really part of issue #1771.)
See original report:
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=143771
From: Neels J Hofmeyr <ne...@elego.de>
To: Subversion Developers <de...@subversion.tigris.org>
Subject: Re: `svn diff' paths abbreviated wrongly
Date: Fri, 03 Oct 2008 05:05:03 +0200
Message-ID: <48...@elego.de>
]]]
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 33405)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -422,7 +422,7 @@
const char *label1, *label2;
svn_boolean_t mt1_binary = FALSE, mt2_binary = FALSE;
const char *path1, *path2;
- int i;
+ apr_size_t len;
/* Get a stream from our output file. */
os = svn_stream_from_aprfile2(diff_cmd_baton->outfile, TRUE, subpool);
@@ -444,20 +444,10 @@
path1 = diff_cmd_baton->orig_path_1;
path2 = diff_cmd_baton->orig_path_2;
+ len = strlen(svn_path_get_longest_ancestor(path1, path2, subpool));
+ path1 = path1 + len;
+ path2 = path2 + len;
- for (i = 0; path1[i] && path2[i] && (path1[i] == path2[i]); i++)
- ;
-
- /* Make sure the prefix is made of whole components. (Issue #1771) */
- if (path1[i] || path2[i])
- {
- for ( ; (i > 0) && (path1[i] != '/'); i--)
- ;
- }
-
- path1 = path1 + i;
- path2 = path2 + i;
-
/* ### Should diff labels print paths in local style? Is there
already a standard for this? In any case, this code depends on
a particular style, so not calling svn_path_local_style() on the
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: `svn diff' paths abbreviated wrongly
Posted by Karl Fogel <kf...@red-bean.com>.
Neels J Hofmeyr <ne...@elego.de> writes:
> Sorry, forgot to add the script for reference.
I can reproduce it over all three RA layers. I'll bet there's a
misguided strncmp() somewhere, that's sure what it looks like.
Debugging now.
By the way, it's not so much harder to write a fully self-contained
reproduction script. Below is what I'm using:
--------------------------------------------------------------------------
#!/bin/sh
# The next line is the only line you should need to adjust.
SVNDIR=/home/kfogel/src/subversion
SVN=${SVNDIR}/subversion/svn/svn
SVNSERVE=${SVNDIR}/subversion/svnserve/svnserve
SVNADMIN=${SVNDIR}/subversion/svnadmin/svnadmin
# 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 import-me
${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
mkdir -p A/foo
echo "baz" > A/foo/baz
mkdir -p A/foobar
echo "quux" > A/foobar/baz
${SVN} add A
${SVN} ci -m "message"
${SVN} diff ${URL}/A/foo/baz ${URL}/A/foobar/baz
cd ..
# Put kill command in a file, in case need to run it manually.
echo "kill -9 `cat svnserve-pid`" > k
chmod a+rwx k
./k
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: `svn diff' paths abbreviated wrongly
Posted by Neels J Hofmeyr <ne...@elego.de>.
Sorry, forgot to add the script for reference.
Neels J Hofmeyr wrote:
> Hi dev,
>
> When I diff two paths that begin similarly like this:
>
> /A/foo/baz
> /A/foobar/baz
>
> , then svn diff's output puts a path separator where there shouldn't be one:
>
> [[[
> $ svn diff svn://localhost/diffpaths/A/foo/baz \
> svn://localhost/diffpaths/A/foobar/baz
> Index: baz
> ===================================================================
> --- baz (.../baz) (revision 1)
> +++ baz (.../bar/baz) (revision 1)
> @@ -1 +1 @@
> -baz
> +quux
> ]]]
>
> It should say something like
> .../foo/baz
> .../foobar/baz
> but it says
> .../baz
> .../bar/baz
> which is wrong since there is no directory called "bar".
>
> I couldn't find an issue for it. (Maybe this is related, although it talks
> about the "Index:" line further above: #1498 "svn diff should mention full
> URL to file")
>
> What's the general stance on this?
>
> ~Neels
>
--
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