[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r18804 - in branches/fs-atomic-renames/subversion: libsvn_fs_base tests/libsvn_fs

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: 2006-03-10 16:55:38 CET

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 <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Received on Fri Mar 10 16:59:32 2006

This is an archived mail posted to the Subversion Dev mailing list.