You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2017/09/05 13:45:37 UTC
Re: svn commit: r1807319 - in
/subversion/branches/better-pristines/subversion: include/
include/private/ libsvn_client/ libsvn_wc/ svn/
brane@apache.org wrote on Tue, 05 Sep 2017 07:45 +0000:
> Author: brane
> Date: Tue Sep 5 07:45:49 2017
> New Revision: 1807319
>
> URL: http://svn.apache.org/viewvc?rev=1807319&view=rev
> Log:
> Introduce the concept of a target format for working copy upgrades
> into the client library.
> +++ subversion/branches/better-pristines/subversion/include/svn_client.h Tue Sep 5 07:45:49 2017
> @@ -4201,13 +4201,34 @@ svn_client_cleanup(const char *dir,
> * @{
> */
>
> -/** Recursively upgrade a working copy from any older format to the current
> - * WC metadata storage format. @a wcroot_dir is the path to the WC root.
> +/**
> + * Recursively upgrade a working copy from any older format to the
> + * given WC metadata storage format. @a wcroot_dir is the path to the
> + * WC root.
> + *
Suggest to clarify that "Recursively" means into nested working copies
(externals), not into subdirs.
> + * @a wc_format_version is version number of the Subversion client
> + * that supports a given WC metadata format; @c NULL means the newest
> + * supported format. Any other value must be a string representing a
> + * version number, e.g., "1.8" or "1.9.3". The earliest supported
> + * version is defined by #SVN_VERSION_SUPPORTED_WC_FORMAT.
> *
When might 1.A.3 and 1.A.4 have different formats?
> * Use @a scratch_pool for any temporary allocations.
> *
> + * @since New in 1.10.
> + */
> +svn_error_t *
> +svn_client_upgrade2(const char *wcroot_dir,
> + const char* wc_format_version,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *scratch_pool);
> +
> +/**
> + * Like svn_client_upgrade2(), but always upgrades to the newest
> + * supported format.
> + *
> - * @since New in 1.7.
> + * @deprecated Provided for backward compatibility with the 1.7 API.
Why drop @since? We don't usually remove @since tags when deprecating.
(Both here and in svn_wc_upgrade())
> */
> +SVN_DEPRECATED
> svn_error_t *
> svn_client_upgrade(const char *wcroot_dir,
> svn_client_ctx_t *ctx,
> +++ subversion/branches/better-pristines/subversion/include/svn_version.h Tue Sep 5 07:45:49 2017
> @@ -134,6 +134,12 @@ extern "C" {
> #define SVN_VERSION SVN_VER_NUMBER SVN_VER_TAG
>
>
> +/**
> + * Earliest supported working copy version.
> + * @since New in 1.10.
> + */
> +#define SVN_VERSION_SUPPORTED_WC_FORMAT "1.8.0"
Shouldn't this be a function? It depends not on the compile-time
library version but on the run-time library version.
> +++ subversion/branches/better-pristines/subversion/include/svn_wc.h Tue Sep 5 07:45:49 2017
> @@ -7447,8 +7447,9 @@ typedef svn_error_t * (*svn_wc_upgrade_g
> * repository uuid, @a repos_info_func (if non-NULL) will be called
> * with @a repos_info_baton to provide the missing information.
> *
> - * @since New in 1.7.
> + * @deprecated Provided for backward compatibility with the 1.7 API.
(This is the @since mentioned above.)
> */
> +SVN_DEPRECATED
> svn_error_t *
> svn_wc_upgrade(svn_wc_context_t *wc_ctx,
> const char *local_abspath,
> +++ subversion/branches/better-pristines/subversion/libsvn_client/upgrade.c Tue Sep 5 07:45:49 2017
> @@ -173,16 +176,30 @@ svn_client_upgrade(const char *path,
> /* Upgrading from <= 1.6, or no svn:properties defined.
> (There is no way to detect the difference from libsvn_client :( ) */
>
> - SVN_ERR(upgrade_externals_from_properties(ctx, local_abspath,
> + SVN_ERR(upgrade_externals_from_properties(ctx, local_abspath, wc_format,
> &info_baton, scratch_pool));
> }
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_client_upgrade2(const char *path,
> + const char *wc_format_version,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *scratch_pool)
> +{
> + int wc_format;
> +
> + SVN_ERR(svn_wc__format_from_version_string(&wc_format,
> + wc_format_version,
> + scratch_pool));
> + return upgrade_internal(path, wc_format, ctx, scratch_pool);
Suggest to use
SVN_ERR(upgrade_internal(...))
or at least svn_error_trace(), for maintainer mode traces' benefit.
> +++ subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.c Tue Sep 5 07:45:49 2017
> @@ -16053,9 +16054,8 @@ svn_wc__db_bump_format(int *result_forma
> }
>
> SVN_ERR(svn_sqlite__read_schema_version(&format, sdb, scratch_pool));
> - /* TODO: Parametrize the target format here. */
> err = svn_wc__upgrade_sdb(result_format, wcroot_abspath,
> - sdb, format, SVN_WC__VERSION, scratch_pool);
> + sdb, format, target_format, scratch_pool);
(For other reviewers: the upgrade logic invoked here was added/changed
in r1807225.)
Cheers,
Daniel
Re: svn commit: r1807319 - in
/subversion/branches/better-pristines/subversion: include/
include/private/ libsvn_client/ libsvn_wc/ svn/
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Tue, 05 Sep 2017 14:06 +0000:
> Branko Čibej wrote on Tue, 05 Sep 2017 15:56 +0200:
> > On 05.09.2017 15:45, Daniel Shahaf wrote:
> > > brane@apache.org wrote on Tue, 05 Sep 2017 07:45 +0000:
> > >> +svn_client_upgrade2(const char *wcroot_dir,
> > >> + const char* wc_format_version,
> > >> + svn_client_ctx_t *ctx,
> > >> + apr_pool_t *scratch_pool);
> > >> +
> > >> +/**
> > >> + * Like svn_client_upgrade2(), but always upgrades to the newest
> > >> + * supported format.
> > >> + *
> > >> - * @since New in 1.7.
> > >> + * @deprecated Provided for backward compatibility with the 1.7 API.
> > > Why drop @since? We don't usually remove @since tags when deprecating.
> > > (Both here and in svn_wc_upgrade())
> >
> > Yes we do. I looked around and didn't find @since and @deprecated used
> > together.
>
> Counter-examples: svn_cmdline_prompt_baton_t,
> svn_cmdline_create_auth_baton(), svn_txdelta_to_svndiff2().
>
> Functions that have @deprecated but not @since are, the way we've usually
> managed deprecations, supposed to be functions that were present in 1.0.
> (Example: svn_txdelta_to_svndiff())
>
> > The deprecation reason gives the original API version anyway.
> >
>
> This is just a coincidence. We could very well have "@since New in 1.1"
> together with "@deprecated ... for ... the 1.9 API." if we deprecate
> some old function.
This example is for a function that was added in 1.1.0 and first became
deprecated in 1.10.0. Accordingly, the @deprecated annotation on
svn_client_upgrade() should say 1.9, not 1.7.
Cheers,
Daniel
Re: svn commit: r1807319 - in
/subversion/branches/better-pristines/subversion: include/
include/private/ libsvn_client/ libsvn_wc/ svn/
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Tue, 05 Sep 2017 15:56 +0200:
> On 05.09.2017 15:45, Daniel Shahaf wrote:
> > brane@apache.org wrote on Tue, 05 Sep 2017 07:45 +0000:
> >> Author: brane
> >> Date: Tue Sep 5 07:45:49 2017
> >> New Revision: 1807319
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1807319&view=rev
> >> Log:
> >> Introduce the concept of a target format for working copy upgrades
> >> into the client library.
> >> +++ subversion/branches/better-pristines/subversion/include/svn_client.h Tue Sep 5 07:45:49 2017
> >> @@ -4201,13 +4201,34 @@ svn_client_cleanup(const char *dir,
> >> * @{
> >> */
> >>
> >> -/** Recursively upgrade a working copy from any older format to the current
> >> - * WC metadata storage format. @a wcroot_dir is the path to the WC root.
> >> +/**
> >> + * Recursively upgrade a working copy from any older format to the
> >> + * given WC metadata storage format. @a wcroot_dir is the path to the
> >> + * WC root.
> >> + *
> > Suggest to clarify that "Recursively" means into nested working copies
> > (externals), not into subdirs.
>
> Ack. Note that this is just a slightly modified copy of the original
> docstring.
>
Haven't noticed this, but the point is valid anyway :-).
> >> + * @a wc_format_version is version number of the Subversion client
> >> + * that supports a given WC metadata format; @c NULL means the newest
> >> + * supported format. Any other value must be a string representing a
> >> + * version number, e.g., "1.8" or "1.9.3". The earliest supported
> >> + * version is defined by #SVN_VERSION_SUPPORTED_WC_FORMAT.
> >> *
> > When might 1.A.3 and 1.A.4 have different formats?
>
> They might not. However, there's no reason to forbid putting the patch
> version in here. The format of this parameter is analogous to the value
> of the --compatible-version option to svnadmin. Also, using SVN_VERSION
> here should be valid. I intend the parser to just extract the first two
> dot-separated integers and ignore everything else; there's no reason to
> be too strict.
>
+1; that's exactly what I was thinking, too.
> >> * Use @a scratch_pool for any temporary allocations.
> >> *
> >> + * @since New in 1.10.
> >> + */
> >> +svn_error_t *
> >> +svn_client_upgrade2(const char *wcroot_dir,
> >> + const char* wc_format_version,
> >> + svn_client_ctx_t *ctx,
> >> + apr_pool_t *scratch_pool);
> >> +
> >> +/**
> >> + * Like svn_client_upgrade2(), but always upgrades to the newest
> >> + * supported format.
> >> + *
> >> - * @since New in 1.7.
> >> + * @deprecated Provided for backward compatibility with the 1.7 API.
> > Why drop @since? We don't usually remove @since tags when deprecating.
> > (Both here and in svn_wc_upgrade())
>
> Yes we do. I looked around and didn't find @since and @deprecated used
> together.
Counter-examples: svn_cmdline_prompt_baton_t,
svn_cmdline_create_auth_baton(), svn_txdelta_to_svndiff2().
Functions that have @deprecated but not @since are, the way we've usually
managed deprecations, supposed to be functions that were present in 1.0.
(Example: svn_txdelta_to_svndiff())
> The deprecation reason gives the original API version anyway.
>
This is just a coincidence. We could very well have "@since New in 1.1"
together with "@deprecated ... for ... the 1.9 API." if we deprecate
some old function.
Cheers,
Daniel
Re: svn commit: r1807319 - in
/subversion/branches/better-pristines/subversion: include/ include/private/
libsvn_client/ libsvn_wc/ svn/
Posted by Branko Čibej <br...@apache.org>.
On 05.09.2017 15:45, Daniel Shahaf wrote:
> brane@apache.org wrote on Tue, 05 Sep 2017 07:45 +0000:
>> Author: brane
>> Date: Tue Sep 5 07:45:49 2017
>> New Revision: 1807319
>>
>> URL: http://svn.apache.org/viewvc?rev=1807319&view=rev
>> Log:
>> Introduce the concept of a target format for working copy upgrades
>> into the client library.
>> +++ subversion/branches/better-pristines/subversion/include/svn_client.h Tue Sep 5 07:45:49 2017
>> @@ -4201,13 +4201,34 @@ svn_client_cleanup(const char *dir,
>> * @{
>> */
>>
>> -/** Recursively upgrade a working copy from any older format to the current
>> - * WC metadata storage format. @a wcroot_dir is the path to the WC root.
>> +/**
>> + * Recursively upgrade a working copy from any older format to the
>> + * given WC metadata storage format. @a wcroot_dir is the path to the
>> + * WC root.
>> + *
> Suggest to clarify that "Recursively" means into nested working copies
> (externals), not into subdirs.
Ack. Note that this is just a slightly modified copy of the original
docstring.
>> + * @a wc_format_version is version number of the Subversion client
>> + * that supports a given WC metadata format; @c NULL means the newest
>> + * supported format. Any other value must be a string representing a
>> + * version number, e.g., "1.8" or "1.9.3". The earliest supported
>> + * version is defined by #SVN_VERSION_SUPPORTED_WC_FORMAT.
>> *
> When might 1.A.3 and 1.A.4 have different formats?
They might not. However, there's no reason to forbid putting the patch
version in here. The format of this parameter is analogous to the value
of the --compatible-version option to svnadmin. Also, using SVN_VERSION
here should be valid. I intend the parser to just extract the first two
dot-separated integers and ignore everything else; there's no reason to
be too strict.
>> * Use @a scratch_pool for any temporary allocations.
>> *
>> + * @since New in 1.10.
>> + */
>> +svn_error_t *
>> +svn_client_upgrade2(const char *wcroot_dir,
>> + const char* wc_format_version,
>> + svn_client_ctx_t *ctx,
>> + apr_pool_t *scratch_pool);
>> +
>> +/**
>> + * Like svn_client_upgrade2(), but always upgrades to the newest
>> + * supported format.
>> + *
>> - * @since New in 1.7.
>> + * @deprecated Provided for backward compatibility with the 1.7 API.
> Why drop @since? We don't usually remove @since tags when deprecating.
> (Both here and in svn_wc_upgrade())
Yes we do. I looked around and didn't find @since and @deprecated used
together. The deprecation reason gives the original API version anyway.
>> +++ subversion/branches/better-pristines/subversion/include/svn_version.h Tue Sep 5 07:45:49 2017
>> @@ -134,6 +134,12 @@ extern "C" {
>> #define SVN_VERSION SVN_VER_NUMBER SVN_VER_TAG
>>
>>
>> +/**
>> + * Earliest supported working copy version.
>> + * @since New in 1.10.
>> + */
>> +#define SVN_VERSION_SUPPORTED_WC_FORMAT "1.8.0"
> Shouldn't this be a function? It depends not on the compile-time
> library version but on the run-time library version.
Hmm ... good point. Will fix.
>> +svn_error_t *
>> +svn_client_upgrade2(const char *path,
>> + const char *wc_format_version,
>> + svn_client_ctx_t *ctx,
>> + apr_pool_t *scratch_pool)
>> +{
>> + int wc_format;
>> +
>> + SVN_ERR(svn_wc__format_from_version_string(&wc_format,
>> + wc_format_version,
>> + scratch_pool));
>> + return upgrade_internal(path, wc_format, ctx, scratch_pool);
> Suggest to use
>
> SVN_ERR(upgrade_internal(...))
>
> or at least svn_error_trace(), for maintainer mode traces' benefit.
Ack.
-- Brane