On Wed, Oct 27, 2010 at 4:06 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: hyrum_at_hyrumwright.org [mailto:hyrum_at_hyrumwright.org] On Behalf Of
>> Hyrum K. Wright
>> Sent: dinsdag 26 oktober 2010 23:49
>> To: Subversion Development
>> Subject: wc-ng: Storing conflict information on the victim: trials and
>> pitfalls
>>
>> This mail is part 2 in a series regarding the conflict information
>> storage (see [1] for part 1).
>>
>> The current Plan (as denoted in both wc-metadata.sql and the
>> conflict-storage notes) is to store conflict information in the ACTUAL
>> node. This works fine (and is currently being done on trunk), but
>> seems to break an assumed invariant that every node will also have an
>> entry in NODES. The docs in wc-metadata.sql state that "A NODES row
>> must exist if this node exists, but an ACTUAL_NODE row can exist on
>> its own if it is just recording info on a non-present node - a tree
>> conflict or a changelist, for example." So in theory we allow ACTAUL
>> nodes with conflict info without a NODE entry, but not in practice.
>>
>> One of the places this bites us is when retrieving children. Child
>> nodes which are victims of a conflict should be returned as part of
>> the child list APIs (which ultimately call
>> libsvn_wc/wc_db.c:gather_children()). Those APIs currently only look
>> in NODES to find children, without tacking on the conflict victims
>> embedded in ACTUAL.tree_conflict_data. In crafting a patch to do
>> this, I run into various places in wc_db where a NODE is assumed if an
>> ACTUAL node exists.
>>
>> For instance, I'd like to rewrite
>> libsvn_client/commit_util.c:bail_on_tree_conflicted_children() so that
>> it iterates over the children of the directory in question, rather
>> than use the (soon-to-disappear) "give me all the tree conflict
>> children on this node" API. However, if conflict victims aren't
>> returned as part of gather_children(), that API has no way of knowing
>> about those children who exist only as conflict victims. I believe
>> there are other reasons to include conflicted children in the child
>> lists as well.
>
> In the current code we have a wc_db api to retrieve 'all conflicted
> children' of a node, which can be used for this specific use case.
This API only exists because it is an artifact of the implementation.
And the implementation is only an artifact of limitations in wc-1.
Currently, all tree conflict information is stored on the parent,
since in some scenarios in wc-1, there would be no entries file in
which to store conflict information. This model goes against what I
feel is one of the great benefits of wc-ng, and is something I'm
working to change (as part of the updated conflict store).
All that being said, I don't think that the current "get all
conflicted children" API is a very good one to continue supporting,
since the same information could conceivably be achieved in other
ways. It's also worth asking "why do we need this API to begin with?"
Though I know the immediate answer, I wonder if there is a
higher-level issue which could be dealt with to remove this
dependency.
> It's also completely impossible to create a svn_wc_entry_t for a node in
> this state, so making it a 'proper node' will require some work in more than
> a few places.
Then just don't. For backward compat, pretend such a node doesn't
exist, and continue pretending the conflict information is stored on
the parent.
>> In the end, I'm not really sure what the best solution is, or how to
>> get there. I've thought about the possibility of having a NODE with
>> op_depth=-1 for nodes who only have an ACTUAL node, if only to satisfy
>> the "ACTUAL must have NODE" invariant. Any other comments or thought
>> are welcome.
>
> I would suggest dropping the 'ACTUAL must have NODE' invariant and keeping
> the rest of the wc_db api as close to the current state as possible.
>
> Making delete-delete tree conflicts proper nodes, will also make simple
> users of svn_wc__db_read_info(), as svn_wc_read_kind() return some node kind
> for this node. (How would we handle that?)
>
> I think svn_wc__db_read_info() should just return SVN_ERR_WC_PATH_NOT_FOUND
> on these nodes.
> (Note that we can fix the delete-delete tree conflicts caused by switching
> and updating, by just keeping a not-present NODE. But this doesn't work for
> delete-delete tree conflicts introduced by merging, as we can only introduce
> these not-present markers in BASE)
Dunno; need to think about these a bit more.
-Hyrum
Received on 2010-10-28 06:22:21 CEST