On Wed, 2010-05-05 at 11:37 +0200, Neels J Hofmeyr wrote:
> On 05/03/2010 10:25 PM, Daniel Näslund wrote:
> > On Mon, May 03, 2010 at 03:07:49PM +0200, Neels J Hofmeyr wrote:
> [...]
> >>> stat->lock_creation_date = 0;
> >>> + stat->conflicted = (tree_conflict != NULL);
> >>
> >> I don't understand -- what about prop and text conflicts? Below,
> >> ->conflicted stands for all three of them.
> >> (If this is correct, I guess it needs a comment)
> >
> > Added a comment in r940600. AFAIK, the only scenario where we can get a
> > conflict on an unversioned node is for an incoming delete to a locally
> > deleted path.
You must be talking about during an update (or switch) that results in a
tree conflict, because if it was a *merge* then the node wouldn't become
unversioned yet, it would still be scheduled for deletion, so it would
still have an "entry" and there would be no problem in storing the
metadata. That's what I'm assuming in the rest of this email.
So what about an incoming delete to a locally *edited* path, during
up/sw? That is another combination that would also leave the node
unversioned and with a tree conflict.
> (It will be troublesome to fit that case into the idea of
> > storing tree conflict info on the node instead of its parent).
>
> ah, ok, that makes sense. Shouldn't be trouble to store such tree-conflict
> in the ng-ACTUAL tree? ...but, thinking aloud a little:
>
> The entry can't be NULL for a schedule-deleted node (asked for with
> show_hidden == TRUE). Analogously, the wc-ng tables do have both ng-BASE and
> ng-WORKING nodes for a locally deleted path.
>
> So, if there is no metadata in the WC on the node, there can't be *any*
> (tree-)conflict on it! Right?
I'm not sure if you're making a statement about how we should design
WC-NG tree conflict handling, or what.
> Let's see:
>
> Another case: obstructed; there is no metadata in the WC, but a file/folder
> sits at a path. Say update wants to put something at that path: it'll create
> a base node (above theory holds -- TC needs to have metadata).
That's good.
> Ok, furthermore, say merge wants to put something there, at the obstructed
> path. It'll create a working node that is added (obstructed_add). Again,
> there is a WC metadata entry.
That's good.
> Ok, even furthermore, say there is nothing, absolutely nothing at WC path
> foo.c, and I do a two-URL merge. The URL I merge from has foo.c on the left,
> but no foo.c on the right. So technically, the merge should want to delete
> foo.c in my WC, where it never existed in any way. (I never thought about
> this before!)
That's the case commonly known as "incoming delete, locally deleted on
my branch (and committed)".
> How does merge handle that?
>
> * neels writes a test script
>
> [[[
> [foo.c never existed on trunk, and r3 deletes foo.c on unrelated branch]
> + svn merge -c3 '^/branch' trunk/
> --- Merging r3 into 'trunk':
> C trunk/foo.c
> --- Recording mergeinfo for merge of r3 into 'trunk':
> U trunk
> Summary of conflicts:
> Tree conflicts: 1
> + svn info trunk/foo.c
> Path: trunk/foo.c
> Name: foo.c
> Node Kind: none
> Tree conflict: local delete, incoming delete upon merge
> Source left: (file) file:///tmp/trunk.EKH/repos/branch/foo.c_at_2
> Source right: (file) file:///tmp/trunk.EKH/repos/branch/foo.c_at_3
>
> + svn status trunk
> M trunk
> ! C trunk/foo.c
> > local delete, incoming delete upon merge
> Summary of conflicts:
> Tree conflicts: 1
> ]]]
>
> Good grief, another hole in tree-conflicts! It says it was locally deleted
That's all good IF we understand "deleted" to mean "not here" or
"deleted, possibly a long time ago on this branch".
> and status '!',
Ah, that status doesn't look right. It should have a space instead of
"!", I think.
> but it never existed locally. It should rather say something
> like... er... 'missing' doesn't really hit home either. 'local void,
> incoming delete' ?? Do we need another conflict_reason to flag this?
It would be better if we recorded a different reason. My suggestion was
always "not here".
> Resolving this automatically would definitely need different behaviour than
> a truly locally deleted node or truly missing node.
>
> * neels attaches the test script
>
> Is this the real justification why there could possibly be a tree-conflict
> on an entry==NULL node?
Yes, I believe so.
Stefan Sperling wrote:
> What it should really be doing is realise that the node never existed
> in the branch and so deleting it is not a conflict (I know svn is not
> that smart yet and won't be for a while...)
>
> But I think the missing status notification is just fine in the
> meantime.
> I agree the text should say "local missing" instead of "local delete",
> but IIRC we don't know the difference since we're not searching branch
> history [yet].
About the reporting of the "local" state
----------------------------------------
I recommend the following states for "not present":
* "not here" if the node is not on the tip of the branch
* "deleted" if the node is scheduled for deletion
* "missing" if the node is supposed to be present but is not found on
the disk
"Missing" is true in a general English sense for any of these states,
but it might be better to reserve it to describe the case where the
metadata says the node is present but it's not there on disk.
Similarly, "local delete" could, in a general English sense, mean "it's
not here and so was presumably deleted by somebody, some time,
somewhere", but instead we should reserve it for its normal Subversion
meaning of "scheduled for deletion on the next commit".
We can determine the difference between these three states by looking at
the local WC; that doesn't require searching the history of the branch.
(If we later want to indicate more precisely what happened to this file
in the history of the branch, it's not as simple as saying "was deleted
on branch" because that doesn't necessarily mean "was present at branch
creation and was deleted exactly once in its history" - there are
several other variants possible.)
- Julian
Received on 2010-05-11 14:06:08 CEST