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/29 01:18:00 UTC

Re: svn commit: r33938 - in trunk: . notes/tree-conflicts subversion/include subversion/include/private subversion/libsvn_auth_kwallet subversion/libsvn_client subversion/libsvn_subr subversion/libsvn_wc subversion/svn subversion/tests/cmdline subversion/tests/cmdline/svntest subversion/tests/libsvn_subr tools/buildbot/slaves/win32-xp-VS2005 tools/server-side www


sbutler@tigris.org wrote:
> Author: sbutler
> Date: Tue Oct 28 09:22:25 2008
> New Revision: 33938
> 
> Log:
> Merge the tree-conflicts-notify branch to trunk.

This

> Merge uses a different set of callbacks, which we haven't touched yet,
> but calls the same notify() function.  To be fixed separately.

is related to this:

> Modified: trunk/subversion/tests/cmdline/merge_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/merge_tests.py?pathrev=33938&r1=33937&r2=33938
> ==============================================================================
> --- trunk/subversion/tests/cmdline/merge_tests.py	Tue Oct 28 08:56:45 2008	(r33937)
> +++ trunk/subversion/tests/cmdline/merge_tests.py	Tue Oct 28 09:22:25 2008	(r33938)
> @@ -12256,7 +12256,7 @@ def del_differing_file(sbox):
>        "--- Merging r2 into '%s':\n" % dir_G2,
>        "C    %s\n" % dir_G2,
>        "Summary of conflicts:\n",
> -      "  Tree conflicts: 1\n",
> +      "  Text conflicts: 1\n",
>        "  Skipped paths: 1\n"])
>  
>    svn_merge(s_rev_pi, source, target, [
> @@ -12264,7 +12264,7 @@ def del_differing_file(sbox):
>        "--- Merging r3 into '%s':\n" % dir_G2,
>        "C    %s\n" % dir_G2,
>        "Summary of conflicts:\n",
> -      "  Tree conflicts: 1\n",
> +      "  Text conflicts: 1\n",
>        "  Skipped paths: 1\n"])
>  
>  
> @@ -12287,7 +12287,7 @@ def del_differing_file(sbox):
>        "--- Merging r2 into '%s':\n" % dir_G3,
>        "C    %s\n" % dir_G3,
>        "Summary of conflicts:\n",
> -      "  Tree conflicts: 1\n",
> +      "  Text conflicts: 1\n",
>        "  Skipped paths: 1\n"])
>  
>    svn_merge(s_rev_pi, source, target, [
> @@ -12295,7 +12295,7 @@ def del_differing_file(sbox):
>        "--- Merging r3 into '%s':\n" % dir_G3,
>        "C    %s\n" % dir_G3,
>        "Summary of conflicts:\n",
> -      "  Tree conflicts: 1\n",
> +      "  Text conflicts: 1\n",
>        "  Skipped paths: 1\n"])
>  
>    os.chdir(saved_cwd)


This change feels very wrong to me. I'd rather XFAIL these tests until merge
notification is fixed. It says "Text conflicts", but these are tree
conflicts!! Furthermore, some tests I wrote about yesterday are inherently
depending on victims' parent dirs being in conflict, so it's a complete
delusion to pass them while taking away this behaviour.

Maybe it's a little obsessive, but it just feels so wrong to PASS this
behaviour. We could go back to counting text conflicts on directories as
tree-conflicts. But we'd use the time better changing merge notification.

Oh, I guess I've volunteered. ;)

So about the strategy. Someone said along the way it would be better to use
an editor instead of the diff callbacks. But we're not going into this now,
are we? I guess it'd take a lot of time. Let's enhance the diff_callbacks
for now. Do you think this is the right decision?

~Neels


Re: svn commit: r33938 - in trunk: . notes/tree-conflicts subversion/include subversion/include/private subversion/libsvn_auth_kwallet subversion/libsvn_client subversion/libsvn_subr subversion/libsvn_wc subversion/svn subversion/tests/cmdline subversion/tests/cmdline/svntest subversion/tests/libsvn_subr tools/buildbot/slaves/win32-xp-VS2005 tools/server-side www

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Oct 29, 2008 at 02:18:00AM +0100, Neels J. Hofmeyr wrote:
> sbutler@tigris.org wrote:
> > Modified: trunk/subversion/tests/cmdline/merge_tests.py
> > URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/merge_tests.py?pathrev=33938&r1=33937&r2=33938
> > ==============================================================================
> > --- trunk/subversion/tests/cmdline/merge_tests.py	Tue Oct 28 08:56:45 2008	(r33937)
> > +++ trunk/subversion/tests/cmdline/merge_tests.py	Tue Oct 28 09:22:25 2008	(r33938)
> > @@ -12256,7 +12256,7 @@ def del_differing_file(sbox):
> >        "--- Merging r2 into '%s':\n" % dir_G2,
> >        "C    %s\n" % dir_G2,
> >        "Summary of conflicts:\n",
> > -      "  Tree conflicts: 1\n",
> > +      "  Text conflicts: 1\n",
> >        "  Skipped paths: 1\n"])
> >  
> >    svn_merge(s_rev_pi, source, target, [
> > @@ -12264,7 +12264,7 @@ def del_differing_file(sbox):
> >        "--- Merging r3 into '%s':\n" % dir_G2,
> >        "C    %s\n" % dir_G2,
> >        "Summary of conflicts:\n",
> > -      "  Tree conflicts: 1\n",
> > +      "  Text conflicts: 1\n",
> >        "  Skipped paths: 1\n"])
> >  
> >  
> > @@ -12287,7 +12287,7 @@ def del_differing_file(sbox):
> >        "--- Merging r2 into '%s':\n" % dir_G3,
> >        "C    %s\n" % dir_G3,
> >        "Summary of conflicts:\n",
> > -      "  Tree conflicts: 1\n",
> > +      "  Text conflicts: 1\n",
> >        "  Skipped paths: 1\n"])
> >  
> >    svn_merge(s_rev_pi, source, target, [
> > @@ -12295,7 +12295,7 @@ def del_differing_file(sbox):
> >        "--- Merging r3 into '%s':\n" % dir_G3,
> >        "C    %s\n" % dir_G3,
> >        "Summary of conflicts:\n",
> > -      "  Tree conflicts: 1\n",
> > +      "  Text conflicts: 1\n",
> >        "  Skipped paths: 1\n"])
> >  
> >    os.chdir(saved_cwd)
> 
> 
> This change feels very wrong to me. I'd rather XFAIL these tests until merge
> notification is fixed. It says "Text conflicts", but these are tree
> conflicts!!

+1, please let's mark these XFAIL. It makes more sense for these
tests to reflect what the output should be.

As an aside: I took a brief look over the rest of the diff,
looks fine to me. Great work lads :)

Stefan

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