You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gb...@apache.org on 2013/11/06 21:05:20 UTC

svn commit: r1539448 - in /subversion/branches/invoke-diff-cmd-feature/subversion: include/ include/private/ libsvn_client/ libsvn_subr/ svn/ tests/cmdline/getopt_tests_data/

Author: gbg
Date: Wed Nov  6 20:05:20 2013
New Revision: 1539448

URL: http://svn.apache.org/r1539448
Log:
On the invoke-diff-cmd-feature branch: Fixed all omissions, errors and
typos found in review: http://mail-archives.apache.org/mod_mbox/subversion-dev/201311.mbox/%3C1383750720.1526.YahooMailNeo@web186106.mail.ir2.yahoo.com%3E

* subversion/include/private/svn_io_private.h

  (svn_io__create_custom_diff_cmd): New function declaration.

* subversion/include/svn_client.h

  (svn_client_diff6): Fix doc string.

  (svn_client_diff_peg6): Fix doc string.

  (svn_client_diff_peg5): Fix doc string.

* subversion/include/svn_io.h

  (__create_custom_diff_cmd): Remove function.

* subversion/libsvn_client/diff.c

  (diff_cmd_baton): Move declaration of invoke_diff_cmd to group with
    diff_cmd.

* subversion/libsvn_subr/io.c

  (diff_content_changed): Add missing space to help string.

  (svn_io__create_custom_diff_cmd): Rename function from
    __create_custom_diff_cmd.  Change internal logic to ensure that
    every present reserved keyword is expanded, including multiple
    occurences thereof in the same word.

  (svn_io_run_external_diff): Restore original function contents from
    /trunk.  Add location of matching test.

* subversion/svn/svn.c

  (svn_cl__options "invoke-diff-cmd"): Update and format help
    information.

* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout

  (--invoke-diff-cmd): Add new entry to help output data.


Modified:
    subversion/branches/invoke-diff-cmd-feature/subversion/include/private/svn_io_private.h
    subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_client.h
    subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h
    subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c
    subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c
    subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c
    subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/include/private/svn_io_private.h
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/include/private/svn_io_private.h?rev=1539448&r1=1539447&r2=1539448&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/include/private/svn_io_private.h (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/include/private/svn_io_private.h Wed Nov  6 20:05:20 2013
@@ -96,6 +96,51 @@ svn_stream__is_buffered(svn_stream_t *st
 apr_file_t *
 svn_stream__aprfile(svn_stream_t *stream);
 
+/** Parse a user defined command to contain dynamically created labels
+ *  and filenames.  This function serves both diff and diff3 parsing
+ *  requirements.
+ *
+ *  When used in a diff context: (responding parse tokens in braces)
+ *
+ *  @a label1 (%svn_label_old) refers to the label of @a tmpfile1
+ *  (%svn_old) which is the pristine copy.
+ *
+ *  @a label2 (%svn_label_new) refers to the label of @a tmpfile2
+ *  (%svn_new) which is the altered copy.
+ *
+ *  When used in a diff3 context:
+ *
+ *  @a label1 refers to the label of @a tmpfile1 which is the 'mine'
+ *  copy.
+ *
+ *  @a label2 refers to the label of @a tmpfile2 which is the 'older'
+ *  copy.
+ *
+ *  @a label3 (%svn_label_base) refers to the label of @a base
+ *  (%svn_base) which is the 'base' copy.
+ *
+ *  In general:
+ *
+ *  @a cmd is a user defined string containing 0 or more parse tokens
+ *  which are expanded by the required labels and filenames.
+ * 
+ *  @a pool is used for temporary allocations.
+ *
+ *  @return A NULL-terminated character array.
+ * 
+ * @since New in 1.9.
+ */
+const char **
+svn_io__create_custom_diff_cmd(const char *label1,
+                               const char *label2,
+                               const char *label3,
+                               const char *from,
+                               const char *to,
+                               const char *base,
+                               const char *cmd,
+                               apr_pool_t *pool);
+
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_client.h
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_client.h?rev=1539448&r1=1539447&r2=1539448&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_client.h (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_client.h Wed Nov  6 20:05:20 2013
@@ -3118,7 +3118,7 @@ svn_client_diff7(const apr_array_header_
                  svn_client_ctx_t *ctx,
                  apr_pool_t *pool);
 
-/** Similar to svn_client_diff7(), but with @a invoke_diff_cmd.
+/** Similar to svn_client_diff7(), but without @a invoke_diff_cmd.
  *
  * @deprecated Provided for backward compatibility with the 1.8 API.
  * @since New in 1.8.
@@ -3314,9 +3314,9 @@ svn_client_diff_peg7(const apr_array_hea
          
 
 /**
- * Similar to svn_client_peg7(), but with @a no_diff_added set to
- * FALSE, @a ignore_properties set to FALSE and @a properties_only
- * set to FALSE.
+ * Similar to svn_client_peg7(), but without @a no_diff_added set to
+ * FALSE, @a ignore_properties set to FALSE and @a properties_only set
+ * to FALSE.
  *
  * @deprecated Provided for backward compatibility with the 1.7 API.
  * @since New in 1.8.
@@ -3346,7 +3346,7 @@ svn_client_diff_peg6(const apr_array_hea
                      apr_pool_t *pool);
 
 /**
- * Similar to svn_client_diff6_peg6(), but with @a outfile and @a errfile,
+ * Similar to svn_client_diff_peg6(), but with @a outfile and @a errfile,
  * instead of @a outstream and @a errstream, and with @a
  * no_diff_added, @a ignore_properties, and @a properties_only always
  * passed as @c FALSE (which means that additions and property changes

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h?rev=1539448&r1=1539447&r2=1539448&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/include/svn_io.h Wed Nov  6 20:05:20 2013
@@ -2361,54 +2361,6 @@ svn_io_file_readline(apr_file_t *file,
 
 /** @} */
 
-/** Parse a user defined command to contain dynamically created labels
- *  and filenames.  This function serves both diff and diff3 parsing
- *  requirements.
- *
- *  When used in a diff context: (responding parse tokens in braces)
- *
- *  @a label1 (;l1) refers to the label of @a tmpfile1 (;f1) which is
- *  the pristine copy.
- *
- *  @a label2 (;l2) refers to the label of @a tmpfile2 (;f2) which
- *  is the altered copy.
- *
- *  When used in a diff3 context:
- *
- *  @a label1 refers to the label of @a tmpfile1 which is the 'mine'
- *  copy.
- *
- *  @a label2 refers to the label of @a tmpfile2 which is the 'older'
- *  copy.
- *
- *  @a label3 (;l3) refers to the label of @a base (;f3) which is
- *  the 'base' copy.
- *
- *  A parse token can be escaped by prefixing a ';'.  Any other
- *  strings containing ';' are not affected.
- *
- *  In general:
- *
- *  @a cmd is a user defined string containing 0 or more parse tokens
- *  which are expanded by the required labels and filenames.
- * 
- *  @a pool is used for temporary allocations.
- *
- *  @return A NULL-terminated character array.
- * 
- * @since New in 1.9.
- */
-const char **
-__create_custom_diff_cmd(const char *label1,
-                         const char *label2,
-                         const char *label3,
-                         const char *from,
-                         const char *to,
-                         const char *base,
-                         const char *cmd,
-                         apr_pool_t *pool);
-
-
 /** Run the external diff command defined by the invoke-diff-cmd
  *  option.
  *  

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c?rev=1539448&r1=1539447&r2=1539448&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_client/diff.c Wed Nov  6 20:05:20 2013
@@ -536,6 +536,9 @@ struct diff_cmd_baton {
   /* If non-null, the external diff command to invoke. */
   const char *diff_cmd;
 
+  /* external custom diff command */
+  const char *invoke_diff_cmd;
+
   /* This is allocated in this struct's pool or a higher-up pool. */
   union {
     /* If 'diff_cmd' is null, then this is the parsed options to
@@ -613,9 +616,6 @@ struct diff_cmd_baton {
   /* Whether the local diff target of a repos->wc diff is a copy. */
   svn_boolean_t repos_wc_diff_target_is_copy;
 
-  /* external custom diff command */
-  const char *invoke_diff_cmd;
-
 };
 
 /* An helper for diff_dir_props_changed, diff_file_changed and diff_file_added
@@ -792,7 +792,7 @@ diff_content_changed(svn_boolean_t *wrot
 
   if (diff_cmd_baton->diff_cmd && diff_cmd_baton->invoke_diff_cmd)
       return svn_error_create(SVN_ERR_CLIENT_DIFF_CMD, NULL,
-                              _("diff-cmd and invoke-diff-cmd are"
+                              _("diff-cmd and invoke-diff-cmd are "
                                 "mutually exclusive."));
 
   if (diff_cmd_baton->diff_cmd || diff_cmd_baton->invoke_diff_cmd)

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c?rev=1539448&r1=1539447&r2=1539448&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/libsvn_subr/io.c Wed Nov  6 20:05:20 2013
@@ -2994,14 +2994,14 @@ svn_io_run_cmd(const char *path,
 }
 
 const char **
-__create_custom_diff_cmd(const char *label1,
-                         const char *label2,
-                         const char *label3,
-                         const char *from,
-                         const char *to,
-                         const char *base,
-                         const char *cmd,
-                         apr_pool_t *pool)
+svn_io__create_custom_diff_cmd(const char *label1,
+                               const char *label2,
+                               const char *label3,
+                               const char *from,
+                               const char *to,
+                               const char *base,
+                               const char *cmd,
+                               apr_pool_t *pool)
 {
   /* 
      This function can be tested with:
@@ -3070,19 +3070,21 @@ __create_custom_diff_cmd(const char *lab
                 }
             }
         }
-
-      for (i = 0; i < delimiters; i++)
+      i = 0;
+      while (i < delimiters)
         {
           char *found = strstr(word->data, tokens_tab[i].delimiter);
 
           if (!found)
-            continue;
+            {
+              i++;
+              continue;
+            }
             
           svn_stringbuf_replace(word, found - word->data,
                                 strlen(tokens_tab[i].delimiter),
                                 tokens_tab[i].replace,
                                 strlen(tokens_tab[i].replace));
-          i = delimiters;
         }
       result[argv] = apr_pstrdup(pool,word->data);
     }  
@@ -3111,7 +3113,7 @@ svn_io_run_external_diff(const char *dir
   if (0 == strlen(external_diff_cmd)) 
      return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL, NULL);
 
-  cmd = __create_custom_diff_cmd(label1, label2, NULL, 
+  cmd = svn_io__create_custom_diff_cmd(label1, label2, NULL, 
                                  tmpfile1, tmpfile2, NULL, 
                                  external_diff_cmd, scratch_pool);
   if (pexitcode == NULL)
@@ -3156,8 +3158,8 @@ svn_io_run_diff2(const char *dir,
                  int num_user_args,
                  const char *label1,
                  const char *label2,
-                 const char *old,
-                 const char *new,
+                 const char *from,
+                 const char *to,
                  int *pexitcode,
                  apr_file_t *outfile,
                  apr_file_t *errfile,
@@ -3168,41 +3170,79 @@ svn_io_run_diff2(const char *dir,
   This function can be tested by using the test 
   /subversion/tests/cmdline/diff_tests.py diff_external_diffcmd
   */
-  svn_stringbuf_t *com;
-  com = svn_stringbuf_create_empty(pool);
-  svn_stringbuf_appendcstr(com, diff_cmd);
-  svn_stringbuf_appendcstr(com, " ");
+
+  const char **args;
+  int i;
+  int exitcode;
+  int nargs = 4; /* the diff command itself, two paths, plus a trailing NULL */
+  apr_pool_t *subpool = svn_pool_create(pool);
+
+  if (pexitcode == NULL)
+    pexitcode = &exitcode;
+
+  if (user_args != NULL)
+    nargs += num_user_args;
+  else
+    nargs += 1; /* -u */
+
+  if (label1 != NULL)
+    nargs += 2; /* the -L and the label itself */
+  if (label2 != NULL)
+    nargs += 2; /* the -L and the label itself */
+
+  args = apr_palloc(subpool, nargs * sizeof(char *));
+
+  i = 0;
+  args[i++] = diff_cmd;
 
   if (user_args != NULL)
     {
       int j;
-      for (j = 0; j < num_user_args; ++j) 
-        {
-          svn_stringbuf_appendcstr(com, user_args[j]);
-          svn_stringbuf_appendcstr(com, " ");
-        }
+      for (j = 0; j < num_user_args; ++j)
+        args[i++] = user_args[j];
     }
-  else /* assume -u if the user didn't give us any args */
-    svn_stringbuf_appendcstr(com, "-u "); 
+  else
+    args[i++] = "-u"; /* assume -u if the user didn't give us any args */
 
   if (label1 != NULL)
-    svn_stringbuf_appendcstr(com,"-L %svn_label_old ");
-
+    {
+      args[i++] = "-L";
+      args[i++] = label1;
+    }
   if (label2 != NULL)
-    svn_stringbuf_appendcstr(com,"-L %svn_label_new ");
+    {
+      args[i++] = "-L";
+      args[i++] = label2;
+    }
 
-  svn_stringbuf_appendcstr(com,"%svn_old %svn_new"); 
+  args[i++] = svn_dirent_local_style(from, subpool);
+  args[i++] = svn_dirent_local_style(to, subpool);
+  args[i++] = NULL;
 
-  return svn_io_run_external_diff(dir,
-                                  label1,
-                                  label2,
-                                  old,
-                                  new,
-                                  pexitcode,
-                                  outfile,
-                                  errfile,
-                                  com->data,
-                                  pool);
+  SVN_ERR_ASSERT(i == nargs);
+
+  SVN_ERR(svn_io_run_cmd(dir, diff_cmd, args, pexitcode, NULL, TRUE,
+                         NULL, outfile, errfile, subpool));
+
+  /* The man page for (GNU) diff describes the return value as:
+
+       "An exit status of 0 means no differences were found, 1 means
+        some differences were found, and 2 means trouble."
+
+     A return value of 2 typically occurs when diff cannot read its input
+     or write to its output, but in any case we probably ought to return an
+     error for anything other than 0 or 1 as the output is likely to be
+     corrupt.
+   */
+  if (*pexitcode != 0 && *pexitcode != 1)
+    return svn_error_createf(SVN_ERR_EXTERNAL_PROGRAM, NULL,
+                             _("'%s' returned %d"),
+                             svn_dirent_local_style(diff_cmd, pool),
+                             *pexitcode);
+
+  svn_pool_destroy(subpool);
+
+  return SVN_NO_ERROR;
 }
 
 svn_error_t *

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c?rev=1539448&r1=1539447&r2=1539448&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/svn/svn.c Wed Nov  6 20:05:20 2013
@@ -346,32 +346,33 @@ const apr_getopt_option_t svn_cl__option
   /* diff options */
   {"diff-cmd",      opt_diff_cmd, 1, N_("use ARG as diff command")},
   {"invoke-diff-cmd", opt_invoke_diff_cmd, 1, 
-                   N_("use ARG as format string for external diff command     \n"
+                   N_("use ARG as format string for external diff command\n"
                       "                             "
-                      "invocation. \n                                         \n" 
+                      "invocation.\n" 
                       "                             "
-                      "Substitutions: %svn_new new file                       \n"
+                      "The following reserved keywords are replaced:\n"
                       "                             "
-                      "               %svn_old old file                       \n"
+                      "    %svn_new -- new file\n"
                       "                             "
-                      "               %svn_label_new  label of the new file   \n"
+                      "    %svn_old -- old file\n"
                       "                             "
-                      "               %svn_label_old  label of the old file   \n"
+                      "    %svn_label_new -- label of the new file\n"
                       "                             "
-                      "Examples:                                              \n"
+                      "    %svn_label_old -- label of the old file\n"
                       "                             "
-                      "--invoke-diff-cmd=\'diff -y %svn_new %svn_old\'        \n"          
+                      "Examples:\n"
                       "                             "
-                      "--invoke-diff-cmd=\"kdiff3 -auto -o /home/u/log \\     \n"
+                      "--invoke-diff-cmd=\'diff -y %svn_new %svn_old\'\n"          
                       "                             "
-                      "      %svn_new %svn_old --L1 %svn_label_new \\         \n"
+                      "--invoke-diff-cmd=\"kdiff3 -auto -o /home/u/log \\\n"
                       "                             "
-                      "     --L2 \"Custom Label\" \'                          \n"
+                      "      %svn_new %svn_old --L1 %svn_label_new \\\n"
                       "                             "
-                      "Substitution variables may be embedded in strings:     \n"
+                      "     --L2 \"Custom Label\" \'\n"
                       "                             "
-                      "+%svn_new, %svn_new- and file=%svn_label_new+          \n"
-     )},
+                      "Reserved keywords may be embedded in strings:\n"
+                      "                             "
+                      "+%svn_new  %svn_new- and file=%svn_label_new+")},
   {"internal-diff", opt_internal_diff, 0,
                        N_("override diff-cmd specified in config file")},
   {"no-diff-added", opt_no_diff_added, 0,

Modified: subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
URL: http://svn.apache.org/viewvc/subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout?rev=1539448&r1=1539447&r2=1539448&view=diff
==============================================================================
--- subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout (original)
+++ subversion/branches/invoke-diff-cmd-feature/subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout Wed Nov  6 20:05:20 2013
@@ -111,6 +111,20 @@ Valid options:
                                -w, --ignore-all-space: Ignore all white space
                                --ignore-eol-style: Ignore changes in EOL style
                                -p, --show-c-function: Show C function name
+  --invoke-diff-cmd ARG    : use ARG as format string for external diff command
+                             invocation.
+                             The following reserved keywords are replaced:
+                                 %svn_new -- new file
+                                 %svn_old -- old file
+                                 %svn_label_new -- label of the new file
+                                 %svn_label_old -- label of the old file
+                             Examples:
+                             --invoke-diff-cmd='diff -y %svn_new %svn_old'
+                             --invoke-diff-cmd="kdiff3 -auto -o /home/u/log \
+                                   %svn_new %svn_old --L1 %svn_label_new \
+                                  --L2 "Custom Label" '
+                             Reserved keywords may be embedded in strings:
+                             +%svn_new  %svn_new- and file=%svn_label_new+
   --search ARG             : use ARG as search pattern (glob syntax)
   --search-and ARG         : combine ARG with the previous search pattern