Re: Tree-conflicts: HEADS UP: update failure with tc-siblings :0

Neels J. Hofmeyr wrote:
> Stephen Butler wrote:
>>> CMD: svn status -v -u -q ...
>>> D 2 1 jrandom /A/B/E/beta
>>> D C * 1 1 jrandom /A/B/E/alpha
>>> D C 2 2 jrandom /A/B/E
>>> D * 1 1 jrandom /A/B/lambda
>>> 2 1 jrandom /A/B/F
>> The missing ' C' for lambda is still missing on trunk. It appears
>> that two log files are written in /A/B/.svn, for lambda and for E, and
>> svn_wc__run_log() reads both of them, but the content of the first log
>> file isn't written to the entry.
>>
>> Strange. I'll keep digging.

I've also done some digging now and am busy fixing it. (Steve, I know you
are or have been on this. Shout if I'm duplicating your work or something.)
Let's explain the problem:

run_log() reads two modify-entry commands from a log file:

<modify-entry
   name=""
   tree-conflicts="B:dir:update:edited:deleted"/>
<modify-entry
   name=""
   tree-conflicts="mu:file:update:edited:deleted"/>

It calls log_do_modify_entry() on both of them.

Common sense says that this tree-conflicts data should be concatenated.
But log_do_modify_entry() *replaces* the information that is already there.
So the first new conflict is overwritten by the second.

Ok, let's step one layer out of this onion. There is the function
svn_wc__loggy_add_tree_conflict() which writes these tree-conflicts to the
log. This function first checks what other tree-conflicts there are in the
entry, adds the new tree-conflicts data and replaces the whole
tree-conflicts data in the entry with the new list.

The problem is that if in one directory, two tree-conflicts appear in the
same run and are recorded using the same log, the second tree-conflict
addition again reads the *old* list of conflicts from the entry, appends the
second entry and writes the whole thing out. So, any earlier tree-conflicts
that were queued in the log file and aren't in the official entry yet get
overwritten with the latest one.

I am busy on a patch that first collects all new tree-conflicts upon update
in the parent dir baton as svn_wc_conflict_description_t* in an apr_array
and logs them all at once on close_directory() (in update_editor.c).

The patch won't make it today, but there are questions I'd like to send out
already:

1)
In libsvn_wc/update_editor.c, around line 1632 in do_entry_deletion(), it says:

[[[
  if (tree_conflict != NULL)
    {
      /* Run the log immediately, so that the tree conflict is recorded. */
      SVN_ERR(svn_wc__write_log(adm_access, *log_number, log_item, pool));
      SVN_ERR(svn_wc__run_log(adm_access, NULL, pool));
      *log_number = 0;
    }
]]]

Can anyone remember why that would be necessary?

2)
We use svn_wc_conflict_description_create_tree() to make a new conflict
description struct. The fact that this function also *allocates* the memory
makes it a little harder to push a new description onto an
apr_array_header_t, since apr_array_push() is also allocating memory and
returning it to be written in.

I'm currently solving this by not storing conflict description structs, but
only pointers to them in the apr_array. I thus avoid changing the signature
of svn_wc_conflict_description_create_tree(), which would otherwise break
with the conventions of svn_wc_conflict_description_create_text() and
svn_wc_conflict_description_create_prop(). Do you guys agree with that, or
would you rather change the function signature or even signatureS?

Thanks
~Neels