Hi. Sorry for the delay reviewing this patch.
It's impressive that you were able to get this far, but I'm afraid there
are some difficult subtleties to consider beyond what you've taken care
of.
On Sun, 2004-06-13 at 17:36, Henry Baldursson wrote:
> One problem I'm having is that my patch appears to work on file://
> access, and not http://. I don't have the resources atm to fully test
> this, but I would have thought implementing the ghost entry addition in
> libsvn_repos would have it work for every protocol since it is a layer
> on top of libsvn_fs and all the protocols should use it.
You would need to modify both network layers to pass through the
add_ghost_entry calls, while worrying about compatibility with old
clients and servers.
(You would also need to add a handler to the default editor.)
> Index: subversion/libsvn_wc/lock.c
> ===================================================================
> - --- subversion/libsvn_wc/lock.c (revision 9980)
> +++ subversion/libsvn_wc/lock.c (working copy)
> @@ -431,6 +431,7 @@
> ~ && (entry->schedule != svn_wc_schedule_add)
> ~ && (entry->schedule != svn_wc_schedule_replace))
> ~ || entry->kind != svn_node_dir
> + || entry->incomplete
> ~ || ! strcmp (entry->name, SVN_WC_ENTRY_THIS_DIR))
> ~ continue;
I think that's unsafe. Just because a directory is incomplte doesn't
mean there's no need to lock it.
> +add_ghost_entry (const char *path,
[...]
> + entry.kind = kind;
> + entry.incomplete = TRUE;
> + entry.absent = FALSE;
> + entry.deleted = FALSE;
> +
> + SVN_ERR (svn_wc_adm_retrieve (&adm_access, eb->adm_access,
> + pb->path, pool));
> + SVN_ERR (svn_wc__entry_modify (adm_access, name, &entry,
> + (SVN_WC__ENTRY_MODIFY_KIND |
> + SVN_WC__ENTRY_MODIFY_DELETED |
> + SVN_WC__ENTRY_MODIFY_INCOMPLETE |
> + SVN_WC__ENTRY_MODIFY_ABSENT),
> + TRUE /* immediate write */, pool));
Simply making an incomplete entry doesn't seem right. To properly
support recursive checkouts, we either need the notion of a directory
which is deliberately incomplete ("I don't have anything other than what
I said, but don't give me adds for things I don't have") or the notion
of an entry which deliberately isn't there ("I don't have this and I
don't want it either"). We don't have a way to represent that in the wc
right now.
> ~ if (!recurse && ((s_entry && s_entry->kind == svn_node_dir)
> ~ || (t_entry && t_entry->kind == svn_node_dir)))
> + {
> + SVN_ERR (b->editor->add_ghost_entry (e_path, svn_node_dir,
> dir_baton, pool));
> ~ return skip_path_info (b, e_path);
> + }
I think this means if I perform a non-recursive update in a directory
that does have some children, I'll get an add_ghost_entry from the
server for each of those children as well as the ones I don't have
(which I might already know about). That doesn't seem right.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jun 18 04:05:20 2004