You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2013/04/10 22:28:42 UTC
svn commit: r1466659 - /subversion/trunk/subversion/svnserve/serve.c
Author: julianfoad
Date: Wed Apr 10 20:28:42 2013
New Revision: 1466659
URL: http://svn.apache.org/r1466659
Log:
Fix issue #4348, "missing directory replace on update 1.8 svnserve 1.7 client".
* subversion/svnserve/serve.c
(update): Correct the backward-compatibility code for the
'ignore_ancestry' parameter introduced in r1465292.
Found by: philip
Modified:
subversion/trunk/subversion/svnserve/serve.c
Modified: subversion/trunk/subversion/svnserve/serve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1466659&r1=1466658&r2=1466659&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/serve.c (original)
+++ subversion/trunk/subversion/svnserve/serve.c Wed Apr 10 20:28:42 2013
@@ -1814,7 +1814,7 @@ static svn_error_t *update(svn_ra_svn_co
const char *target, *full_path, *depth_word;
svn_boolean_t recurse;
apr_uint64_t send_copyfrom_args; /* Optional; default FALSE */
- apr_uint64_t ignore_ancestry; /* Optional; default TRUE */
+ apr_uint64_t ignore_ancestry; /* Optional; default FALSE */
/* Default to unknown. Old clients won't send depth, but we'll
handle that by converting recurse if necessary. */
svn_depth_t depth = svn_depth_unknown;
@@ -1842,7 +1842,7 @@ static svn_error_t *update(svn_ra_svn_co
conn, pool, b, rev, target, NULL, TRUE,
depth,
(send_copyfrom_args == TRUE) /* send_copyfrom_args */,
- (ignore_ancestry != FALSE) /* ignore_ancestry */));
+ (ignore_ancestry == TRUE) /* ignore_ancestry */));
if (is_checkout)
{
SVN_ERR(log_command(b, conn, pool, "%s",
Re: svn commit: r1466659 - /subversion/trunk/subversion/svnserve/serve.c
Posted by Julian Foad <ju...@btopenworld.com>.
Bert Huijben wrote:
> This value is actually tri-state to allow handling not provided,
> so the == check is correct. (Otherwise it would have been a
> svn_boolean_t, not a apr_int_64_t and a ‘b’ in the pattern
> instead of a ‘B’)
>
> The behaviour change in Julian's original patch for the copy args
> looks good to. (Probably an old bug)
My patch didn't change the behaviour of the 'send_copyfrom_args' parameter. It just simplified the implementation and made it consistent with your implementation of the same functionality in the 'switch' function. I guess that wasn't 100% clear from the log message.
Quoting from my original patch <http://svn.apache.org/viewvc?view=revision&revision=r1466570>:
[[[
* subversion/svnserve/serve.c
(update): Parse and use the 'ignore_ancestry' parameter; default to false
if not present. Simplify the 'send_copyfrom_args' handling to match how
it is done in 'switch'.
--- subversion/trunk/subversion/svnserve/serve.c
+++ subversion/trunk/subversion/svnserve/serve.c
@@ -1813,16 +1813,17 @@
svn_revnum_t rev;
const char *target, *full_path, *depth_word;
svn_boolean_t recurse;
- svn_boolean_t send_copyfrom_args;
- apr_uint64_t send_copyfrom_param;
+ apr_uint64_t send_copyfrom_args; /* Optional; default FALSE */
+ apr_uint64_t ignore_ancestry; /* Optional; default TRUE */
[...]
- SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cb?wB", &rev, &target,
- &recurse, &depth_word, &send_copyfrom_param));
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cb?wB?B", &rev, &target,
+ &recurse, &depth_word,
+ &send_copyfrom_args, &ignore_ancestry));
[...]
- send_copyfrom_args = (send_copyfrom_param == SVN_RA_SVN_UNSPECIFIED_NUMBER) ?
- FALSE : (svn_boolean_t) send_copyfrom_param;
-
[...] SVN_ERR(accept_report(&is_checkout, NULL, conn, pool, b, rev, target, NULL, TRUE,
- depth, send_copyfrom_args, FALSE));
+ depth,
+ (send_copyfrom_args == TRUE) /* send_copyfrom_args */,
+ (ignore_ancestry != FALSE) /* ignore_ancestry */));
]]]
- Julian
Re: svn commit: r1466659 - /subversion/trunk/subversion/svnserve/serve.c
Posted by Bert Huijben <be...@qqmail.nl>.
This value is actually tri-state to allow handling not provided, so the == check is correct. (Otherwise it would have been a svn_boolean_t, not a apr_int_64_t and a ‘b’ in the pattern instead of a ‘B’)
The behaviour change in Julian's original patch for the copy args looks good to. (Probably an old bug)
Bert
Sent from Windows Mail
From: Ivan Zhakov
Sent: Wednesday, April 10, 2013 10:31 PM
To: julianfoad@apache.org
Cc: dev@subversion.apache.org
On Thu, Apr 11, 2013 at 12:28 AM, <ju...@apache.org> wrote:
> Author: julianfoad
> Date: Wed Apr 10 20:28:42 2013
> New Revision: 1466659
>
> URL: http://svn.apache.org/r1466659
> Log:
> Fix issue #4348, "missing directory replace on update 1.8 svnserve 1.7 client".
>
> * subversion/svnserve/serve.c
> (update): Correct the backward-compatibility code for the
> 'ignore_ancestry' parameter introduced in r1465292.
>
> Found by: philip
>
> Modified:
> subversion/trunk/subversion/svnserve/serve.c
>
> Modified: subversion/trunk/subversion/svnserve/serve.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1466659&r1=1466658&r2=1466659&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnserve/serve.c (original)
> +++ subversion/trunk/subversion/svnserve/serve.c Wed Apr 10 20:28:42 2013
> @@ -1814,7 +1814,7 @@ static svn_error_t *update(svn_ra_svn_co
> const char *target, *full_path, *depth_word;
> svn_boolean_t recurse;
> apr_uint64_t send_copyfrom_args; /* Optional; default FALSE */
> - apr_uint64_t ignore_ancestry; /* Optional; default TRUE */
> + apr_uint64_t ignore_ancestry; /* Optional; default FALSE */
> /* Default to unknown. Old clients won't send depth, but we'll
> handle that by converting recurse if necessary. */
> svn_depth_t depth = svn_depth_unknown;
> @@ -1842,7 +1842,7 @@ static svn_error_t *update(svn_ra_svn_co
> conn, pool, b, rev, target, NULL, TRUE,
> depth,
> (send_copyfrom_args == TRUE) /* send_copyfrom_args */,
> - (ignore_ancestry != FALSE) /* ignore_ancestry */));
> + (ignore_ancestry == TRUE) /* ignore_ancestry */));
As far I remember any non-zero value considered as boolean 'true'. So
correct expression should be just 'ignore_ancestry'.
--
Ivan Zhakov
Re: svn commit: r1466659 - /subversion/trunk/subversion/svnserve/serve.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, Apr 11, 2013 at 12:28 AM, <ju...@apache.org> wrote:
> Author: julianfoad
> Date: Wed Apr 10 20:28:42 2013
> New Revision: 1466659
>
> URL: http://svn.apache.org/r1466659
> Log:
> Fix issue #4348, "missing directory replace on update 1.8 svnserve 1.7 client".
>
> * subversion/svnserve/serve.c
> (update): Correct the backward-compatibility code for the
> 'ignore_ancestry' parameter introduced in r1465292.
>
> Found by: philip
>
> Modified:
> subversion/trunk/subversion/svnserve/serve.c
>
> Modified: subversion/trunk/subversion/svnserve/serve.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1466659&r1=1466658&r2=1466659&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnserve/serve.c (original)
> +++ subversion/trunk/subversion/svnserve/serve.c Wed Apr 10 20:28:42 2013
> @@ -1814,7 +1814,7 @@ static svn_error_t *update(svn_ra_svn_co
> const char *target, *full_path, *depth_word;
> svn_boolean_t recurse;
> apr_uint64_t send_copyfrom_args; /* Optional; default FALSE */
> - apr_uint64_t ignore_ancestry; /* Optional; default TRUE */
> + apr_uint64_t ignore_ancestry; /* Optional; default FALSE */
> /* Default to unknown. Old clients won't send depth, but we'll
> handle that by converting recurse if necessary. */
> svn_depth_t depth = svn_depth_unknown;
> @@ -1842,7 +1842,7 @@ static svn_error_t *update(svn_ra_svn_co
> conn, pool, b, rev, target, NULL, TRUE,
> depth,
> (send_copyfrom_args == TRUE) /* send_copyfrom_args */,
> - (ignore_ancestry != FALSE) /* ignore_ancestry */));
> + (ignore_ancestry == TRUE) /* ignore_ancestry */));
As far I remember any non-zero value considered as boolean 'true'. So
correct expression should be just 'ignore_ancestry'.
--
Ivan Zhakov