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 2010/06/10 21:49:23 UTC

svn commit: r953428 - in /subversion/trunk/subversion: include/ libsvn_subr/ svn/

Author: julianfoad
Date: Thu Jun 10 19:49:22 2010
New Revision: 953428

URL: http://svn.apache.org/viewvc?rev=953428&view=rev
Log:
Remove svn_opt_eat_peg_revisions() from the public API name space, making it
a private function in the 'svn' client, named accordingly.  It was
introduced in 1.6.x with a private double-underscore name and has not yet
gone public in 1.7, so we can do this.

The idea of introducing and using this function was to allow arguments that
do not take a peg revision to nevertheless be written with peg revision
syntax (i.e. a trailing "@" sign can be given to "escape" any preceding "@"
sign.  However, this particular API is not a good one to make public because
the caller is unable to detect if an inappropriate peg specifier was given.

* subversion/include/svn_opt.h
  (svn_opt_eat_peg_revisions): Move and rename to ...

* subversionsvn/cl.h
  (svn_cl__eat_peg_revisions) ... here.

* subversion/libsvn_subr/opt.c
  (svn_opt__eat_peg_revisions): Remove this wrapper, as compatibility for
    1.6 for svn executable is not required.
  (svn_opt_eat_peg_revisions): ... move and rename to ...

* subversion/svn/util.c
  (svn_cl__eat_peg_revisions) ... here.

* subversion/svn/add-cmd.c
* subversion/svn/changelist-cmd.c
* subversion/svn/cleanup-cmd.c
* subversion/svn/commit-cmd.c
* subversion/svn/copy-cmd.c
* subversion/svn/delete-cmd.c
* subversion/svn/lock-cmd.c
* subversion/svn/mkdir-cmd.c
* subversion/svn/move-cmd.c
* subversion/svn/patch-cmd.c
* subversion/svn/propdel-cmd.c
* subversion/svn/propedit-cmd.c
* subversion/svn/propset-cmd.c
* subversion/svn/resolve-cmd.c
* subversion/svn/resolved-cmd.c
* subversion/svn/revert-cmd.c
* subversion/svn/status-cmd.c
* subversion/svn/unlock-cmd.c
* subversion/svn/update-cmd.c
* subversion/svn/upgrade-cmd.c
  Adjust callers.

Modified:
    subversion/trunk/subversion/include/svn_opt.h
    subversion/trunk/subversion/libsvn_subr/opt.c
    subversion/trunk/subversion/svn/add-cmd.c
    subversion/trunk/subversion/svn/changelist-cmd.c
    subversion/trunk/subversion/svn/cl.h
    subversion/trunk/subversion/svn/cleanup-cmd.c
    subversion/trunk/subversion/svn/commit-cmd.c
    subversion/trunk/subversion/svn/copy-cmd.c
    subversion/trunk/subversion/svn/delete-cmd.c
    subversion/trunk/subversion/svn/lock-cmd.c
    subversion/trunk/subversion/svn/mkdir-cmd.c
    subversion/trunk/subversion/svn/move-cmd.c
    subversion/trunk/subversion/svn/patch-cmd.c
    subversion/trunk/subversion/svn/propdel-cmd.c
    subversion/trunk/subversion/svn/propedit-cmd.c
    subversion/trunk/subversion/svn/propset-cmd.c
    subversion/trunk/subversion/svn/resolve-cmd.c
    subversion/trunk/subversion/svn/resolved-cmd.c
    subversion/trunk/subversion/svn/revert-cmd.c
    subversion/trunk/subversion/svn/status-cmd.c
    subversion/trunk/subversion/svn/unlock-cmd.c
    subversion/trunk/subversion/svn/update-cmd.c
    subversion/trunk/subversion/svn/upgrade-cmd.c
    subversion/trunk/subversion/svn/util.c

Modified: subversion/trunk/subversion/include/svn_opt.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_opt.h?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_opt.h (original)
+++ subversion/trunk/subversion/include/svn_opt.h Thu Jun 10 19:49:22 2010
@@ -744,29 +744,6 @@ svn_opt_print_help(apr_getopt_t *os,
                    const char *footer,
                    apr_pool_t *pool);
 
-/* Return, in @a *true_targets_p, a copy of @a targets with peg revision
- * specifiers snipped off the end of each element.
- *
- * This function is useful for subcommands for which peg revisions
- * do not make any sense. Such subcommands still need to allow peg
- * revisions to be specified on the command line so that users of
- * the command line client can consistently escape '@' characters
- * in filenames by appending an '@' character, regardless of the
- * subcommand being used.
- *
- * If a peg revision is present but cannot be parsed, an error is thrown.
- * The user has likely forgotten to escape an '@' character in a filename.
- *
- * It is safe to pass the address of @a targets as @a true_targets_p.
- *
- * Do all allocations in @a pool.
- *
- * @since New in 1.7. */
-svn_error_t *
-svn_opt_eat_peg_revisions(apr_array_header_t **true_targets_p,
-                          const apr_array_header_t *targets,
-                          apr_pool_t *pool);
-
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/trunk/subversion/libsvn_subr/opt.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/opt.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/opt.c (original)
+++ subversion/trunk/subversion/libsvn_subr/opt.c Thu Jun 10 19:49:22 2010
@@ -1064,46 +1064,3 @@ svn_opt_print_help3(apr_getopt_t *os,
 
   return SVN_NO_ERROR;
 }
-
-/* svn_opt__eat_peg_revisions was added to the 1.6 branch and, despite
-   its double underscore name, it is called from the application
-   layer.  So to remain ABI compatibilty with 1.6 the name must
-   continue to exist.  Adding this name is sufficient for the 1.6
-   client to link against the 1.7 libraries. */
-svn_error_t *
-svn_opt__eat_peg_revisions(apr_array_header_t **true_targets_p,
-                           const apr_array_header_t *targets,
-                           apr_pool_t *pool);
-svn_error_t *
-svn_opt__eat_peg_revisions(apr_array_header_t **true_targets_p,
-                           const apr_array_header_t *targets,
-                           apr_pool_t *pool)
-{
-  return svn_opt_eat_peg_revisions(true_targets_p, targets, pool);
-}
-
-svn_error_t *
-svn_opt_eat_peg_revisions(apr_array_header_t **true_targets_p,
-                          const apr_array_header_t *targets,
-                          apr_pool_t *pool)
-{
-  int i;
-  apr_array_header_t *true_targets;
-
-  true_targets = apr_array_make(pool, DEFAULT_ARRAY_SIZE, sizeof(const char *));
-
-  for (i = 0; i < targets->nelts; i++)
-    {
-      const char *target = APR_ARRAY_IDX(targets, i, const char *);
-      const char *true_target;
-
-      SVN_ERR(svn_opt__split_arg_at_peg_revision(&true_target, NULL,
-                                                 target, pool));
-      APR_ARRAY_PUSH(true_targets, const char *) = true_target;
-    }
-
-  SVN_ERR_ASSERT(true_targets_p);
-  *true_targets_p = true_targets;
-
-  return SVN_NO_ERROR;
-}

Modified: subversion/trunk/subversion/svn/add-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/add-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/add-cmd.c (original)
+++ subversion/trunk/subversion/svn/add-cmd.c Thu Jun 10 19:49:22 2010
@@ -64,7 +64,7 @@ svn_cl__add(apr_getopt_t *os,
   if (opt_state->depth == svn_depth_unknown)
     opt_state->depth = svn_depth_infinity;
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   subpool = svn_pool_create(pool);
   for (i = 0; i < targets->nelts; i++)

Modified: subversion/trunk/subversion/svn/changelist-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/changelist-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/changelist-cmd.c (original)
+++ subversion/trunk/subversion/svn/changelist-cmd.c Thu Jun 10 19:49:22 2010
@@ -97,7 +97,7 @@ svn_cl__changelist(apr_getopt_t *os,
   if (depth == svn_depth_unknown)
     depth = svn_depth_empty;
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   if (changelist_name)
     {

Modified: subversion/trunk/subversion/svn/cl.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cl.h?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cl.h (original)
+++ subversion/trunk/subversion/svn/cl.h Thu Jun 10 19:49:22 2010
@@ -736,6 +736,50 @@ svn_cl__path_join(const char *base,
                   const char *component,
                   apr_pool_t *pool);
 
+/* Return, in @a *true_targets_p, a copy of @a targets with peg revision
+ * specifiers snipped off the end of each element.
+ *
+ * ### JAF TODO: This function is not good because it does not allow the
+ * ### caller to detect if an invalid peg revision was specified.
+ * ###
+ * ### Callers should never have a need to silently *discard* all peg
+ * ### revisions, even if they are doing this *after* saving any peg
+ * ### revisions that might be of interest on certain arguments: I don't
+ * ### think it can ever be correct to silently ignore a peg revision that
+ * ### is specified, whether it makes semantic sense or not.
+ * ###
+ * ### Instead, callers should parse all the arguments and silently
+ * ### ignore an *empty* peg revision part (just an "@", which can be
+ * ### used to escape an earlier "@" in the argument) on any argument,
+ * ### even an argument on which a peg revision does not make sense,
+ * ### but should not silently ignore a non-empty peg when it does not
+ * ### make sense.
+ * ###
+ * ### Something like:
+ * ###   For each (URL-like?) argument that doesn't accept a peg rev:
+ * ###     Parse into peg-rev and true-path parts;
+ * ###     If (peg rev != unspecified)
+ * ###       Error("This arg doesn't accept a peg rev.").
+ * ###     Use the true-path part.
+ *
+ * This function is useful for subcommands for which peg revisions
+ * do not make any sense. Such subcommands still need to allow peg
+ * revisions to be specified on the command line so that users of
+ * the command line client can consistently escape '@' characters
+ * in filenames by appending an '@' character, regardless of the
+ * subcommand being used.
+ *
+ * If a peg revision is present but cannot be parsed, an error is thrown.
+ * The user has likely forgotten to escape an '@' character in a filename.
+ *
+ * It is safe to pass the address of @a targets as @a true_targets_p.
+ *
+ * Do all allocations in @a pool. */
+svn_error_t *
+svn_cl__eat_peg_revisions(apr_array_header_t **true_targets_p,
+                          const apr_array_header_t *targets,
+                          apr_pool_t *pool);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/trunk/subversion/svn/cleanup-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cleanup-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cleanup-cmd.c (original)
+++ subversion/trunk/subversion/svn/cleanup-cmd.c Thu Jun 10 19:49:22 2010
@@ -55,7 +55,7 @@ svn_cl__cleanup(apr_getopt_t *os,
   /* Add "." if user passed 0 arguments */
   svn_opt_push_implicit_dot_target(targets, pool);
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   subpool = svn_pool_create(pool);
   for (i = 0; i < targets->nelts; i++)

Modified: subversion/trunk/subversion/svn/commit-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/commit-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/commit-cmd.c (original)
+++ subversion/trunk/subversion/svn/commit-cmd.c Thu Jun 10 19:49:22 2010
@@ -74,7 +74,7 @@ svn_cl__commit(apr_getopt_t *os,
   /* Add "." if user passed 0 arguments. */
   svn_opt_push_implicit_dot_target(targets, pool);
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   /* Condense the targets (like commit does)... */
   SVN_ERR(svn_path_condense_targets(&base_dir,

Modified: subversion/trunk/subversion/svn/copy-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/copy-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/copy-cmd.c (original)
+++ subversion/trunk/subversion/svn/copy-cmd.c Thu Jun 10 19:49:22 2010
@@ -77,7 +77,7 @@ svn_cl__copy(apr_getopt_t *os,
       APR_ARRAY_PUSH(sources, svn_client_copy_source_t *) = source;
     }
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   /* Figure out which type of trace editor to use.
      If the src_paths are not homogeneous, setup_copy will return an error. */

Modified: subversion/trunk/subversion/svn/delete-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/delete-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/delete-cmd.c (original)
+++ subversion/trunk/subversion/svn/delete-cmd.c Thu Jun 10 19:49:22 2010
@@ -77,7 +77,7 @@ svn_cl__delete(apr_getopt_t *os,
                                          NULL, ctx->config, pool));
     }
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   err = svn_client_delete3(&commit_info, targets, opt_state->force,
                            opt_state->keep_local, opt_state->revprop_table,

Modified: subversion/trunk/subversion/svn/lock-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/lock-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/lock-cmd.c (original)
+++ subversion/trunk/subversion/svn/lock-cmd.c Thu Jun 10 19:49:22 2010
@@ -104,7 +104,7 @@ svn_cl__lock(apr_getopt_t *os,
   SVN_ERR(svn_cl__get_notifier(&ctx->notify_func2, &ctx->notify_baton2, FALSE,
                                FALSE, FALSE, pool));
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   return svn_client_lock(targets, comment, opt_state->force, ctx, pool);
 }

Modified: subversion/trunk/subversion/svn/mkdir-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/mkdir-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/mkdir-cmd.c (original)
+++ subversion/trunk/subversion/svn/mkdir-cmd.c Thu Jun 10 19:49:22 2010
@@ -78,7 +78,7 @@ svn_cl__mkdir(apr_getopt_t *os,
                                          NULL, ctx->config, pool));
     }
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   err = svn_client_mkdir3(&commit_info, targets, opt_state->parents,
                           opt_state->revprop_table, ctx, pool);

Modified: subversion/trunk/subversion/svn/move-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/move-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/move-cmd.c (original)
+++ subversion/trunk/subversion/svn/move-cmd.c Thu Jun 10 19:49:22 2010
@@ -87,7 +87,7 @@ svn_cl__move(apr_getopt_t *os,
     SVN_ERR(svn_cl__make_log_msg_baton(&(ctx->log_msg_baton3), opt_state,
                                        NULL, ctx->config, pool));
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   err = svn_client_move5(&commit_info, targets, dst_path, opt_state->force,
                          TRUE, opt_state->parents, opt_state->revprop_table,

Modified: subversion/trunk/subversion/svn/patch-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/patch-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/patch-cmd.c (original)
+++ subversion/trunk/subversion/svn/patch-cmd.c Thu Jun 10 19:49:22 2010
@@ -67,7 +67,7 @@ svn_cl__patch(apr_getopt_t *os,
     return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, 0, NULL);
 
   svn_opt_push_implicit_dot_target(targets, pool);
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
   SVN_ERR(svn_dirent_get_absolute(&abs_target_path,
                                   APR_ARRAY_IDX(targets, 0, const char *),
                                   pool));

Modified: subversion/trunk/subversion/svn/propdel-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/propdel-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/propdel-cmd.c (original)
+++ subversion/trunk/subversion/svn/propdel-cmd.c Thu Jun 10 19:49:22 2010
@@ -97,7 +97,7 @@ svn_cl__propdel(apr_getopt_t *os,
       ctx->notify_baton2 = &nwb;
     }
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
   if (opt_state->revprop)  /* operate on a revprop */
     {

Modified: subversion/trunk/subversion/svn/propedit-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/propedit-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/propedit-cmd.c (original)
+++ subversion/trunk/subversion/svn/propedit-cmd.c Thu Jun 10 19:49:22 2010
@@ -176,7 +176,7 @@ svn_cl__propedit(apr_getopt_t *os,
              _("Explicit target argument required"));
         }
 
-      SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
+      SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, pool));
 
       /* For each target, edit the property PNAME. */
       for (i = 0; i < targets->nelts; i++)

Modified: subversion/trunk/subversion/svn/propset-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/propset-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/propset-cmd.c (original)
+++ subversion/trunk/subversion/svn/propset-cmd.c Thu Jun 10 19:49:22 2010
@@ -109,7 +109,7 @@ svn_cl__propset(apr_getopt_t *os,
   if (opt_state->revprop)
     svn_opt_push_implicit_dot_target(targets, scratch_pool);
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, scratch_pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
 
   if (opt_state->revprop)  /* operate on a revprop */
     {

Modified: subversion/trunk/subversion/svn/resolve-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/resolve-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/resolve-cmd.c (original)
+++ subversion/trunk/subversion/svn/resolve-cmd.c Thu Jun 10 19:49:22 2010
@@ -95,7 +95,7 @@ svn_cl__resolve(apr_getopt_t *os,
   if (opt_state->depth == svn_depth_unknown)
     opt_state->depth = svn_depth_empty;
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, scratch_pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
 
   iterpool = svn_pool_create(scratch_pool);
   for (i = 0; i < targets->nelts; i++)

Modified: subversion/trunk/subversion/svn/resolved-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/resolved-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/resolved-cmd.c (original)
+++ subversion/trunk/subversion/svn/resolved-cmd.c Thu Jun 10 19:49:22 2010
@@ -66,7 +66,7 @@ svn_cl__resolved(apr_getopt_t *os,
   if (opt_state->depth == svn_depth_unknown)
     opt_state->depth = svn_depth_empty;
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, scratch_pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
 
   iterpool = svn_pool_create(scratch_pool);
   for (i = 0; i < targets->nelts; i++)

Modified: subversion/trunk/subversion/svn/revert-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/revert-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/revert-cmd.c (original)
+++ subversion/trunk/subversion/svn/revert-cmd.c Thu Jun 10 19:49:22 2010
@@ -65,7 +65,7 @@ svn_cl__revert(apr_getopt_t *os,
   if (opt_state->depth == svn_depth_unknown)
     opt_state->depth = svn_depth_empty;
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, scratch_pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
 
   err = svn_client_revert2(targets, opt_state->depth,
                            opt_state->changelists, ctx, scratch_pool);

Modified: subversion/trunk/subversion/svn/status-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/status-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/status-cmd.c (original)
+++ subversion/trunk/subversion/svn/status-cmd.c Thu Jun 10 19:49:22 2010
@@ -290,7 +290,7 @@ svn_cl__status(apr_getopt_t *os,
   sb.tree_conflicts = 0;
   sb.ctx = ctx;
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, scratch_pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
 
   iterpool = svn_pool_create(scratch_pool);
   for (i = 0; i < targets->nelts; i++)

Modified: subversion/trunk/subversion/svn/unlock-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/unlock-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/unlock-cmd.c (original)
+++ subversion/trunk/subversion/svn/unlock-cmd.c Thu Jun 10 19:49:22 2010
@@ -60,7 +60,7 @@ svn_cl__unlock(apr_getopt_t *os,
   SVN_ERR(svn_cl__get_notifier(&ctx->notify_func2, &ctx->notify_baton2, FALSE,
                                FALSE, FALSE, scratch_pool));
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, scratch_pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
 
   return svn_error_return(
     svn_client_unlock(targets, opt_state->force, ctx, scratch_pool));

Modified: subversion/trunk/subversion/svn/update-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/update-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/update-cmd.c (original)
+++ subversion/trunk/subversion/svn/update-cmd.c Thu Jun 10 19:49:22 2010
@@ -58,7 +58,7 @@ svn_cl__update(apr_getopt_t *os,
   /* Add "." if user passed 0 arguments */
   svn_opt_push_implicit_dot_target(targets, scratch_pool);
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, scratch_pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
 
   /* If using changelists, convert targets into a set of paths that
      match the specified changelist(s). */

Modified: subversion/trunk/subversion/svn/upgrade-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/upgrade-cmd.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/upgrade-cmd.c (original)
+++ subversion/trunk/subversion/svn/upgrade-cmd.c Thu Jun 10 19:49:22 2010
@@ -61,7 +61,7 @@ svn_cl__upgrade(apr_getopt_t *os,
     SVN_ERR(svn_cl__get_notifier(&ctx->notify_func2, &ctx->notify_baton2,
                                  FALSE, FALSE, FALSE, scratch_pool));
 
-  SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, scratch_pool));
+  SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
 
   iterpool = svn_pool_create(scratch_pool);
   for (i = 0; i < targets->nelts; i++)

Modified: subversion/trunk/subversion/svn/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/util.c?rev=953428&r1=953427&r2=953428&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/util.c (original)
+++ subversion/trunk/subversion/svn/util.c Thu Jun 10 19:49:22 2010
@@ -60,6 +60,7 @@
 #include "cl.h"
 
 #include "private/svn_token.h"
+#include "private/svn_opt_private.h"
 
 
 
@@ -1299,3 +1300,29 @@ svn_cl__path_join(const char *base,
   else
     return svn_dirent_join(base, component, pool);
 }
+
+svn_error_t *
+svn_cl__eat_peg_revisions(apr_array_header_t **true_targets_p,
+                          const apr_array_header_t *targets,
+                          apr_pool_t *pool)
+{
+  int i;
+  apr_array_header_t *true_targets;
+
+  true_targets = apr_array_make(pool, targets->nelts, sizeof(const char *));
+
+  for (i = 0; i < targets->nelts; i++)
+    {
+      const char *target = APR_ARRAY_IDX(targets, i, const char *);
+      const char *true_target;
+
+      SVN_ERR(svn_opt__split_arg_at_peg_revision(&true_target, NULL,
+                                                 target, pool));
+      APR_ARRAY_PUSH(true_targets, const char *) = true_target;
+    }
+
+  SVN_ERR_ASSERT(true_targets_p);
+  *true_targets_p = true_targets;
+
+  return SVN_NO_ERROR;
+}



Re: [RFC] Compatibility - 1.6 svn with 1.7 libs

Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@wandisco.com> writes:

> Ah, you added that svn_opt__eat_peg_revisions() forwarding function
> specifically to support this?  Now I understand where you're coming
> from.

You may not have realised, but svn_opt__eat_peg_revisions didn't have
a visible prototype outside the library source file.  Nothing could
compile against it (well not without a warning or creating a
prototype), it just existed to satisfy the depenency at run time.

> OK, perhaps we'll re-add this.  And/or make it a non-library function in
> 1.6.(>=12) so that at least we'll be able to test *some* versions of
> 1.6.x.
>
> Any preference?

No.

-- 
Philip

Re: [RFC] Compatibility - 1.6 svn with 1.7 libs

Posted by Julian Foad <ju...@wandisco.com>.
On Fri, 2010-06-11 at 11:19 +0100, Philip Martin wrote:
> Greg Stein <gs...@gmail.com> writes:
> 
> > svn_wc_private.h has seen massive changes in 1.7. A 1.6 libsvn_client
> > will not function against 1.7 libraries at all.
> 
> It depends what you mean by "at all".
> 
> svn up up -r953427 ../src
> ./config.nice --disable-full-version-match
> make -j3
> for i in subversion/libsvn_*/.libs/libsvn_*so.0.0.0;do cp $i ../obj-1.6/$i;done
> cd ../obj-1.6
> 
> Now I have a 1.6 client with 1.7 libraries.  It works well enough to
> checkout and commit.
> 
> rm subversion/libsvn_client/libsvn_client-1.la
> make libsvn_client
> 
> Now I have a 1.6 svn client and 1.6 libsvn_client with all other 1.7
> libraries.  It works well enough to do a checkout but wc locks get
> left requiring cleanup to be used.
> 
> Commit fails with an abspath assertion.  That could mean that we are
> missing an abspath conversion and so allowing a 1.6 non-abspath into
> our 1.7 libraries.  We need to test for that sort of thing before
> realeasing 1.7.  How will we do that if we can't even run our own
> client?
> 
> > We said a while back that the svn executables and libraries should be
> > upgraded/downgraded/patched as a group. These internal dependencies
> > will (thus) remain in sync across the link units.
> 
> I'd certainly agree that we only support upgrading all the libraries
> together.  I like to allow the libraries to be upgraded without the
> client since that is what we expect other people to do, although I
> don't think we should support it as such.
> 
> > The fact is: we are sloppy with versioning guidelines on private APIs.
> > Third-party code using our public APIs receive all of our strict
> > guideline benefits.
> >
> > Cheers,
> > -g
> >
> > On Fri, Jun 11, 2010 at 05:26, Julian Foad <ju...@btopenworld.com> wrote:
> >> A question on removing a private API.
> >>
> >> In v1.6.5 (r878898) we added the API "svn_opt__eat_peg_revisions()" and
> >> called it from the command-line client.  This means 1.6.(>=5) 'svn' is
> >> not compatible with 1.6.(<5) libraries.
> >>
> >> What we should have done is to make it a private function within the
> >> client code, not in the libraries.  But that's history and we can live
> >> with it.
> >>
> >> In r953428 on trunk I have moved the function out of the libraries into
> >> the client code.[*]  (We are allowed to introduce new library functions
> >> on trunk, of course, but we don't want to keep this particular function
> >> in the API long term.)
> >>
> >> A consequence is that we will not be able to run svn 1.6.(>=5)
> >> executables against 1.7.x libraries.  It would be useful to do so for
> >> testing, of course.
> >>
> >> The questions are:
> >>
> >> How well can we test 1.6.x (or older) 'svn' against newer libraries
> >> anyway?  Are there other private-API changes that make it impossible
> >> without special compatibility code insertions?  Has anyone tried?
> 
> I fixed two problems some time ago: svn_opt__eat_peg_revisions that
> allows the 1.6 client to use the 1.7 libraries and one other that
> allows 1.6 libsvn_client to use the 1.7 libraries.

Ah, you added that svn_opt__eat_peg_revisions() forwarding function
specifically to support this?  Now I understand where you're coming
from.

OK, perhaps we'll re-add this.  And/or make it a non-library function in
1.6.(>=12) so that at least we'll be able to test *some* versions of
1.6.x.

Any preference?  (My preference is something like the latter, because I
really odn't want us to keep that particular function: I want to get rid
of even the non-library version of it soon.)

- Julian


> >>
> >> Do we want to put this private API back into the 1.7 libraries for
> >> compatibility testing purposes
> 
> Yes.
> 
> >> or for any other reason?
> 


Re: [RFC] Compatibility - 1.6 svn with 1.7 libs

Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:

> svn_wc_private.h has seen massive changes in 1.7. A 1.6 libsvn_client
> will not function against 1.7 libraries at all.

It depends what you mean by "at all".

svn up up -r953427 ../src
./config.nice --disable-full-version-match
make -j3
for i in subversion/libsvn_*/.libs/libsvn_*so.0.0.0;do cp $i ../obj-1.6/$i;done
cd ../obj-1.6

Now I have a 1.6 client with 1.7 libraries.  It works well enough to
checkout and commit.

rm subversion/libsvn_client/libsvn_client-1.la
make libsvn_client

Now I have a 1.6 svn client and 1.6 libsvn_client with all other 1.7
libraries.  It works well enough to do a checkout but wc locks get
left requiring cleanup to be used.

Commit fails with an abspath assertion.  That could mean that we are
missing an abspath conversion and so allowing a 1.6 non-abspath into
our 1.7 libraries.  We need to test for that sort of thing before
realeasing 1.7.  How will we do that if we can't even run our own
client?

> We said a while back that the svn executables and libraries should be
> upgraded/downgraded/patched as a group. These internal dependencies
> will (thus) remain in sync across the link units.

I'd certainly agree that we only support upgrading all the libraries
together.  I like to allow the libraries to be upgraded without the
client since that is what we expect other people to do, although I
don't think we should support it as such.

> The fact is: we are sloppy with versioning guidelines on private APIs.
> Third-party code using our public APIs receive all of our strict
> guideline benefits.
>
> Cheers,
> -g
>
> On Fri, Jun 11, 2010 at 05:26, Julian Foad <ju...@btopenworld.com> wrote:
>> A question on removing a private API.
>>
>> In v1.6.5 (r878898) we added the API "svn_opt__eat_peg_revisions()" and
>> called it from the command-line client.  This means 1.6.(>=5) 'svn' is
>> not compatible with 1.6.(<5) libraries.
>>
>> What we should have done is to make it a private function within the
>> client code, not in the libraries.  But that's history and we can live
>> with it.
>>
>> In r953428 on trunk I have moved the function out of the libraries into
>> the client code.[*]  (We are allowed to introduce new library functions
>> on trunk, of course, but we don't want to keep this particular function
>> in the API long term.)
>>
>> A consequence is that we will not be able to run svn 1.6.(>=5)
>> executables against 1.7.x libraries.  It would be useful to do so for
>> testing, of course.
>>
>> The questions are:
>>
>> How well can we test 1.6.x (or older) 'svn' against newer libraries
>> anyway?  Are there other private-API changes that make it impossible
>> without special compatibility code insertions?  Has anyone tried?

I fixed two problems some time ago: svn_opt__eat_peg_revisions that
allows the 1.6 client to use the 1.7 libraries and one other that
allows 1.6 libsvn_client to use the 1.7 libraries.

>>
>> Do we want to put this private API back into the 1.7 libraries for
>> compatibility testing purposes

Yes.

>> or for any other reason?

-- 
Philip

Re: [RFC] Compatibility - 1.6 svn with 1.7 libs [was: svn commit: r953428]

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Jun 11, 2010 at 05:54, Julian Foad <ju...@wandisco.com> wrote:
> Greg Stein wrote:
>> svn_wc_private.h has seen massive changes in 1.7. A 1.6 libsvn_client
>> will not function against 1.7 libraries at all.
>
> Yup, but just to clarify, here I'm not talking about mixing one version
> of libsvn_client versus the other libs, but rather the client executable
> as in "subversion/svn/*.c" versus "the set of libraries"...

Yes. And it might be a little easier because the svn cmdline doesn't
take strong advantage of these internal/private APIs. But the overall
logic still holds: we've revamped internal APIs that preclude
mix/match.

... hmm. Thinking on this, our inter-library dependency check allows
for "newer" libraries to pass the check. But that isn't true if some
private APIs are no longer available.

Ugh.

>> We said a while back that the svn executables and libraries should be
>> upgraded/downgraded/patched as a group. These internal dependencies
>> will (thus) remain in sync across the link units.
>
> ... but yes, that's what I was thinking, if I understand you right.
>
> The question of whether we want to try running svn 1.6 against libs 1.7,
> for our own interest, still remains.

It is *very* interesting. But... it's going to be hard. There were
some private entries-based functions that we've eliminated. Those
would need to be resurrected in some fashion.

I'd like to be able to try running 1.x against a 1.7 wc library (and
that impiies 1.7 of subr/diff/delta) to ensure that our
backwards-compat code is Good.

Cheers,
-g

Re: [RFC] Compatibility - 1.6 svn with 1.7 libs [was: svn commit: r953428]

Posted by Julian Foad <ju...@wandisco.com>.
Greg Stein wrote:
> svn_wc_private.h has seen massive changes in 1.7. A 1.6 libsvn_client
> will not function against 1.7 libraries at all.

Yup, but just to clarify, here I'm not talking about mixing one version
of libsvn_client versus the other libs, but rather the client executable
as in "subversion/svn/*.c" versus "the set of libraries"...

> We said a while back that the svn executables and libraries should be
> upgraded/downgraded/patched as a group. These internal dependencies
> will (thus) remain in sync across the link units.

... but yes, that's what I was thinking, if I understand you right.

The question of whether we want to try running svn 1.6 against libs 1.7,
for our own interest, still remains.

- Julian


> The fact is: we are sloppy with versioning guidelines on private APIs.
> Third-party code using our public APIs receive all of our strict
> guideline benefits.
> 
> Cheers,
> -g
> 
> On Fri, Jun 11, 2010 at 05:26, Julian Foad <ju...@btopenworld.com> wrote:
> > A question on removing a private API.
> >
> > In v1.6.5 (r878898) we added the API "svn_opt__eat_peg_revisions()" and
> > called it from the command-line client.  This means 1.6.(>=5) 'svn' is
> > not compatible with 1.6.(<5) libraries.
> >
> > What we should have done is to make it a private function within the
> > client code, not in the libraries.  But that's history and we can live
> > with it.
> >
> > In r953428 on trunk I have moved the function out of the libraries into
> > the client code.[*]  (We are allowed to introduce new library functions
> > on trunk, of course, but we don't want to keep this particular function
> > in the API long term.)
> >
> > A consequence is that we will not be able to run svn 1.6.(>=5)
> > executables against 1.7.x libraries.  It would be useful to do so for
> > testing, of course.
> >
> > The questions are:
> >
> > How well can we test 1.6.x (or older) 'svn' against newer libraries
> > anyway?  Are there other private-API changes that make it impossible
> > without special compatibility code insertions?  Has anyone tried?
> >
> > Do we want to put this private API back into the 1.7 libraries for
> > compatibility testing purposes or for any other reason?
> >
> > ([*] Note: Also in the same commit I removed the public name
> > svn_opt_eat_peg_revisions() but that is not a concern because it has not
> > been published in any way.)
> >
> > - Julian
> >
> >
> > On Fri, 2010-06-11, Philip Martin wrote:
> >> julianfoad@apache.org writes:
> >>
> >> > Author: julianfoad
> >> > Date: Thu Jun 10 19:49:22 2010
> >> > New Revision: 953428
> >>
> >> > * subversion/libsvn_subr/opt.c
> >> >   (svn_opt__eat_peg_revisions): Remove this wrapper, as compatibility for
> >> >     1.6 for svn executable is not required.
> >>
> >> Why is this not required?
> >>
> >> We abused the API guidelines by adding this to the 1.6 branch (it
> >> should have been added to the command line client code).  We can work
> >> around that by keeping the symbol available.
> >>
> >> Keeping it also helps with backward compatibility testing.  We have
> >> vastly more backward compatibility code in 1.7 compared to earlier
> >> releases.  Being able to run the 1.6 client with 1.7 libraries seems
> >> like a good idea to me.  How much do you think works at present?
> >>
> >
> >
> >


Re: [RFC] Compatibility - 1.6 svn with 1.7 libs [was: svn commit: r953428]

Posted by Greg Stein <gs...@gmail.com>.
svn_wc_private.h has seen massive changes in 1.7. A 1.6 libsvn_client
will not function against 1.7 libraries at all.

We said a while back that the svn executables and libraries should be
upgraded/downgraded/patched as a group. These internal dependencies
will (thus) remain in sync across the link units.

The fact is: we are sloppy with versioning guidelines on private APIs.
Third-party code using our public APIs receive all of our strict
guideline benefits.

Cheers,
-g

On Fri, Jun 11, 2010 at 05:26, Julian Foad <ju...@btopenworld.com> wrote:
> A question on removing a private API.
>
> In v1.6.5 (r878898) we added the API "svn_opt__eat_peg_revisions()" and
> called it from the command-line client.  This means 1.6.(>=5) 'svn' is
> not compatible with 1.6.(<5) libraries.
>
> What we should have done is to make it a private function within the
> client code, not in the libraries.  But that's history and we can live
> with it.
>
> In r953428 on trunk I have moved the function out of the libraries into
> the client code.[*]  (We are allowed to introduce new library functions
> on trunk, of course, but we don't want to keep this particular function
> in the API long term.)
>
> A consequence is that we will not be able to run svn 1.6.(>=5)
> executables against 1.7.x libraries.  It would be useful to do so for
> testing, of course.
>
> The questions are:
>
> How well can we test 1.6.x (or older) 'svn' against newer libraries
> anyway?  Are there other private-API changes that make it impossible
> without special compatibility code insertions?  Has anyone tried?
>
> Do we want to put this private API back into the 1.7 libraries for
> compatibility testing purposes or for any other reason?
>
> ([*] Note: Also in the same commit I removed the public name
> svn_opt_eat_peg_revisions() but that is not a concern because it has not
> been published in any way.)
>
> - Julian
>
>
> On Fri, 2010-06-11, Philip Martin wrote:
>> julianfoad@apache.org writes:
>>
>> > Author: julianfoad
>> > Date: Thu Jun 10 19:49:22 2010
>> > New Revision: 953428
>>
>> > * subversion/libsvn_subr/opt.c
>> >   (svn_opt__eat_peg_revisions): Remove this wrapper, as compatibility for
>> >     1.6 for svn executable is not required.
>>
>> Why is this not required?
>>
>> We abused the API guidelines by adding this to the 1.6 branch (it
>> should have been added to the command line client code).  We can work
>> around that by keeping the symbol available.
>>
>> Keeping it also helps with backward compatibility testing.  We have
>> vastly more backward compatibility code in 1.7 compared to earlier
>> releases.  Being able to run the 1.6 client with 1.7 libraries seems
>> like a good idea to me.  How much do you think works at present?
>>
>
>
>

[RFC] Compatibility - 1.6 svn with 1.7 libs [was: svn commit: r953428]

Posted by Julian Foad <ju...@btopenworld.com>.
A question on removing a private API.

In v1.6.5 (r878898) we added the API "svn_opt__eat_peg_revisions()" and
called it from the command-line client.  This means 1.6.(>=5) 'svn' is
not compatible with 1.6.(<5) libraries.

What we should have done is to make it a private function within the
client code, not in the libraries.  But that's history and we can live
with it. 

In r953428 on trunk I have moved the function out of the libraries into
the client code.[*]  (We are allowed to introduce new library functions
on trunk, of course, but we don't want to keep this particular function
in the API long term.)

A consequence is that we will not be able to run svn 1.6.(>=5)
executables against 1.7.x libraries.  It would be useful to do so for
testing, of course.

The questions are: 

How well can we test 1.6.x (or older) 'svn' against newer libraries
anyway?  Are there other private-API changes that make it impossible
without special compatibility code insertions?  Has anyone tried?

Do we want to put this private API back into the 1.7 libraries for
compatibility testing purposes or for any other reason?

([*] Note: Also in the same commit I removed the public name
svn_opt_eat_peg_revisions() but that is not a concern because it has not
been published in any way.)

- Julian


On Fri, 2010-06-11, Philip Martin wrote:
> julianfoad@apache.org writes:
> 
> > Author: julianfoad
> > Date: Thu Jun 10 19:49:22 2010
> > New Revision: 953428
> 
> > * subversion/libsvn_subr/opt.c
> >   (svn_opt__eat_peg_revisions): Remove this wrapper, as compatibility for
> >     1.6 for svn executable is not required.
> 
> Why is this not required?
> 
> We abused the API guidelines by adding this to the 1.6 branch (it
> should have been added to the command line client code).  We can work
> around that by keeping the symbol available.
> 
> Keeping it also helps with backward compatibility testing.  We have
> vastly more backward compatibility code in 1.7 compared to earlier
> releases.  Being able to run the 1.6 client with 1.7 libraries seems
> like a good idea to me.  How much do you think works at present?
> 


Re: svn commit: r953428 - in /subversion/trunk/subversion: include/ libsvn_subr/ svn/

Posted by Philip Martin <ph...@wandisco.com>.
julianfoad@apache.org writes:

> Author: julianfoad
> Date: Thu Jun 10 19:49:22 2010
> New Revision: 953428

> * subversion/libsvn_subr/opt.c
>   (svn_opt__eat_peg_revisions): Remove this wrapper, as compatibility for
>     1.6 for svn executable is not required.

Why is this not required?

We abused the API guidelines by adding this to the 1.6 branch (it
should have been added to the command line client code).  We can work
around that by keeping the symbol available.

Keeping it also helps with backward compatibility testing.  We have
vastly more backward compatibility code in 1.7 compared to earlier
releases.  Being able to run the 1.6 client with 1.7 libraries seems
like a good idea to me.  How much do you think works at present?

-- 
Philip