You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2017/09/05 07:45:50 UTC

svn commit: r1807319 - in /subversion/branches/better-pristines/subversion: include/ include/private/ libsvn_client/ libsvn_wc/ svn/

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/include/svn_client.h
  (svn_client_upgrade2): New public function.
  (svn_client_upgrade): Deprecate.
* subversion/libsvn_client/upgrade.c
  (upgrade_externals_from_properties): Add parameter wc_format.
  (upgrade_internal): New helper function, extracted from svn_client_upgrade.
  (svn_client_upgrade2): Implement.
  (upgrade_external_item): Add parameter wc_format.
* subversion/libsvn_client/deprecated.c
  (svn_client_upgrade): Move here from upgrade.c

* subversion/include/svn_wc.h
  (svn_wc_upgrade): Deprecate.
* subversion/include/private/svn_wc_private.h
  (svn_wc__format_from_version_string, svn_wc__upgrade): New.
* subversion/libsvn_wc/wc_db.h
  (svn_wc__db_bump_format): Add parameter target_format.
* subversion/libsvn_wc/upgrade.c
  (svn_wc__format_from_version_string): Implement.
  (svn_wc__upgrade): Implement; renamed and updated from svn_wc_upgrade.
* subversion/libsvn_wc/wc_db.c
  (svn_wc__db_bump_format): Use parameter target_format.
* subversion/libsvn_wc/deprecated.c
  (svn_wc_upgrade): Move here from upgrade.c

* subversion/include/svn_version.h
  (SVN_VERSION_SUPPORTED_WC_FORMAT): New.

* subversion/svn/upgrade-cmd.c
  (svn_cl__upgrade): Call svn_client_upgrade2.

Modified:
    subversion/branches/better-pristines/subversion/include/private/svn_wc_private.h
    subversion/branches/better-pristines/subversion/include/svn_client.h
    subversion/branches/better-pristines/subversion/include/svn_version.h
    subversion/branches/better-pristines/subversion/include/svn_wc.h
    subversion/branches/better-pristines/subversion/libsvn_client/deprecated.c
    subversion/branches/better-pristines/subversion/libsvn_client/upgrade.c
    subversion/branches/better-pristines/subversion/libsvn_wc/deprecated.c
    subversion/branches/better-pristines/subversion/libsvn_wc/upgrade.c
    subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.c
    subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.h
    subversion/branches/better-pristines/subversion/svn/upgrade-cmd.c

Modified: subversion/branches/better-pristines/subversion/include/private/svn_wc_private.h
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/include/private/svn_wc_private.h?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/include/private/svn_wc_private.h (original)
+++ subversion/branches/better-pristines/subversion/include/private/svn_wc_private.h Tue Sep  5 07:45:49 2017
@@ -2073,6 +2073,52 @@ svn_wc__translated_stream(svn_stream_t *
                           apr_pool_t *result_pool,
                           apr_pool_t *scratch_pool);
 
+
+/**
+ * Convert @a version, a string representation of a version number, to
+ * that version's characteristic working copy format, returned in @a
+ * format.
+ *
+ * Use @a scratch_pool for temporary allocations.
+ *
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_wc__format_from_version_string(int *format, const char* version,
+                                   apr_pool_t *scratch_pool);
+
+/**
+ * Upgrade the working copy at @a local_abspath to the metadata
+ * storage format indicated by @a target_format.  @a local_abspath
+ * should be an absolute path to the root of the working copy.
+ *
+ * If @a cancel_func is non-NULL, invoke it with @a cancel_baton at
+ * various points during the operation.  If it returns an error
+ * (typically #SVN_ERR_CANCELLED), return that error immediately.
+ *
+ * For each directory converted, @a notify_func will be called with
+ * in @a notify_baton action #svn_wc_notify_upgraded_path and as path
+ * the path of the upgraded directory. @a notify_func may be @c NULL
+ * if this notification is not needed.
+ *
+ * If the old working copy doesn't contain a repository root and/or
+ * 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.10.
+ */
+svn_error_t *
+svn_wc__upgrade(svn_wc_context_t *wc_ctx,
+                const char *local_abspath,
+                int target_format,
+                svn_wc_upgrade_get_repos_info_t repos_info_func,
+                void *repos_info_baton,
+                svn_cancel_func_t cancel_func,
+                void *cancel_baton,
+                svn_wc_notify_func2_t notify_func,
+                void *notify_baton,
+                apr_pool_t *scratch_pool);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/branches/better-pristines/subversion/include/svn_client.h
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/include/svn_client.h?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/include/svn_client.h (original)
+++ 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.
+ *
+ * @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.
  *
  * Use @a scratch_pool for any temporary allocations.
  *
- * @since New in 1.7.
+ * @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.
+ *
+ * @deprecated Provided for backward compatibility with the 1.7 API.
  */
+SVN_DEPRECATED
 svn_error_t *
 svn_client_upgrade(const char *wcroot_dir,
                    svn_client_ctx_t *ctx,

Modified: subversion/branches/better-pristines/subversion/include/svn_version.h
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/include/svn_version.h?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/include/svn_version.h (original)
+++ 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"
+
 
 /* Version queries and compatibility checks */
 

Modified: subversion/branches/better-pristines/subversion/include/svn_wc.h
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/include/svn_wc.h?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/include/svn_wc.h (original)
+++ 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.
  */
+SVN_DEPRECATED
 svn_error_t *
 svn_wc_upgrade(svn_wc_context_t *wc_ctx,
                const char *local_abspath,

Modified: subversion/branches/better-pristines/subversion/libsvn_client/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/libsvn_client/deprecated.c?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/libsvn_client/deprecated.c (original)
+++ subversion/branches/better-pristines/subversion/libsvn_client/deprecated.c Tue Sep  5 07:45:49 2017
@@ -3107,3 +3107,11 @@ svn_client_cleanup(const char *path,
                                              FALSE /* include_externals */,
                                              ctx, scratch_pool));
 }
+
+svn_error_t *
+svn_client_upgrade(const char *path,
+                   svn_client_ctx_t *ctx,
+                   apr_pool_t *scratch_pool)
+{
+  return svn_client_upgrade2(path, NULL, ctx, scratch_pool);
+}

Modified: subversion/branches/better-pristines/subversion/libsvn_client/upgrade.c
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/libsvn_client/upgrade.c?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/libsvn_client/upgrade.c (original)
+++ subversion/branches/better-pristines/subversion/libsvn_client/upgrade.c Tue Sep  5 07:45:49 2017
@@ -89,13 +89,15 @@ fetch_repos_info(const char **repos_root
 static svn_error_t *
 upgrade_externals_from_properties(svn_client_ctx_t *ctx,
                                   const char *local_abspath,
+                                  int wc_format,
                                   struct repos_info_baton *info_baton,
                                   apr_pool_t *scratch_pool);
 
-svn_error_t *
-svn_client_upgrade(const char *path,
-                   svn_client_ctx_t *ctx,
-                   apr_pool_t *scratch_pool)
+static svn_error_t *
+upgrade_internal(const char *path,
+                 int wc_format,
+                 svn_client_ctx_t *ctx,
+                 apr_pool_t *scratch_pool)
 {
   const char *local_abspath;
   apr_hash_t *externals;
@@ -111,11 +113,11 @@ svn_client_upgrade(const char *path,
                              _("'%s' is not a local path"), path);
 
   SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, scratch_pool));
-  SVN_ERR(svn_wc_upgrade(ctx->wc_ctx, local_abspath,
-                         fetch_repos_info, &info_baton,
-                         ctx->cancel_func, ctx->cancel_baton,
-                         ctx->notify_func2, ctx->notify_baton2,
-                         scratch_pool));
+  SVN_ERR(svn_wc__upgrade(ctx->wc_ctx, local_abspath, wc_format,
+                          fetch_repos_info, &info_baton,
+                          ctx->cancel_func, ctx->cancel_baton,
+                          ctx->notify_func2, ctx->notify_baton2,
+                          scratch_pool));
 
   SVN_ERR(svn_wc__externals_defined_below(&externals,
                                           ctx->wc_ctx, local_abspath,
@@ -149,7 +151,8 @@ svn_client_upgrade(const char *path,
 
           if (kind == svn_node_dir)
             {
-              svn_error_t *err = svn_client_upgrade(ext_abspath, ctx, iterpool);
+              svn_error_t *err = upgrade_internal(ext_abspath, wc_format,
+                                                  ctx, iterpool);
 
               if (err)
                 {
@@ -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);
+}
+
 /* Helper for upgrade_externals_from_properties: upgrades one external ITEM
    in EXTERNALS_PARENT. Uses SCRATCH_POOL for temporary allocations. */
 static svn_error_t *
-upgrade_external_item(svn_client_ctx_t *ctx,
+upgrade_external_item(svn_client_ctx_t *ctx, int wc_format,
                       const char *externals_parent_abspath,
                       const char *externals_parent_url,
                       const char *externals_parent_repos_root_url,
@@ -211,7 +228,7 @@ upgrade_external_item(svn_client_ctx_t *
               externals_parent_url,
               scratch_pool, scratch_pool));
 
-  /* This is a hack. We only need to call svn_wc_upgrade() on external
+  /* This is a hack. We only need to call svn_wc__upgrade() on external
    * dirs, as file externals are upgraded along with their defining
    * WC.  Reading the kind will throw an exception on an external dir,
    * saying that the wc must be upgraded.  If it's a file, the lookup
@@ -225,7 +242,7 @@ upgrade_external_item(svn_client_ctx_t *
     {
       svn_error_clear(err);
 
-      SVN_ERR(svn_client_upgrade(external_abspath, ctx, scratch_pool));
+      SVN_ERR(upgrade_internal(external_abspath, wc_format, ctx, scratch_pool));
     }
   else if (err)
     return svn_error_trace(err);
@@ -298,6 +315,7 @@ upgrade_external_item(svn_client_ctx_t *
 static svn_error_t *
 upgrade_externals_from_properties(svn_client_ctx_t *ctx,
                                   const char *local_abspath,
+                                  int wc_format,
                                   struct repos_info_baton *info_baton,
                                   apr_pool_t *scratch_pool)
 {
@@ -383,7 +401,8 @@ upgrade_externals_from_properties(svn_cl
           item = APR_ARRAY_IDX(externals_p, i, svn_wc_external_item2_t*);
 
           svn_pool_clear(iterpool2);
-          err = upgrade_external_item(ctx, externals_parent_abspath,
+          err = upgrade_external_item(ctx, wc_format,
+                                      externals_parent_abspath,
                                       externals_parent_url,
                                       externals_parent_repos_root_url,
                                       item, info_baton, iterpool2);

Modified: subversion/branches/better-pristines/subversion/libsvn_wc/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/libsvn_wc/deprecated.c?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/libsvn_wc/deprecated.c (original)
+++ subversion/branches/better-pristines/subversion/libsvn_wc/deprecated.c Tue Sep  5 07:45:49 2017
@@ -4846,3 +4846,21 @@ svn_wc__conflict_description2_dup(const
 {
   return svn_wc_conflict_description2_dup(conflict, pool);
 }
+
+svn_error_t *
+svn_wc_upgrade(svn_wc_context_t *wc_ctx,
+               const char *local_abspath,
+               svn_wc_upgrade_get_repos_info_t repos_info_func,
+               void *repos_info_baton,
+               svn_cancel_func_t cancel_func,
+               void *cancel_baton,
+               svn_wc_notify_func2_t notify_func,
+               void *notify_baton,
+               apr_pool_t *scratch_pool)
+{
+  return svn_wc__upgrade(wc_ctx, local_abspath, SVN_WC__VERSION,
+                         repos_info_func, repos_info_baton,
+                         cancel_func, cancel_baton,
+                         notify_func, notify_baton,
+                         scratch_pool);
+}

Modified: subversion/branches/better-pristines/subversion/libsvn_wc/upgrade.c
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/libsvn_wc/upgrade.c?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/libsvn_wc/upgrade.c (original)
+++ subversion/branches/better-pristines/subversion/libsvn_wc/upgrade.c Tue Sep  5 07:45:49 2017
@@ -2072,6 +2072,15 @@ svn_wc__version_string_from_format(int w
   return _("(unreleased development version)");
 }
 
+svn_error_t *
+svn_wc__format_from_version_string(int *format,
+                                   const char *version_string,
+                                   apr_pool_t *scratch_pool)
+{
+  /* TODO: Parse VERSION_STRING to get *FORMAT */
+  *format = SVN_WC__VERSION;
+  return SVN_NO_ERROR;
+}
 
 svn_error_t *
 svn_wc__upgrade_sdb(int *result_format,
@@ -2416,15 +2425,16 @@ is_old_wcroot(const char *local_abspath,
 }
 
 svn_error_t *
-svn_wc_upgrade(svn_wc_context_t *wc_ctx,
-               const char *local_abspath,
-               svn_wc_upgrade_get_repos_info_t repos_info_func,
-               void *repos_info_baton,
-               svn_cancel_func_t cancel_func,
-               void *cancel_baton,
-               svn_wc_notify_func2_t notify_func,
-               void *notify_baton,
-               apr_pool_t *scratch_pool)
+svn_wc__upgrade(svn_wc_context_t *wc_ctx,
+                const char *local_abspath,
+                int target_format,
+                svn_wc_upgrade_get_repos_info_t repos_info_func,
+                void *repos_info_baton,
+                svn_cancel_func_t cancel_func,
+                void *cancel_baton,
+                svn_wc_notify_func2_t notify_func,
+                void *notify_baton,
+                apr_pool_t *scratch_pool)
 {
   svn_wc__db_t *db;
   struct upgrade_data_t data = { NULL };
@@ -2444,7 +2454,7 @@ svn_wc_upgrade(svn_wc_context_t *wc_ctx,
 
 
   err = svn_wc__db_bump_format(&result_format, &bumped_format,
-                               db, local_abspath,
+                               db, local_abspath, target_format,
                                scratch_pool);
   if (err)
     {

Modified: subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.c?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.c Tue Sep  5 07:45:49 2017
@@ -16010,6 +16010,7 @@ svn_wc__db_bump_format(int *result_forma
                        svn_boolean_t *bumped_format,
                        svn_wc__db_t *db,
                        const char *wcroot_abspath,
+                       int target_format,
                        apr_pool_t *scratch_pool)
 {
   svn_sqlite__db_t *sdb;
@@ -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);
 
   if (err == SVN_NO_ERROR && bumped_format)
     *bumped_format = (*result_format > format);

Modified: subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.h?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/branches/better-pristines/subversion/libsvn_wc/wc_db.h Tue Sep  5 07:45:49 2017
@@ -2955,7 +2955,7 @@ svn_wc__db_upgrade_insert_external(svn_w
                                    apr_pool_t *scratch_pool);
 
 /* Upgrade the metadata concerning the WC at WCROOT_ABSPATH, in DB,
- * to the SVN_WC__VERSION format.
+ * to the TARGET_FORMAT metadata format version.
  *
  * This function is used for upgrading wc-ng working copies to a newer
  * wc-ng format. If a pre-1.7 working copy is found, this function
@@ -2973,6 +2973,7 @@ svn_wc__db_bump_format(int *result_forma
                        svn_boolean_t *bumped_format,
                        svn_wc__db_t *db,
                        const char *wcroot_abspath,
+                       int target_format,
                        apr_pool_t *scratch_pool);
 
 /* @} */

Modified: subversion/branches/better-pristines/subversion/svn/upgrade-cmd.c
URL: http://svn.apache.org/viewvc/subversion/branches/better-pristines/subversion/svn/upgrade-cmd.c?rev=1807319&r1=1807318&r2=1807319&view=diff
==============================================================================
--- subversion/branches/better-pristines/subversion/svn/upgrade-cmd.c (original)
+++ subversion/branches/better-pristines/subversion/svn/upgrade-cmd.c Tue Sep  5 07:45:49 2017
@@ -70,7 +70,7 @@ svn_cl__upgrade(apr_getopt_t *os,
 
       svn_pool_clear(iterpool);
       SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
-      SVN_ERR(svn_client_upgrade(target, ctx, scratch_pool));
+      SVN_ERR(svn_client_upgrade2(target, /* TODO: */NULL, ctx, scratch_pool));
     }
   svn_pool_destroy(iterpool);
 



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

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>.
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>.
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