On Tue, Jan 13, 2009 at 05:53:43PM +0000, Julian Foad wrote:
> Stefan and I just had a short debugging session. I think in merge_file()
> it's getting confused between the file that had modifications ('alpha')
> and the target where it's trying to put them ('alpha.moved') and making
> wrong if/else decisions, ending up calling
> svn_wc__merge_internal(target="alpha.moved") when it shouldn't.
>
> I've added "###" comments about this into that function. That's all I
> can do today. Anyone else?
>
> Putting the issue number in the Subject line so I can find the thread
> again.
Here's a diff that "fixes" the issue, or rather, illustrates that
the root of the problem is some incompatibility between copy
improvements and tree conflicts:
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 35215)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -661,6 +661,7 @@
*copyfrom_path = NULL;
*copyfrom_rev = SVN_INVALID_REVNUM;
+#if 0 /* copyfrom functionality has been disabled, see issue #3354 */
if (b->send_copyfrom_args)
{
/* Find the destination of the nearest 'copy event' which may have
@@ -700,6 +701,7 @@
}
}
}
+#endif
return b->editor->add_file(path, parent_baton,
*copyfrom_path, *copyfrom_rev,
While this diff would technically enable us to remove this blocking
issue from TODO-1.6, I'd rather like to find a proper fix, rather
than committing this diff.
I guess a proper fix would either:
1) Reconcile the two features in a way that prevents them from
stepping on each others' toes.
This might actually be a fairly simple fix once it is clear
what needs to be done. I don't understand yet what needs to
be done. It does not look like anyone else does either, because
the issue is still open :)
From #svn-dev:
<stsp> my brain is honestly overloaded with the task of imagining all the
stuff that happens during the update
<stsp> it's complex because tree conflicts do their thing, copy improvements
do their thing, and normal update does its thing
<stsp> and I think somewhere they are stepping on each others' toes, but I
don't know yet who is stepping on whom
<julianf> yeah, we really need to separate it better.
Anyone willing to help?
2) Back out copy improvements completely.
First off, here's what Ben Collins-Sussman said about this in #svn-dev:
<sussman> I say, feel free to rewrite or scratch the copy-impromevments
<sussman> I agree that we need a more holistic approach
<sussman> and TC seems like it's much more holistic
But this might be easier said than done, especially so close
before the 1.6 release.
Some food for thought:
The copy improvements made in 1.5 were designed to mitigate the problem
of losing local changes made to a file which is moved during an update.
Copy improvements...
a) ... are known to only work when an update carries out operations
in a certain order. More precisely, if the update happens to carry
out the deletion of the file before the corresponding add,
local changes are *not* moved over to the new location.
Users have reported that they see no effect from the improvements made,
see comment by Yann FOUBERT in issue #503
http://subversion.tigris.org/issues/show_bug.cgi?id=503
Effectively the feature is non-deterministic from the user's
point of view.
b) ... are a bit redundant now that we flag this situation as
a tree conflict anyway. Then again, they save users from
manually moving local changes over to a moved file.
c) ... are planned to be re-worked in a reliably way eventually
as part of the tree-conflicts effort. But note that this is a
plan, or rather, and idea. There are no concrete design proposals
yet. We may want to consider this vapour-ware for now, rather
than a serious alternative to the current copy improvements
implementation.
Copy improvements involved many public API modifications, and
RA protocol modifications (mostly adding "copyfrom args").
These would need to stay until 2.0 at least, but operations
involving this API could probably be turned into no-ops.
Depending on how 2c) will eventually look like, we might end
up reusing this API or parts of it.
For example, r26503 is a huge diff which touches lots of public API.
r26471 added some small bits of public API, too.
Opinions?
Stefan
Received on 2009-01-13 21:14:54 CET