I'll port the mergeinfo-capability branch changes to trunk soon, but
I'd appreciate some review, both on the general approach and on a few
specific questions. (See issue #3089 for the problem.)
The relevant revisions are: r29259, r29263, r29264, r29265, r29266,
and you can get the full branch diff like this:
$ svn diff -r29257:HEAD \
https://svn.collab.net/repos/svn/branches/mergeinfo-capability
Specific questions:
* Does svn_ra_get_mergeinfo() return SVN_RA_UNSUPPORTED_FEATURE
reliably when the repository in question does not support
mergeinfo? (I can test this myself, just sending up a flare that
the question is out there, because cmpilato mentioned something
about how there may be a problem with FSFS in this regard.)
* I hate, hate, hate adding an extra turnaround like this, but I
don't see any way to avoid it. The specific repository is sadly
unavailable to the initial capabilities-exchange code on the
server side (in both ra_dav and ra_svn), so asking whether
mergeinfo is supported requires a separate trip. If anyone sees
a way out of this, let me know.
* We already had a lot of duplication in the capabilities code in
general, and it got worse with these commits. I started the
'capabilities-abstraction' branch to explore a way to reduce the
duplication. However, I think it's not worth solving this for
1.5 -- *as long as* none of the current code duplication pokes
through to API-visibility. I believe none does, and that
therefore we can deal with it after 1.5, but would appreciate
people checking that claim carefully
Because I love you [___this___] much, here are all the log messages
for the 'mergeinfo-capability' branch, bound together in a fine gold
and leather Folio Classics edition that's sure to go up in value:
------------------------------------------------------------------------
r29266 | kfogel | 2008-02-10 23:27:48 -0500 (Sun, 10 Feb 2008) | 9 lines
On the mergeinfo-capability branch:
* subversion/libsvn_ra_serf/serf.c
(svn_ra_serf__has_capability): Include string constant by reference,
instead of writing it out again.
* subversion/libsvn_ra_neon/session.c
(svn_ra_neon__has_capability): Same.
------------------------------------------------------------------------
r29265 | kfogel | 2008-02-10 23:25:24 -0500 (Sun, 10 Feb 2008) | 5 lines
On the mergeinfo-capability branch:
* subversion/libsvn_client/update.c
(svn_client__update_internal): Remove debugging hack.
------------------------------------------------------------------------
r29264 | kfogel | 2008-02-10 23:24:57 -0500 (Sun, 10 Feb 2008) | 20 lines
On the mergeinfo-capability branch:
Certain errors actually indicate that the repository supports mergeinfo;
handle them accordingly.
* subversion/include/svn_ra.h
(svn_ra_get_mergeinfo): Document error behaviors that we depend on.
* subversion/libsvn_ra_svn/client.c
(ra_svn_has_capability): Treat path-not-found as success.
* subversion/libsvn_ra_local/ra_plugin.c
(svn_ra_local__has_capability): Same.
* subversion/libsvn_ra_serf/serf.c
(svn_ra_serf__has_capability): Same.
* subversion/libsvn_ra_neon/session.c
(svn_ra_neon__has_capability): Same.
------------------------------------------------------------------------
r29263 | kfogel | 2008-02-10 20:37:52 -0500 (Sun, 10 Feb 2008) | 22 lines
On the mergeinfo-capability branch:
Take care of ra_svn:
* subversion/libsvn_ra_svn/ra_svn.h
(svn_ra_svn__session_baton_t): New field repository_supports_mergeinfo.
* subversion/libsvn_ra_svn/client.c
(open_session): Initialize above new field.
(ra_svn_has_capability): If necessary, ask the repository whether it
supports mergeinfo, and store the answer in the new field.
And do ra_local the same way:
* subversion/libsvn_ra_local/ra_local.h
(svn_ra_local__session_baton_t): New field repository_supports_mergeinfo.
* subversion/libsvn_ra_local/ra_plugin.c
(svn_ra_local__open): Initialize above new field.
(svn_ra_local__has_capability): If necessary, ask the repository
whether it supports mergeinfo, and store the answer in the new field.
------------------------------------------------------------------------
r29259 | kfogel | 2008-02-09 23:50:26 -0500 (Sat, 09 Feb 2008) | 35 lines
On the mergeinfo-capability branch:
Start on issue #3089: only report mergeinfo capability if repository
supports it.
Add correct mergeinfo probing to the client side for DAV RA layers.
Still to come: ra_svn, ra_local, and ensuring that FSFS backend
actually returns unsupported-feature svn_fs_get_mergeinfo is called on
an un-upgraded repository.
[Note: the code duplication here, plus that already present in the RA
capabilities code, led me to start the capabilities-abstraction branch.
However, I'm not sure yet that the benefits of de-duplication will
outweigh the costs -- the path below may well be the one we stay on.]
* subversion/libsvn_ra_serf/serf.c
(capability_server_yes): New capability state.
(capabilities_headers_iterator_callback): Set above state for mergeinfo.
(svn_ra_serf__has_capability): Only claim mergeinfo capability if
repository supports it as well as server.
* subversion/libsvn_ra_neon/session.c
(capability_server_yes): New capability state.
(parse_capabilities): Set above state for mergeinfo.
(svn_ra_neon__has_capability): Only't claim mergeinfo capability if
repository supports it as well as server.
* subversion/include/svn_ra.h
(svn_ra_get_mergeinfo): Correct a comment about mergeinfo support.
* subversion/libsvn_client/update.c
(svn_client__update_internal): Insert temporary debugging call to
probe mergeinfo capability during updates.
------------------------------------------------------------------------
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-11 05:50:00 CET