On 8/9/06, David Glasser <glasser@mit.edu> wrote:
> So is the former API clean enough to be worth implementing, or is the
> latter not too invasive, or is there some other option that I'm
> completely missing?
I decided to give this a shot. Attached is a patch which begins to
implement it; the big obvious lack right now is that it only works
with ra_local and ra_svnserve. This is by far the biggest subversion
patch I've worked on, so I figure it's best to stick it out for review
as soon as possible.
Notes:
* I recognize that the thread about this has not really settled on a
choice of API, and some folks support adding this capability
to the svn_delta_editor_t type. Arguably neither way of doing it is
really clean: either you pollute the delta description with extra
properties, or you allow one (and only one) RA API to be used in the
middle of the editor stream. I currently prefer the latter, since
the other way would require upgrading about 50 APIs that use
svn_delta_editor_t, and this one just requires one new API and one
API documentation change (relaxing a "don't do this" constraint).
(Well, and indirectly two other API bumps.)
However, I seriously do not have much ego invested in this
particular way of doing things, and if committers want to say
"thanks for the work, but we don't like the API", then, well, that's
how it is. It's been pretty educational anyway.
* Do I need to put anything in libsvn_ra/wrapper_template.h? That is,
do I need to support the use of a brand new RA API in the old
ra_plugin conventions?
* Similarly, do I have to support the use of a brand new svnserve command
under non-pipelined editing?
* With ra_svn, the only way that you can find out that the remote
repository doesn't
support set-rev-prop is by trying it and getting an error back. As
far as I can tell this
is how the locking commands work too. Presumably users of this API who want
to be able to work against older servers will need to explicitly
trap the "command
not recognized" error.
* An earlier version of this patch didn't create
svn_repos_get_commit_editor5 and instead had each of its relevant
invokers manually create and deal with aborting the txn. This was a
huge pain, since I basically ended up needing to copy-and-paste code from
libsvn_repos to create the txns and then create wrapper editors
to deal with abortion; this effort was duplicated in both RA implementations.
I think the svn_repos_get_commit_editor5 API is saner than trying to simulate
it without it existing.
It does create a slight behavior change, though it's still
consistent with the API's documentation: the managed txn is now made
in svn_repos_get_commit_editorN itself instead of waiting until
open_root is called. Is this a problem?
* I changed svnsync to use the new API, mostly for testing
purposes. The version in this patch does *not* fall back to
non-atomic revprop copying if the server doesn't support it, since
for debugging purposes it's helpful to know that the server isn't
supporting it yet. I will certainly add that to a final patch.
* No support in any of the WebDAV-related modules. I am not
particularly familiar with any of them; I'd be happy if somebody who
was wanted to write their implementations, but also could certainly
dive in myself.
* Tested this using the ra_local C tests and svnsync. Currently passes
"make check".
[[[
New svn_ra_set_revprop_for_commit API, which allows atomically setting
revprops at initial commit time over the RA layer.
* subversion/include/svn_ra.h
(svn_ra_set_revprop_for_commit): New RA API.
(svn_ra_get_commit_editor2): Document that it's OK to call the
new RA API during an editor session.
* subversion/libsvn_ra/ra_loader.c
(svn_ra_set_revprop_for_commit): Implement vtable dispatch for new
RA API.
* subversion/libsvn_ra/ra_load.h
(svn_ra__vtable_t): Add slot for new RA API.
* subversion/include/svn_error_codes.h
(SVN_ERR_RA_SVN_INVALID_SET_REVPROP): New error for use of
set-rev-prop command in a non-committing editor session.
* subversion/include/svn_repos.h
(svn_repos_get_commit_editor5): API upgrade to allow the caller
to request access to a repos-managed txn.
* subversion/include/svn_ra_svn.h
(svn_ra_svn_drive_editor2): API upgrade to give driver access to
txn.
* subversion/libsvn_ra_local/ra_plugin.c
(svn_ra_local__get_commit_editor): Get txn from repos commit editor.
(svn_ra_local__set_revprop_for_commit): Implement new RA API.
(ra_local_vtable): Add implementation of new RA API to vtable.
* subversion/libsvn_ra_local/ra_local.h
(svn_ra_local__session_baton_t): Add slot for txn.
* subversion/tests/libsvn_ra_local/ra-local-test.c
(set_revprops_atomically, commit_callback): Add new test
which tests new RA API on ra_local (and for that matter
tests commits).
(test_funcs): Call new test.
* subversion/svnsync/main.c
(copy_revprops_atomically): Like copy_revprops, but uses new RA API;
used when copying a revision for the first time.
(do_synchronize): Use copy_revprops_atomically instead of copy_revprops
when you can.
* subversion/libsvn_repos/commit.c
(open_root): Move managed txn creation from here to ...
(svn_repos_get_commit_editor5): ... here, which is an upgraded
API which can return the managed txn.
* subversion/libsvn_ra_svn/client.c
(ra_svn_set_revprop_for_commit): Implement new RA API.
(ra_svn_vtable): Add implementation of new RA API to vtable.
* subversion/libsvn_ra_svn/protocol:
Document new set-rev-prop editor command.
* subversion/libsvn_ra_svn/editor.c
(svn_ra_svn_drive_editor2): Upgrade API to accept txn.
* subversion/libsvn_ra_svn/editorp.c
(ra_svn_driver_state_t): Add slot for txn.
(ra_svn_handle_set_revprop_for_commit): Handle new RA API.
(ra_svn_edit_cmds): Add handler for new RA API.
(svn_ra_svn__drive_editorp): Store the given txn in the
driver state.
* subversion/libsvn_ra_svn/ra_svn.h
(svn_ra_svn__drive_editorp): Take an extra txn argument.
* subversion/svnserve/serve.c
(commit): Get txn from repos_get_commit_editor5 and pass it
through to svn_ra_svn_drive_editor2.
]]]
--
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Aug 11 20:43:48 2006