Re: [PATCH] please review: kicking off the tree-conflicts branch

Hi Erik,

Thanks for the quick feedback.

Quoting Erik Huelsmann <ehuels@gmail.com>:
> - The description of svn_wc__write_tree_conflict_descs() looks
> incomplete (see the line before the last)

Will fix the typo (an excess word) in the comment.

> - svn_wc__is_tree_conflict_victim() has an output argument
> (*tree_conflict_victim), but it's placed in the 'wrong' place: the
> convention in our code is to put pools last and output arguments first
> in the argument list.

Will fix the argument order.

> - Since svn_wc__write_tree_conflict_descs() adds information to the
> directory entry, how do you envision this to be idempotent? The
> comment seems to suggest the loggy system has to do with that, but the
> loggy system is there for atomicity (not idempotenticity).

In svn_wc__add_tree_conflict_data() we add info to an entry field by
reading the field, then rewriting the entire field in one loggy step.

In svn_wc__tree_conflict_resolved() we remove info from the entry field
in the same way.

svn_wc__write_tree_conflict_descs() doesn't alter the entry. It
converts the info into readable, localized sentences and writes them
to a temp file. Then svn_wc__loggy_move() replaces the user-visible
file with the temp file. FWIW, we don't use the deprecated function
svn_wc__loggy_append(), because it's not idempotent.

We think this design is idempotent (assuming that

> - The patch picks up a comment line in libsvn_wc/wc.h about there
> being no #define for DEPTH because it's only relevant to the THIS_DIR
> entry. From your description, I get the feeling the same applies to
> the TREE_CONFLICT_DATA and TREE_CONFLICT_DESCRIPTION items. Is that
> correct? And do you think it's like DEPTH enough to do without the
> defines?

I'm afraid I don't know enough about DEPTH. I simply took CONFLICT_WRK
and friends as my model in setting up the new entry fields. They are
written in the working copy but not transmitted to the repository, and
the tree conflict fields should have the same behavior. The #defines
for the new fields are needed in entries.c and log.c.

Thanks,
Steve

-- 
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org