Re: Tree conflicts from incoming entry prop edits?

Julian Foad wrote:
>>> C. Michael Pilato wrote:
>>>> I was attempting to test a theory today. The theory goes like this: when
>>>> committed the deleted of a locked file over HTTP, if the commit fails and
>>>> --no-unlock was *not* specified, the lock will be dropped by the server but
>>>> the client won't know it until it next updates. I think I proved that to be
>>>> true, but I ran into a tree conflict along the way. I setup my test,
>>>> eventually resulting in my having a working copy with a deleted, locked
>>>> file. I add a pre-commit hook to my repository that fails all commits. I
>>>> try to commit from my working copy, and (surprise!) the commit fails. But
>>>> then I tried to update that working copy, and got a tree conflict. What?
>>>>
>>>> $ svn st lock-wc
>>>> D KC lock-wc/foo
>>>> > local delete, incoming edit upon update
>>>> $
>
> OK, the tree conflict.
>
> In subversion/libsvn_wc/update_editor.c:open_file(), we say
>
> /* Is this path the victim of a newly-discovered tree conflict? */
> if (!already_conflicted)
> SVN_ERR(check_tree_conflict(&tree_conflict, eb, local_abspath,
> pb->log_accum, entry,
> svn_wc_conflict_action_edit,
> svn_node_file, fb->new_URL, pool));
>
> I tested using file:// (I know your script says I must use http:// but
> I'm trying file://), and can reproduce the tree conflict. We assume that
> because the editor driver called "open_file" it is making an "edit" to
> this file, either a text edit or a property edit. Because the file is
> locally deleted, this raises a tree conflict. It looks like that
> assumption is wrong, or the other possible explanation could be the
> editor is being driven wrongly.

I believe the editor is being properly driven. The tree conflict code just
failed to be another instance of the special-casing required for the entry
props functionality to work.

> Then I disabled the tree conflict detection by commenting out the
> above-quoted lines, to find out why it's opened the file. In GDB I see
> the editor driver is doing neither a text edit nor a (user-visible)
> property edit, but it is calling
>
> open_file()
> change_file_prop(svn:entry:committed-rev)
> change_file_prop(svn:entry:committed-date)
> change_file_prop(svn:entry:last-author)
> change_file_prop(svn:entry:uuid)
> close_file()
>
> It doesn't seem to do anything with the lock token, which is odd.

It's not so odd when you keep in mind that the very reason I wanted folks to
run the recipe over HTTP was because "doing something with the lock token"
was the original problem I was trying to validate. :-)

> It looks like we should defer the tree conflict detection until
> "apply_textdelta" and "change_file_prop" callbacks instead of assuming
> that one of those will be called, and we should ensure that entry-prop
> changes do not trigger a tree conflict.

At first blush, I'd say "Yup." But my ignorance of the various tree
conflict behaviors causes me some concern: were this a true tree conflict
scenario, what is the penalty for only detecting the conflict potentially
after entry-props have been passed through the system. Will the entry props
cause changes to occur on the wrong node (the reason for the tree conflict
in the first place)? I can't answer that myself right now.

> On the other hand, I'm wondering whether the server should be opening
> that file at all. If I change your script around to lock the file after
> it is initially committed, and then unlock it just before deleting it
> from disk, then open_file/change_file_prop/close_file are not called at
> all. Is that asymmetry intended or part of the problem here?

I can explain the asymmetry in the DAV case. I cannot in the file:// case.

Another thing I didn't point out before, but can't explain: in the DAV
case, it isn't only the lock token that is deleted by the server, but also
the last-committed-author entry prop! Not the last-commmitted-rev or -date,
but the -author. Why? I dunno.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2395598