So I've been banging my head against the "make history work with
moves" for the past few days, and I though I'd share some of the fun.
Here's my current work-in-progress patch against the fs-atomic-renames
branch. Note the XXX hardcoded for our current test comments strewn
about:
Index: subversion/libsvn_fs_base/tree.c
===================================================================
--- subversion/libsvn_fs_base/tree.c (revision 18770)
+++ subversion/libsvn_fs_base/tree.c (working copy)
@@ -556,7 +556,6 @@
const char *child_copy_id, *parent_copy_id;
const char *id_path = NULL;
const char *move_id;
- copy_t *copy;
/* Make some assertions about the function input. */
assert(child && child->parent && txn_id);
@@ -4042,7 +4041,20 @@
if ((*copy)->kind != copy_kind_soft_copy)
return SVN_NO_ERROR;
}
+ else if (parent_path->copy_inherit == copy_id_inherit_fixed)
+ {
+ /* Ok, this came from a move, so we get our copy via the move_id we
+ * stashed in copy_data. */
+ const char *move_id = parent_path->copy_data;
+ SVN_ERR(svn_fs_bdb__get_copy(copy, fs, move_id, trail, pool));
+ if ((*copy)->kind != copy_kind_soft_move)
+ {
+ *copy_id = move_id;
+ return SVN_NO_ERROR;
+ }
+ }
+
/* Otherwise, our answer is dependent upon our parent. */
return examine_copy_inheritance(copy_id, copy, fs,
parent_path->parent, trail, pool);
@@ -4176,6 +4188,7 @@
const char *remainder;
dag_node_t *dst_node;
const char *copy_dst;
+ svn_fs_path_change_t *change;
/* Get the COPY record if we haven't already fetched it. */
if (! copy)
@@ -4186,8 +4199,21 @@
SVN_ERR(svn_fs_base__dag_get_node(&dst_node, fs,
copy->dst_noderev_id,
trail, trail->pool));
- copy_dst = svn_fs_base__dag_get_created_path(dst_node);
+ if (copy->kind != copy_kind_move)
+ copy_dst = svn_fs_base__dag_get_created_path(dst_node);
+ else
+ {
+ /* XXX Look this up in changes?
+ *
+ * Oh wait! We can't! The stuff in changes is keyed
+ * off of DESTINATION paths, not SOURCE paths, like we
+ * have access to, so it's TOTALLY USELESS for finding
+ * the destination of a copy based on the source of it.
+ */
+ copy_dst = "/Z"; /* XXX hardcoded for our current test... */
+ }
+
/* If our current path was the very destination of the copy,
then our new current path will be the copy source. If our
current path was instead the *child* of the destination of
@@ -4206,12 +4232,21 @@
/* If we get here, then our current path is the destination
of, or the child of the destination of, a copy. Fill
in the return values and get outta here. */
- SVN_ERR(svn_fs_base__txn_get_revision
- (&src_rev, fs, copy->src_txn_id, trail, trail->pool));
- SVN_ERR(svn_fs_base__txn_get_revision
- (&dst_rev, fs,
- svn_fs_base__id_txn_id(copy->dst_noderev_id),
- trail, trail->pool));
+ if (copy->kind != copy_kind_move)
+ {
+ SVN_ERR(svn_fs_base__txn_get_revision
+ (&src_rev, fs, copy->src_txn_id, trail, trail->pool));
+ SVN_ERR(svn_fs_base__txn_get_revision
+ (&dst_rev, fs,
+ svn_fs_base__id_txn_id(copy->dst_noderev_id),
+ trail, trail->pool));
+ }
+ else
+ {
+ src_rev = 1; /* XXX hardcoded for our current test... */
+ dst_rev = 2; /* XXX hardcoded for our current test... */
+ }
+
src_path = svn_path_join(copy->src_path, remainder,
trail->pool);
if (copy->kind == copy_kind_soft_copy
So the way this is currently going is as follows:
Inside examine_copy_inheritance we now handle the
copy_id_inherit_fixed case, which means that this was in fact a move
and thus we need to use our stashed away move_id data to get the
associated copy. Then, back in txn_body_history_prev where we call
examine_copy_inheritance we now have access to the proper copy. Cool.
Here's where it gets problematic. We now have a few places where we
need to handle the "this was a move" case specially, namely where we
calculate the copy_dst, the src_rev and the dst_rev.
My first instinct (well, actually it's about my 12th instinct, but
it's the one I'm beating my head against now) was to find the copy
destination by looking up the changes record from the rev that
included the move. So we grab the txn id from parent_path's parent
and look it up. This will get us the copies, but we have a bit of a
snag. The copies store destination paths, not source paths, but more
importantly they're also KEYED on destination paths, so we can't even
GET TO the proper changes record based on the information currently
available.
Once we've got the dest path, it seems like it shouldn't be all that
hard to go from there to the source and dest revisions, but I'm kinda
stuck on the path part...
Work arounds? Well, I could add the source path to the changes record
(optional, only there for a move), then add a "hey, get me all the
changes with source paths in a hash keyed off the source path" call,
but that seems kind of hideous. Alternatively I could start storing
this kind of information in the copies table, since I already have
access to a copy object associated with this move. It really seems
like I'm gonna need to get this stuff out of the changes table
eventually though, since I can't do what we do in the face of a copy
and pull the dest revision out of the copy's dest noderev id, since
the copy's dest noderev id hasn't changed, and thus points to an old
revision...
Or maybe I'm just totally misunderstanding the "right" way to go about
fixing this part of the code. I'm basically working off the "compare
how the test works with svn_fs_move and svn_fs_copy and try and 'fix'
the parts that give different answers" technique, so it's quite
possible I'm missunderstanding something fundamental here.
Anwyay, I'm just going in circles at this point, so I'm sending out
this desperate plea for help in the hopes that someone[*] with a bit
better understanding of how this is supposed to work will save me.
-garrett
* for those playing at home, that "someone" would be cmpilato ;-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 9 02:39:46 2006