You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alexey Neyman <st...@att.net> on 2012/09/01 00:24:18 UTC
[PATCH] SVN 1.7 and 1.8 generates extra header when diffing with external tool
Hi SVN developers,
I noticed an issue which prevents the 'post-review' tool from ReviewBoard
(http://www.reviewboard.org) from working with Subversion 1.7.x and trunk
(1.8.0-dev).
This post-review tool uses external diff command to generate a diff file. With
Subversion 1.7 and 1.8, using external diff tool results in an extra "Index:"
header being printed. Here is a test case:
[[[
repo=/tmp/repo.test
wc=/tmp/wc.test
svnadmin create $repo
svn mkdir -m "create subdir" file://$repo/dir1
svn co file://$repo $wc
cd $wc
echo hi > dir1/file2
svn add dir1/file2
svn ps foo:bar baz dir1/file2
svn mv dir1 dir2
svn diff --diff-cmd=diff dir2 > output.diff
]]]
It produces the following diff file:
[[[
Index: dir2/file2
===================================================================
--- dir2/file2 (revision 0)
+++ dir2/file2 (working copy)
@@ -0,0 +1 @@
+hi
Index: dir2/file2
===================================================================
--- dir2/file2 (revision 1)
+++ dir2/file2 (working copy)
Property changes on: dir2/file2
___________________________________________________________________
Added: foo:bar
## -0,0 +1 ##
+baz
\ No newline at end of property
]]]
Not only there are two "Index:" headers printed, but the second one is
incorrect: it refers to dir2/file2 as being present in revision 1. The post-
review tool, seeing this header, fails to find the origin of dir2/file2 in
revision 1 and produces an error.
The reason is that the branch in diff_content_changed() that invokes an
external diff tool does not mark the path as visited even though it emits the
"Index:" header. Marking it as visited is sufficient, as the header is
incorrect only if the file is added - in which case, one of the branches in
diff_content_changed() is going to output the diff (diff_cmd_baton-
>force_empty is set for added files). Therefore, diff_prop_changed() will
never output "Index:" header for added files.
Patch attached.
[[[
* subversion/libsvn_client/diff.c:
(diff_content_changed): Mark path as visited if diffed by external tool.
]]]
Regards,
Alexey.
Re: [PATCH] SVN 1.7 and 1.8 generates extra header when diffing with
external tool
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Alexey Neyman wrote on Tue, Sep 04, 2012 at 23:40:58 -0700:
> On Tuesday, September 04, 2012 05:56:27 PM Stefan Sperling wrote:
> > On Fri, Aug 31, 2012 at 03:24:18PM -0700, Alexey Neyman wrote:
> > > Hi SVN developers,
> > >
> > > I noticed an issue which prevents the 'post-review' tool from ReviewBoard
> > > (http://www.reviewboard.org) from working with Subversion 1.7.x and trunk
> > > (1.8.0-dev).
> > >
> > > This post-review tool uses external diff command to generate a diff file.
> > > With Subversion 1.7 and 1.8, using external diff tool results in an extra
> > > "Index:"
> > > header being printed. Here is a test case:
> > Thanks Alexey, patch committed in r1380697.
>
> Thanks Stefan. Is it possible to have this patch nominated for 1.7 backport?
It already is: https://svn.apache.org/repos/asf/subversion/branches/1.7.x/STATUS
>
> Regards,
> Alexey.
Re: [PATCH] SVN 1.7 and 1.8 generates extra header when diffing with external tool
Posted by Alexey Neyman <st...@att.net>.
On Tuesday, September 04, 2012 05:56:27 PM Stefan Sperling wrote:
> On Fri, Aug 31, 2012 at 03:24:18PM -0700, Alexey Neyman wrote:
> > Hi SVN developers,
> >
> > I noticed an issue which prevents the 'post-review' tool from ReviewBoard
> > (http://www.reviewboard.org) from working with Subversion 1.7.x and trunk
> > (1.8.0-dev).
> >
> > This post-review tool uses external diff command to generate a diff file.
> > With Subversion 1.7 and 1.8, using external diff tool results in an extra
> > "Index:"
> > header being printed. Here is a test case:
> Thanks Alexey, patch committed in r1380697.
Thanks Stefan. Is it possible to have this patch nominated for 1.7 backport?
Regards,
Alexey.
Re: [PATCH] SVN 1.7 and 1.8 generates extra header when diffing with
external tool
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Aug 31, 2012 at 03:24:18PM -0700, Alexey Neyman wrote:
> Hi SVN developers,
>
> I noticed an issue which prevents the 'post-review' tool from ReviewBoard
> (http://www.reviewboard.org) from working with Subversion 1.7.x and trunk
> (1.8.0-dev).
>
> This post-review tool uses external diff command to generate a diff file. With
> Subversion 1.7 and 1.8, using external diff tool results in an extra "Index:"
> header being printed. Here is a test case:
Thanks Alexey, patch committed in r1380697.
Re: [PATCH] SVN 1.7 and 1.8 generates extra header when diffing with external tool
Posted by Alexey Neyman <st...@att.net>.
Could somebody confirm this issue and/or apply the patch?
If the patch is accepted, could it be applied to 1.7.x branch as well?
Thanks,
Alexey.
On Friday, August 31, 2012 03:24:18 pm Alexey Neyman wrote:
> Hi SVN developers,
>
> I noticed an issue which prevents the 'post-review' tool from ReviewBoard
> (http://www.reviewboard.org) from working with Subversion 1.7.x and trunk
> (1.8.0-dev).
>
> This post-review tool uses external diff command to generate a diff file.
> With Subversion 1.7 and 1.8, using external diff tool results in an extra
> "Index:" header being printed. Here is a test case:
>
> [[[
> repo=/tmp/repo.test
> wc=/tmp/wc.test
>
> svnadmin create $repo
> svn mkdir -m "create subdir" file://$repo/dir1
> svn co file://$repo $wc
> cd $wc
>
> echo hi > dir1/file2
> svn add dir1/file2
> svn ps foo:bar baz dir1/file2
> svn mv dir1 dir2
> svn diff --diff-cmd=diff dir2 > output.diff
> ]]]
>
> It produces the following diff file:
> [[[
> Index: dir2/file2
> ===================================================================
> --- dir2/file2 (revision 0)
> +++ dir2/file2 (working copy)
> @@ -0,0 +1 @@
> +hi
> Index: dir2/file2
> ===================================================================
> --- dir2/file2 (revision 1)
> +++ dir2/file2 (working copy)
>
> Property changes on: dir2/file2
> ___________________________________________________________________
> Added: foo:bar
> ## -0,0 +1 ##
> +baz
> \ No newline at end of property
> ]]]
>
> Not only there are two "Index:" headers printed, but the second one is
> incorrect: it refers to dir2/file2 as being present in revision 1. The
> post- review tool, seeing this header, fails to find the origin of
> dir2/file2 in revision 1 and produces an error.
>
> The reason is that the branch in diff_content_changed() that invokes an
> external diff tool does not mark the path as visited even though it emits
> the "Index:" header. Marking it as visited is sufficient, as the header is
> incorrect only if the file is added - in which case, one of the branches
> in diff_content_changed() is going to output the diff (diff_cmd_baton-
>
> >force_empty is set for added files). Therefore, diff_prop_changed() will
>
> never output "Index:" header for added files.
>
> Patch attached.
>
> [[[
> * subversion/libsvn_client/diff.c:
> (diff_content_changed): Mark path as visited if diffed by external tool.
> ]]]
>
> Regards,
> Alexey.