Yoshiki Hayashi <yoshiki@xemacs.org> writes:
> Yes, I'm willing to take 2 and 4. They are almost there. :-)
> Seriously, addtional changes needed for my previous patch is
> that
> 1. call merge function inside svn_fs_commit_txn
> 2. change interface to hands back some data about conflict
> 3. new test case to be sure non overlapping commit works and
> overlapping commit fails
> 4. provide a way to set revision property within the same DB
> transaction
Wonderful. Would you be willing to finish and post the complete test
cases before starting on the library code itself? It's really
important to know we're starting with a thorough test on these. (And
yes, that's the order I'm working in too. :-) ).
You can #ifdef out the test case code until the functionality is
there, so the test passes. It's just important that we have the tests
in front of us from the start.
The interface changes sound good to me.
Best,
-K
> For 1, I'll just add a call to svn_fs__dag_merge which
> doesn't exist yet. svn_fs__dag_merge will take dag_node_t
> and trail as arguments and merge the differences
> recursively and do most of the work of svn_fs_merge.
>
> For 2, I'm not sure what is the best interface but perhaps
> just adding conflict_p like below is what I have in mind
> right now. (BTW, PROP is for change 4. Let's ignore it for
> now.)
>
> svn_error_t *svn_fs_commit_txn (const char **conflict_p,
> svn_revnum_t *new_rev,
> svn_fs_txn_t *txn,
> apr_hash_t *prop);
>
> I'm not too sure about what will be the best interface for
> underlying merge functions. For example, should it report
> only one conflict or all conflicts, as Jim pointed out. I
> think svn_fs_merge needs options to control finding every
> conflicts and put conflict points in apr_array_header_t.
> However, internal merge of svn_fs_commit_txn is for
> non-overlapping commit so it is sufficient to stop merging
> if it finds one conflict. If caller wants to find out all
> conflict points, it can call svn_fs_merge.
>
> This is off the point of svn_fs_commit_txn but I'd like to
> note the reason svn_fs_merge should have options to report
> all conflict. Suppose a user tries to commit many many
> files and a few of them, say README and the like, conflict.
> If the changes made to those files are not significant, a
> user can say forget the conflict and commit the rest. I'm
> not saying this should be implemented in the client
> subversion provides but it could be a useful feature.
>
> Back to the subject...
>
> For 3, I'll just steal what Karl will write to
> svn_fs_merge. :-)
> There will be a test case for revision properties, too.
>
> For 4, I propose adding apr_hash_t argument to
> svn_fs_commit_txn like above. I can't imagine any revision
> properties other than log message which needs to be added as
> soon as new revision come to existence. But this interface
> is general enough to handle it without interface change. If
> PROP is NULL, no properties are added. If it's not NULL,
> key and value of each entries represents property name and
> value respectively.
>
> This is where I am now. As soon as we settle on interface
> design, I'll start implementation.
>
> --
> Yoshiki Hayashi
Received on Sat Oct 21 14:36:25 2006