[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Problems with commit feature negotiation and write-through proxies

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Wed, 10 Jan 2018 18:54:40 +0300

Philip Martin <philip_at_codematters.co.uk> writes:

> We already have system for handling create-txn and create-txn-with-props
> so I was thinking of extending that code:
>
> Index: subversion/mod_dav_svn/version.c
> ===================================================================
> --- subversion/mod_dav_svn/version.c (revision 1820704)
> +++ subversion/mod_dav_svn/version.c (working copy)
> @@ -152,9 +152,6 @@ get_vsn_options(apr_pool_t *p, apr_text_header *ph
> apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_INHERITED_PROPS);
> apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_INLINE_PROPS);
> apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_REVERSE_FILE_REVS);
> - apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_SVNDIFF1);
> - apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_SVNDIFF2);
> - apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_PUT_RESULT_CHECKSUM);
> apr_text_append(p, phdr, SVN_DAV_NS_DAV_SVN_LIST);
> /* Mergeinfo is a special case: here we merely say that the server
> * knows how to handle mergeinfo -- whether the repository does too
> @@ -297,6 +294,19 @@ get_option(const dav_resource *resource,
> { "create-txn-with-props", { 1, 8, 0, "" } },
> };
>
> + /* These capabilities are used during commit and when acting as
> + a WebDAV slave (SVNMasterURI) their availablity depends on
> + the master version (SVNMasterVersion) rather than our own
> + (slave) version. */
> + struct capability_versions_t {
> + const char *capability_name;
> + svn_version_t min_version;
> + } capabilities[] = {
> + { SVN_DAV_NS_DAV_SVN_SVNDIFF1, { 1, 7, 0, ""} },
> + { SVN_DAV_NS_DAV_SVN_SVNDIFF2, { 1, 10, 0, ""} },
> + { SVN_DAV_NS_DAV_SVN_PUT_RESULT_CHECKSUM, { 1, 10, 0, ""} },
> + };

I would be fine with this approach as well, but perhaps with a few tweaks:

 - I think that it would be better to handle all four capabilities that
   relate to proxied writes in the same place:

     SVN_DAV_NS_DAV_SVN_EPHEMERAL_TXNPROPS
     SVN_DAV_NS_DAV_SVN_SVNDIFF1
     SVN_DAV_NS_DAV_SVN_SVNDIFF2
     SVN_DAV_NS_DAV_SVN_PUT_RESULT_CHECKSUM

   Otherwise, the handling becomes asymmetric, as the first of them is still
   checked using the separate dav_svn__check_ephemeral_txnprops_support()
   function, but the three new capabilities are checked using the new array-
   based approach.

 - The SVN_DAV_NS_DAV_SVN_SVNDIFF1 capability should probably be
   bound to version 1.10 instead of 1.7.

   The reason for that is that mod_dav_svn didn't properly advertise
   svndiff1 support with this specific header until 1.10, and I think
   that we should have the same behavior for the setups with a write-
   through proxy.

   (Even though mod_dav_svn has been able to parse it for a long time,
    currently ra_serf avoids sending svndiff1 to servers that don't
    advertise it with this specific header, as there might be third-party
    implementations of the Subversion's HTTP protocol that can only
    parse svndiff0).

Thanks,
Evgeny Kotkov
Received on 2018-01-10 16:55:09 CET

This is an archived mail posted to the Subversion Dev mailing list.