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

Re: [PATCH] Fix a bug with property validation logic during 'svnadmin load'

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: Thu, 26 Jan 2012 08:14:06 -0500

On 01/25/2012 09:51 AM, vijay wrote:
> Index: subversion/include/svn_repos.h
> ===================================================================
> --- subversion/include/svn_repos.h (revision 1235706)
> +++ subversion/include/svn_repos.h (working copy)
> @@ -2649,7 +2649,7 @@
> /**
> * Similar to svn_repos_load_fs3(), but with @a feedback_stream in
> * place of the #svn_repos_notify_func_t and baton and with
> - * @a validate_props always FALSE.
> + * @a validate_props always TRUE.
> *
> * @since New in 1.2.
> * @deprecated Provided for backward compatibility with the 1.6 API.
> @@ -2885,7 +2885,7 @@
> /**
> * Similar to svn_repos_get_fs_build_parser3(), but with @a outstream
> * in place if a #svn_repos_notify_func_t and baton and with
> - * @a validate_props always FALSE.
> + * @a validate_props always TRUE.

Vijay, I think you have misunderstood what these docstrings are claiming.

They are not saying, "When Subversion calls the FS-loading API, it tends to
pass FALSE for validate_props by default."

They are describing the quite literal implementation of the older function
as it related to the newer one. They are saying, "Calling
svn_repos_load_fs2() is just like calling svn_repos_load_fs3() with all the
same parameters, except the new parameter validate_props is always FALSE.

Your docstring change above is incorrect, because the implementations of the
older svn_repos_load_fs2() and svn_repos_get_fs_build_parser2() functions
has not changed. Furthermore, those old imlementations are not allowed to
change, because that would change the behaviors of those functions, breaking
our compatibility promise.

> Index: subversion/libsvn_repos/load-fs-vtable.c
> ===================================================================
> --- subversion/libsvn_repos/load-fs-vtable.c (revision 1235706)
> +++ subversion/libsvn_repos/load-fs-vtable.c (working copy)
> @@ -160,12 +160,12 @@
> apr_pool_t *pool)
> {
> if (validate_props)
> - return svn_fs_change_rev_prop2(svn_repos_fs(repos), revision, name,
> - NULL, value, pool);
> - else
> return svn_repos_fs_change_rev_prop4(repos, revision, NULL, name,
> NULL, value, FALSE, FALSE,
> NULL, NULL, pool);
> + else
> + return svn_fs_change_rev_prop2(svn_repos_fs(repos), revision, name,
> + NULL, value, pool);
> }
>
> /* Change property NAME to VALUE for PATH in TXN_ROOT. If

This part of your patch looks correct, though.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Received on 2012-01-26 14:14:42 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.