> URL: http://svn.apache.org/r1446169
> Log:
> Following up on r1446118, make the knowledge whether reversed obtaining of
> file revisions is available visible by adding an ra capability.
>
> This to allow a future blame improvement to decide whether it can use an
> improved algorithm instead of the current one without waiting for an error
> on old servers.
Nice.
A few questions.
When you first added this "reverse order" functionality in <http://svn.apache.org/r1445973>, you documented for svn_ra_get_file_revs2() and svn_repos_get_file_revs2():
+ * On subversion 1.8 and newer servers this function has been enabled
+ * to support reversion of the revision range for @a include_merged_revision
+ * @c FALSE reporting by switching @a end with @a start.
Is that still the case or do those functions now support a backwards range with merged revisions included?
Reference the capability string in those doc strings.
There's still a big comment on the definition (rather than declaration) of svn_repos_get_file_revs2() which says reverse ranges are not supported; that should go away or be updated.
For testing, in r1445973:
* subversion/tests/libsvn_repos/repos-test.c
(test_get_file_revs): Extend test to also walk the revisions backwards.
Would it be true to say that the results of a reverse range should always be the reversal of the results from the equivalent forward range? If so, that would be a useful test. The current test doesn't appear to verify that the results are in any particular order.
More below...
> * subversion/include/svn_dav.h
> (SVN_DAV_NS_DAV_SVN_ATOMIC_REVPROPS): Move new in 1.7 define below one that
> exists in 1.6.
> (SVN_DAV_NS_DAV_SVN_INHERITED_PROPS): Mark new in 1.8.
> (SVN_DAV_NS_DAV_SVN_EPHEMERAL_TXNPROPS): Mark new in 1.8.
> (SVN_DAV_NS_DAV_SVN_INLINE_PROPS): Mark new in 1.8.
> (SVN_DAV_NS_DAV_SVN_GET_FILE_REVS_REVERSE): New macro.
>
> * subversion/include/svn_ra.h
> (SVN_RA_CAPABILITY_GET_FILE_REVS_REVERSE): New macro.
>
> * subversion/include/svn_ra_svn.h
> (SVN_RA_SVN_CAP_GET_FILE_REVS_REVERSE): New macro.
>
> * subversion/libsvn_ra_local/ra_plugin.c
> (svn_ra_local__has_capability): Declare support for reversed file
> revisions *and* ephemeral txnprops.
That's an unrelated change, fixing ra-local to advertise ephemeral txnprops. That's worth drawing attention to, as it indicates we aren't testing that feature. (It would have been better as a separate commit.)
> * subversion/libsvn_ra_serf/options.c
> (capabilities_headers_iterator_callback): Forward
> SVN_RA_SVN_CAP_GET_FILE_REVS_REVERSE support.
>
> * subversion/libsvn_ra_svn/client.c
> (ra_svn_has_capability): Make function table driven. Add
> SVN_RA_SVN_CAP_GET_FILE_REVS_REVERSE support.
>
> * subversion/mod_dav_svn/version.c
> (get_vsn_options): Write SVN_DAV_NS_DAV_SVN_GET_FILE_REVS_REVERSE.
>
> * subversion/svnserve/serve.c
> (serve): Send SVN_RA_SVN_CAP_GET_FILE_REVS_REVERSE support.
> +/**
> + * The capability of a server to walk revisions backwards in
> + * svn_ra_get_file_revs2
Doxygen will make this a hyperlink if you add '()' after the function name.
> + *
> + * @since New in 1.8.
> + */
> +#define SVN_RA_CAPABILITY_GET_FILE_REVS_REVERSE "get-file-revs-reversed"
> +/** Presence of this in a DAV header in an OPTIONS response indicates
> + * that the transmitter (in this case, the server) knows how to handle
> + * a reversed fetch of file versions.
> + * @since New in 1.8. */
> +#define SVN_DAV_NS_DAV_SVN_GET_FILE_REVS_REVERSE\
> + SVN_DAV_PROP_NS_DAV "svn/get-file-revs-rvrs"
> +/* maps to SVN_RA_CAPABILITY_GET_FILE_REVS_REVERSE */
> +#define SVN_RA_SVN_CAP_GET_FILE_REVS_REVERSE "file-revs-reverse"
Would it be useful to refer to the relevant DAV/svnserve API functions from these two doc strings?
In the doc string of svn_ra_get_file_revs2(), detail what the presence or absence of this capability really means to a caller. Something like:
[[[
With a forward range, svn_ra_get_file_revs2() calculates the list
of revisions before starting to send them to the caller.
When a backward range is requested, with the 'reverse' capability
(#SVN_RA_CAPABILITY_GET_FILE_REVS_REVERSE) svn_ra_get_file_revs2()
returns the revisions streamily in backward order, but a request
for merged revisions throws an error; without the capability, it
either throws an error or returns an incomplete result.
]]]
- Julian
Received on 2013-02-14 16:15:44 CET