fs status -- one big thing left

[Karl Fogel <kf...@galois.collab.net>]

It's basically down to svn_fs_merge and svn_fs_commit (whee-hoo!).
This opens a possibility for a little quick parallelization.  We need
to have these four things, and they can be developed simultaneously:

   1. a test case for svn_fs_merge
   2. a test case for svn_fs_commit_txn
   3. svn_fs_merge
   4. svn_fs_commit_txn  (depends on svn_fs_merge)

(I've listed them in that order deliberately -- writing the test case
first has become religion around here, and it's really helpful.)

Anyway, I'm going to start on 1 right now, followed by 3.  Yoshiki,
would you like to take 2 and then 4?  I'll race you. :-)

If other people are interested as well, we can break it down even
more, just say the word.

-Karl

Re: fs status -- one big thing left

[Karl Fogel <kf...@galois.collab.net>]

Yoshiki Hayashi <yo...@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

Re: fs status -- one big thing left

[Yoshiki Hayashi <yo...@xemacs.org>]

Karl Fogel <kf...@galois.ch.collab.net> writes:

> It's basically down to svn_fs_merge and svn_fs_commit (whee-hoo!).
> This opens a possibility for a little quick parallelization.  We need
> to have these four things, and they can be developed simultaneously:
> 
>    1. a test case for svn_fs_merge
>    2. a test case for svn_fs_commit_txn
>    3. svn_fs_merge
>    4. svn_fs_commit_txn  (depends on svn_fs_merge)
> 
> (I've listed them in that order deliberately -- writing the test case
> first has become religion around here, and it's really helpful.)
> 
> Anyway, I'm going to start on 1 right now, followed by 3.  Yoshiki,
> would you like to take 2 and then 4?  I'll race you. :-)

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

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