You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2006/03/10 15:55:38 UTC
Re: svn commit: r18804 - in branches/fs-atomic-renames/subversion:
libsvn_fs_base tests/libsvn_fs
rooneg@tigris.org wrote:
> Author: rooneg
> Date: Thu Mar 9 17:52:40 2006
> New Revision: 18804
[...]
> Modified: branches/fs-atomic-renames/subversion/libsvn_fs_base/tree.c
> Url: http://svn.collab.net/viewcvs/svn/branches/fs-atomic-renames/subversion/libsvn_fs_base/tree.c?rev=18804&p1=branches/fs-atomic-renames/subversion/libsvn_fs_base/tree.c&p2=branches/fs-atomic-renames/subversion/libsvn_fs_base/tree.c&r1=18803&r2=18804
> ==============================================================================
> --- branches/fs-atomic-renames/subversion/libsvn_fs_base/tree.c (original)
> +++ branches/fs-atomic-renames/subversion/libsvn_fs_base/tree.c Thu Mar 9 17:52:40 2006
> @@ -4185,7 +4198,49 @@
> 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
> + {
> + /* If this was a move, the dest node IS the same as the
> + * source node, so just grabbing its created path will
> + * give us the wrong path. Instead, we need to go through
> + * the changes for the revision the move occurred in, find
> + * the appropriate change_t object, and use its path. */
> +
> + const svn_fs_id_t *id = svn_fs_base__dag_get_id(dst_node);
> + apr_array_header_t *changes;
> + change_t *change = NULL;
> + int idx;
> +
> + /* Using svn_fs_bdb__changes_fetch wouldn't buy us anything,
> + * since we need to iterate over them all looking for one that
> + * matches on the node id anyway... */
> + SVN_ERR(svn_fs_bdb__changes_fetch_raw(&changes,
> + fs,
> + copy->src_txn_id,
> + trail,
> + trail->pool));
> +
> + for (idx = 0; idx < changes->nelts; ++idx)
> + {
> + change = APR_ARRAY_IDX(changes, idx, change_t *);
> +
> + if (svn_fs_base__id_compare(id, change->noderev_id) == 0)
> + break;
> +
> + change = NULL;
> + }
> +
> + /* As far as I know this should not happen, but hey, we're reading
> + * data off of disk, so anything is theoretically possible. */
> + if (! change)
> + return svn_error_create
> + (SVN_ERR_FS_CORRUPT, NULL,
> + _("Couldn't find this move's associated change"));
> +
> + copy_dst = change->path;
> + }
>
> /* If our current path was the very destination of the copy,
> then our new current path will be the copy source. If our
Oh, man, is that ugly. There's got to be a better way than crawling the
changes table like that.
Perhaps the 'move' items in the "copies" table should grow destination
paths? We could start storing them for new copies too, so that
determining the target of a copy can be done in one step (if that path
is available, of course) instead of looking up the target node-rev-id in
the nodes table and reading its committed-path.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r18804 - in branches/fs-atomic-renames/subversion: libsvn_fs_base tests/libsvn_fs
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 3/10/06, Garrett Rooney <ro...@electricjellyfish.net> wrote:
> > Perhaps the 'move' items in the "copies" table should grow destination
> > paths? We could start storing them for new copies too, so that
> > determining the target of a copy can be done in one step (if that path
> > is available, of course) instead of looking up the target node-rev-id in
> > the nodes table and reading its committed-path.
>
> That sounds like it should work. I'll look into doing it that way today.
See r18840. Unfortunately I also found another problem, we can't just
assume that the source rev for the node is it's origin, it breaks down
when a node is moved more than once. I'm going to look into pulling
the same trick, storing enough info in the copy record to figure it
out.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18804 - in branches/fs-atomic-renames/subversion: libsvn_fs_base tests/libsvn_fs
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 3/10/06, C. Michael Pilato <cm...@collab.net> wrote:
> Oh, man, is that ugly. There's got to be a better way than crawling the
> changes table like that.
Oh, I agree completely. There's a reason it took me three days to
even come up with that solution, crawling the changes records didn't
even occur to me until I'd tried everything else. ;-)
> Perhaps the 'move' items in the "copies" table should grow destination
> paths? We could start storing them for new copies too, so that
> determining the target of a copy can be done in one step (if that path
> is available, of course) instead of looking up the target node-rev-id in
> the nodes table and reading its committed-path.
That sounds like it should work. I'll look into doing it that way today.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org