Folks, some review (and discussion about compatibility) would be
greatly appreciated. I've fixed (I think) both the client and the
server in this one change. Ben Collins-Sussman and I actually worked
out whether or not some sort of compatibility upgrade path should be
taken, and agreed that in this case, no. In the worst case, I think
we'll be trading one bug (working copies with invalid wc props) for
another (inability to complete a switch operation), but in most cases
the workaround is straightforward. Instead of doing:
svn switch REPOS-URL /path/to/wc
do:
(cd /path/to/wc, svn switch REPOS-URL)
<<<
Fix bugs in svn switch over ra-dav.
The history of this change is complicated, but goes something like
this:
Once upon a time there were two lovers, each Subversion-y, each of
1.0 caliber. One was a server; the other, a client. These two
transmitted messages over the internet (sometimes using secret
codes, even) in order to coordinate the development of their
budding relationship. Each had flaws, but both simply avoided
activities which brought the others' flaw to light.
Then one day, the client met a man named Greg Hudson. Now, this
fellow was exceedingly intelligent, with an eye for simplifying the
lives of those around him. With good intent, he encouraged the
client to begin improving her flaws, while exploring the formerly
untrodden paths in the her relationship with the server. The
server responded to this new direction also with good intent -- but
alas, with imperfection. His flaws were brought to light, not as
obvious evils, but as a subtle sort of destruction, as the dryrot
of a rubber band is hidden until the band is stretched. The result
was perplexing and disappointing -- the land was stained with
invalid working copy props far and near.
Something had to be done.
This change should be evaluated in conjunction with r9705 and r9698,
in which the client was changed to begin driving the server in a way
it wasn't prepared to handle.
* subversion/mod_dav_svn/update.c
(update_ctx_t): Add 'target' member.
(item_baton_t): Add 'parent' member.
(make_child_baton): Populate the 'parent' member of the
item_baton_t. And calculate path3 more accurately in the presence
of a target.
(dav_svn__update_report): Populate the 'target' member of the
update_ctx_t baton, and drive the resource walk based on the
anchor, not the target.
* subversion/libsvn_ra_dav/fetch.c
(start_element): Revert the 9705 change -- accept the resource path
as given to us from the server (and so go back to expecting the
server to drive the resource walk relative to the anchor).
>>>
Index: subversion/mod_dav_svn/update.c
===================================================================
--- subversion/mod_dav_svn/update.c (revision 11918)
+++ subversion/mod_dav_svn/update.c (working copy)
@@ -46,6 +46,7 @@
svn_fs_root_t *rev_root;
const char *anchor;
+ const char *target;
/* if doing a regular update, then dst_path == anchor. if this is a
'switch' operation, then this field is the fs path that is being
@@ -75,9 +76,10 @@
} update_ctx_t;
-typedef struct {
+typedef struct item_baton_t {
apr_pool_t *pool;
update_ctx_t *uc;
+ struct item_baton_t *parent; /* the parent of this item. */
const char *name; /* the single-component name of this item */
const char *path; /* a telescoping extension of uc->anchor */
const char *path2; /* a telescoping extension of uc->dst_path */
@@ -281,6 +283,7 @@
baton->pool = pool;
baton->uc = parent->uc;
baton->name = svn_path_basename(path, pool);
+ baton->parent = parent;
/* Telescope the path based on uc->anchor. */
baton->path = svn_path_join(parent->path, baton->name, pool);
@@ -288,9 +291,15 @@
/* Telescope the path based on uc->dst_path in the exact same way. */
baton->path2 = svn_path_join(parent->path2, baton->name, pool);
- /* Telescope the third path: it's relative, not absolute, to dst_path. */
- baton->path3 = svn_path_join(parent->path3, baton->name, pool);
-
+ /* Telescope the third path: it's relative, not absolute, to
+ dst_path. Now, we gotta be careful here, because if this
+ operation had a target, and we're it, then we have to use the
+ basename of our source reflection instead of our own. */
+ if ((*baton->uc->target) && (! parent->parent))
+ baton->path3 = svn_path_join(parent->path3, baton->uc->target, pool);
+ else
+ baton->path3 = svn_path_join(parent->path3, baton->name, pool);
+
return baton;
}
@@ -685,7 +694,10 @@
DEBUG_CR, base_revision) );
}
- SVN_ERR( send_vsn_url(b, pool) );
+ /* Only transmit the root directory's Version Resource URL if
+ there's no target. */
+ if (! *uc->target)
+ SVN_ERR( send_vsn_url(b, pool) );
if (uc->resource_walk)
SVN_ERR( dav_svn__send_xml(uc->bb, uc->output,
@@ -1194,6 +1206,7 @@
uc.resource = resource;
uc.output = output;
uc.anchor = src_path;
+ uc.target = target;
uc.bb = apr_brigade_create(resource->pool, output->c->bucket_alloc);
uc.pathmap = NULL;
if (dst_path) /* we're doing a 'switch' */
@@ -1359,83 +1372,56 @@
/* The potential "resource walk" part of the update-report. */
if (dst_path && resource_walk) /* this was a 'switch' operation */
{
- /* Sanity check: if we switched a file, we can't do a resource
- walk. dir_delta would choke if we pass a filepath as the
- 'target'. Also, there's no need to do the walk, since the
- new vsn-rsc-url was already in the earlier part of the report. */
- svn_node_kind_t dst_kind;
-
- serr = svn_fs_check_path(&dst_kind, uc.rev_root, dst_path,
- resource->pool);
+ /* send a second embedded <S:resource-walk> tree that contains
+ the new vsn-rsc-urls for the switched dir. this walk
+ contains essentially nothing but <add> tags. */
+ svn_fs_root_t *zero_root;
+ serr = svn_fs_revision_root(&zero_root, repos->fs, 0,
+ resource->pool);
if (serr)
{
derr = dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "Failed to find the kind of a path",
+ "Failed to find the revision root",
resource->pool);
goto cleanup;
}
- if (dst_kind == svn_node_dir)
+ serr = dav_svn__send_xml(uc.bb, uc.output, "<S:resource-walk>" DEBUG_CR);
+ if (serr)
{
- /* send a second embedded <S:resource-walk> tree that contains
- the new vsn-rsc-urls for the switched dir. this walk
- contains essentially nothing but <add> tags. */
- svn_fs_root_t *zero_root;
- serr = svn_fs_revision_root(&zero_root, repos->fs, 0,
- resource->pool);
- if (serr)
- {
- derr = dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "Failed to find the revision root",
- resource->pool);
- goto cleanup;
- }
+ derr = dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "Unable to begin resource walk",
+ resource->pool);
+ goto cleanup;
+ }
- serr = dav_svn__send_xml(uc.bb, uc.output,
- "<S:resource-walk>" DEBUG_CR);
- if (serr)
- {
- derr = dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "Unable to begin resource walk",
- resource->pool);
- goto cleanup;
- }
+ uc.resource_walk = TRUE;
- uc.resource_walk = TRUE;
-
- /* Compare subtree DST_PATH within a pristine revision to
- revision 0. This should result in nothing but 'add' calls
- to the editor. */
- serr = svn_repos_dir_delta(/* source is revision 0: */
- zero_root, "", "",
- /* target is 'switch' location: */
- uc.rev_root, dst_path,
- /* re-use the editor */
- editor, &uc,
- dav_svn_authz_read,
- &arb,
- FALSE, /* no text deltas */
- recurse,
- TRUE, /* send entryprops */
- FALSE, /* don't ignore ancestry */
- resource->pool);
- if (serr)
- {
- derr = dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "Resource walk failed.",
- resource->pool);
- goto cleanup;
- }
+ /* Compare subtree DST_PATH within a pristine revision to
+ revision 0. This should result in nothing but 'add' calls
+ to the editor. */
+ serr = svn_repos_dir_delta(zero_root, "", target,
+ uc.rev_root, dst_path,
+ /* re-use the editor */
+ editor, &uc, dav_svn_authz_read,
+ &arb, FALSE /* text-deltas */, recurse,
+ TRUE /* entryprops */,
+ FALSE /* ignore-ancestry */, resource->pool);
+ if (serr)
+ {
+ derr = dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "Resource walk failed.", resource->pool);
+ goto cleanup;
+ }
- serr = dav_svn__send_xml(uc.bb, uc.output,
- "</S:resource-walk>" DEBUG_CR);
- if (serr)
- {
- derr = dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "Unable to complete resource walk.",
- resource->pool);
- goto cleanup;
- }
+ serr = dav_svn__send_xml(uc.bb, uc.output,
+ "</S:resource-walk>" DEBUG_CR);
+ if (serr)
+ {
+ derr = dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "Unable to complete resource walk.",
+ resource->pool);
+ goto cleanup;
}
}
Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 11918)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -1578,7 +1578,7 @@
case ELEM_resource:
att = svn_xml_get_attr_value("path", atts);
/* ### verify we got it. punt on error. */
- rb->current_wcprop_path = svn_path_join(rb->target, att, rb->ras->pool);
+ rb->current_wcprop_path = apr_pstrdup(rb->ras->pool, att);
break;
case ELEM_open_directory:
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Nov 16 04:48:29 2004