On Sun, 2008-08-24 at 13:21 +0200, Stefan Sperling wrote:
> On Sat, Aug 23, 2008 at 10:43:11PM +0200, Neels Hofmeyr wrote:
> > We've got these functions:
> >
> > do_entry_deletion() "Helper for delete_entry()."
> > delete_entry() Yes, it calls do_entry_deletion().
> > close_edit() Why, this one *also* calls do_entry_deletion().
> >
[...]
> > So, I investigated and came up with below patch. It does this:
> >
> > - do *not* skip tree-conflicts testing for that special case
> > (parent_adm_access == NULL), retrieve adm_access as done on trunk.
> >
> > That itself causes an error message "Directory '%s' is missing", in
> > check_tree_conflict() calling entry_has_local_mods().
> >
> > So, I gather, this is actually use case 3: update tries to delete, but the
> > user has locally removed the directory (without marking it as removed, though).
>
> That is not use case 3. It would be use case 3 if the working copy
> meta data agreed with what is on disk.
>
> Anytime disk and meta data disagree, we just call it "obstruction"
> or "missing", because we can't proceed anyway. The user messed up the
> working copy and cannot reasonably expect us to know what's going on.
Yes, we decided that we should handle "obstructions" (all cases where
the disk is inconsistent with the metadata, including where the disk
file or directory is missing), as an error condition, first, and after
that proceed to checking for tree conflicts once we know that the WC
state is sane in itself.
However, I haven't paid attention to the handling of obstructions in the
implementation. If cases of obstruction get flagged as a tree conflict,
maybe with a "reason" that is misleading because we're not specifically
diagnosing the obstruction, well, to me that is not a big concern at the
moment. At least the problem will be brought to the user's attention.
Once tree conflicts are working properly in non-obstructed cases, then
we can look at improving the behaviour in cases of obstruction.
[...]
> Sounds all sane to me, but I'll leave double checking to Julian.
I committed Neels' later version of this patch without addressing these
points. (I forgot to check back against this email.)
> > Index: subversion/libsvn_wc/update_editor.c
> > ===================================================================
> > --- subversion/libsvn_wc/update_editor.c (revision 32658)
> > +++ subversion/libsvn_wc/update_editor.c (working copy)
> > @@ -1198,31 +1198,47 @@
> > else
> > {
> > svn_boolean_t modified;
> > + svn_node_kind_t kind;
> >
> > - /* If we are about to delete a path that has local mods,
> > + /* If we are about to delete a path that is locally missing,
> > * mark the containing directory as tree conflicted.
> > - * This is tree conflict use case 2 as described in the
> > - * paper attached to issue #2282
> > + * This is tree conflict use case 3.
> > * See also notes/tree-conflicts/detection.txt
>
> detection.txt says:
>
> If 'svn update' deletes a file that has been scheduled for deletion in
> the working copy, the file is a tree conflict victim
>
> But the file has not been scheduled for deletion in this case.
>
> I'd suggest simply stating:
>
> /* If we are about to delete a path that is missing from disk
> * but present in meta data, raise a tree conflict.
> * The merge cannot succeed because the working copy is broken.
> */
>
> Essentially, this is the inverse of our 'obstructed' tree conflict
> cases, which happen when a path is present on disk which meta data
> says should not be there (yet).
We ought to tweak that comment.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-08 20:54:16 CEST