On Thu, 2009-09-24 at 13:13 +0100, Stefan Sperling wrote:
> On Wed, Sep 23, 2009 at 11:59:07PM +0100, Julian Foad wrote:
> > On Wed, 2009-09-23 at 20:25 +0100, Stefan Sperling wrote:
> > > On Wed, Sep 23, 2009 at 07:23:52PM +0100, Stefan Sperling wrote:
> > > > However, the local schedule *does* matter if the item is locally deleted.
> > > > After all, a user could want the merge to add another file on
> > > > top of a locally deleted file, causing it to be scheduled replace.
> >
> > That's true, but the code path we are looking at is inside "case
> > svn_node_file" in a switch(node kind on disk). Therefore we know that
> > the file is NOT properly locally deleted. In "case svn_node_file", if it
> > is schedule-delete then there is an obstruction on disk so the WC state
> > is inconsistent and the call to obstructed_or_missing() earlier in the
> > function should have caused it to be skipped.
> >
> > Therefore there is no need to look for schedule-delete in this case.
>
> Julian,
>
> thanks for helping me with this. After having gotten some sleep,
> the whole function fits into my brain again.
>
> I have adapted the code again with your feedback in mind.
>
> I also believe I found the cause of remaining "attempt to add tree
> conflict that already exists" problem reports while doing so.
> tree_conflict_on_add() has a bug which causes it to add conflicts
> on top of existing conflicts in some situations. See the diff below
> which I'm now running through make check.
>
> Any more clever insights you can provide are appreciated :)
>
> Stefan
>
> [[[
> * subversion/libsvn_client/merge.c
> (tree_conflict_on_add): Do not ever try to add new tree conflicts
> before removing existing ones. The old logic would try to do this
> in some cases since it did not consider the "existing_conflict != NULL"
> case in isolation.
> (merge_file_added): If there are no obstructions or missing nodes,
> but there is no node on disk, only add the new file if no tree
> conflict has been flagged at this path yet. Otherwise, branch out
> to tree_conflict_on_add().
> If there are no obstructions or missing nodes, and a file is already
> present on disk, flag a tree conflict as before, but do not consider
> the existing file's schedule as an indicator for the tree conflict
> reason. Rather, just flag the conflict with reason "added", based
> on the assumption that an add must have happened in the history of
> the branch.
> ]]]
>
> Index: subversion/libsvn_client/merge.c
[...]
> @@ -1589,6 +1598,7 @@ merge_file_added(svn_wc_adm_access_t *adm_access,
> const char *copyfrom_url = NULL;
> svn_revnum_t copyfrom_rev = SVN_INVALID_REVNUM;
> svn_stream_t *new_base_contents;
> + svn_wc_conflict_version_t *existing_conflict;
This needs to be svn_wc_conflict_description2_t. (You only test whether
it's null so the incorrect type doesn't prevent it from working.)
Apart from that, this new patch looks "spot on". Excellent.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2399324
Received on 2009-09-24 17:22:45 CEST