You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2012/12/21 21:57:39 UTC

svn commit: r1425140 - in /subversion/trunk/subversion: svnmucc/svnmucc.c tests/cmdline/svnmucc_tests.py tests/cmdline/svntest/main.py

Author: cmpilato
Date: Fri Dec 21 20:57:38 2012
New Revision: 1425140

URL: http://svn.apache.org/viewvc?rev=1425140&view=rev
Log:
Fix issue #3418 ("svnmucc doesn't prompt for log message"), teach
svnmucc to consult $EDITOR et al for a log message when interactive
and when no other log message has been provided.

This doesn't quite have the full functionality offered in 'svn'
itself, but should be a sane enough starting point.

* subversion/svnmucc/svnmucc.c
  (execute): Use svn_cmdline__edit_string_externally() to prompt for a
    log message if none has been provided.
  (mutually_exclusive_logs_error, sanitize_log_sources): New helper
    functions.
  (main): Use sanitize_log_sources() to ensure that the user hasn't
    tried to provide a log message via more than one of the three
    possible ways to do so (and to get that information into the
    revprops hash).

* subversion/tests/cmdline/svntest/main.py
  (_with_log_message): Remove as unused.
  (run_svnmucc): Don't use the _with_log_message() wrapper any longer.

* subversion/tests/cmdline/svntest/svnmucc_tests.py
  (reject_bogus_mergeinfo, propset_root_internal): Pass a log message
    to 'svnmucc'.
  (basic_svnmucc): Pass log message commandline args to test_svnmucc()
    and xtest_svnmucc().
  (no_log_msg_non_interactive): New tests.
  (test_list): Add references to new tests.

Modified:
    subversion/trunk/subversion/svnmucc/svnmucc.c
    subversion/trunk/subversion/tests/cmdline/svnmucc_tests.py
    subversion/trunk/subversion/tests/cmdline/svntest/main.py

Modified: subversion/trunk/subversion/svnmucc/svnmucc.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnmucc/svnmucc.c?rev=1425140&r1=1425139&r2=1425140&view=diff
==============================================================================
--- subversion/trunk/subversion/svnmucc/svnmucc.c (original)
+++ subversion/trunk/subversion/svnmucc/svnmucc.c Fri Dec 21 20:57:38 2012
@@ -55,6 +55,7 @@
 
 #include "private/svn_cmdline_private.h"
 #include "private/svn_ra_private.h"
+#include "private/svn_string_private.h"
 
 #include "svn_private_config.h"
 
@@ -752,6 +753,29 @@ execute(const apr_array_header_t *action
                                             "svnmucc: ", "--config-option"));
   cfg_config = apr_hash_get(config, SVN_CONFIG_CATEGORY_CONFIG,
                             APR_HASH_KEY_STRING);
+
+  if (! apr_hash_get(revprops, SVN_PROP_REVISION_LOG, APR_HASH_KEY_STRING))
+    {
+      svn_string_t *msg = svn_string_create("", pool);
+
+      /* If we can do so, try to pop up $EDITOR to fetch a log message. */
+      if (non_interactive)
+        {
+          return svn_error_create
+            (SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
+             _("Cannot invoke editor to get log message "
+               "when non-interactive"));
+        }
+      else
+        {
+          SVN_ERR(svn_cmdline__edit_string_externally(
+                      &msg, NULL, NULL, "", msg, "svnmucc-commit", config,
+                      TRUE, NULL, apr_hash_pool_get(revprops)));
+        }
+
+      apr_hash_set(revprops, SVN_PROP_REVISION_LOG, APR_HASH_KEY_STRING, msg);
+    }
+
   SVN_ERR(create_ra_callbacks(&ra_callbacks, username, password, config_dir,
                               cfg_config, non_interactive, no_auth_cache,
                               pool));
@@ -966,6 +990,53 @@ display_version(apr_getopt_t *os, apr_po
   return SVN_NO_ERROR;
 }
 
+/* Return an error about the mutual exclusivity of the -m, -F, and
+   --with-revprop=svn:log command-line options. */
+static svn_error_t *
+mutually_exclusive_logs_error(void)
+{
+  return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                          _("--message (-m), --file (-F), and "
+                            "--with-revprop=svn:log are mutually "
+                            "exclusive"));
+}
+
+/* Ensure that the REVPROPS hash contains a command-line-provided log
+   message, if any, and that there was but one source of such a thing
+   provided on that command-line.  */
+static svn_error_t *
+sanitize_log_sources(apr_hash_t *revprops,
+                     const char *message,
+                     svn_stringbuf_t *filedata)
+{
+  apr_pool_t *hash_pool = apr_hash_pool_get(revprops);
+
+  /* If we already have a log message in the revprop hash, then just
+     make sure the user didn't try to also use -m or -F.  Otherwise,
+     we need to consult -m or -F to find a log message, if any. */
+  if (apr_hash_get(revprops, SVN_PROP_REVISION_LOG, APR_HASH_KEY_STRING))
+    {
+      if (filedata || message)
+        return mutually_exclusive_logs_error();
+    }
+  else if (filedata)
+    {
+      if (message)
+        return mutually_exclusive_logs_error();
+
+      SVN_ERR(svn_utf_cstring_to_utf8(&message, filedata->data, hash_pool));
+      apr_hash_set(revprops, SVN_PROP_REVISION_LOG, APR_HASH_KEY_STRING,
+                   svn_stringbuf__morph_into_string(filedata));
+    }
+  else if (message)
+    {
+      apr_hash_set(revprops, SVN_PROP_REVISION_LOG, APR_HASH_KEY_STRING,
+                   svn_string_create(message, hash_pool));
+    }
+  
+  return SVN_NO_ERROR;
+}
+
 int
 main(int argc, const char **argv)
 {
@@ -1001,6 +1072,7 @@ main(int argc, const char **argv)
     {NULL, 0, 0, NULL}
   };
   const char *message = NULL;
+  svn_stringbuf_t *filedata = NULL;
   const char *username = NULL, *password = NULL;
   const char *root_url = NULL, *extra_args_file = NULL;
   const char *config_dir = NULL;
@@ -1038,12 +1110,9 @@ main(int argc, const char **argv)
         case 'F':
           {
             const char *arg_utf8;
-            svn_stringbuf_t *contents;
             err = svn_utf_cstring_to_utf8(&arg_utf8, arg, pool);
             if (! err)
-              err = svn_stringbuf_from_file2(&contents, arg, pool);
-            if (! err)
-              err = svn_utf_cstring_to_utf8(&message, contents->data, pool);
+              err = svn_stringbuf_from_file2(&filedata, arg, pool);
             if (err)
               handle_error(err, pool);
           }
@@ -1116,6 +1185,11 @@ main(int argc, const char **argv)
         }
     }
 
+  /* Make sure we have a log message to use. */
+  err = sanitize_log_sources(revprops, message, filedata);
+  if (err)
+    handle_error(err, pool);
+
   /* Copy the rest of our command-line arguments to an array,
      UTF-8-ing them along the way. */
   action_args = apr_array_make(pool, opts->argc, sizeof(const char *));
@@ -1331,21 +1405,6 @@ main(int argc, const char **argv)
   if (! actions->nelts)
     usage(pool, EXIT_FAILURE);
 
-  if (message == NULL)
-    {
-      if (apr_hash_get(revprops, SVN_PROP_REVISION_LOG,
-                       APR_HASH_KEY_STRING) == NULL)
-        /* None of -F, -m, or --with-revprop=svn:log specified; default. */
-        apr_hash_set(revprops, SVN_PROP_REVISION_LOG, APR_HASH_KEY_STRING,
-                     svn_string_create("committed using svnmucc", pool));
-    }
-  else
-    {
-      /* -F or -m specified; use that even if --with-revprop=svn:log. */
-      apr_hash_set(revprops, SVN_PROP_REVISION_LOG, APR_HASH_KEY_STRING,
-                   svn_string_create(message, pool));
-    }
-
   if ((err = execute(actions, anchor, revprops, username, password,
                      config_dir, config_options, non_interactive,
                      no_auth_cache, base_revision, pool)))

Modified: subversion/trunk/subversion/tests/cmdline/svnmucc_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnmucc_tests.py?rev=1425140&r1=1425139&r2=1425140&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnmucc_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnmucc_tests.py Fri Dec 21 20:57:38 2012
@@ -45,6 +45,7 @@ def reject_bogus_mergeinfo(sbox):
   # validate the mergeinfo up front then it will only test the client
   svntest.actions.run_and_verify_svnmucc(None, [], expected_error,
                                          'propset', 'svn:mergeinfo', '/B:0',
+                                         '-m', 'log msg',
                                          sbox.repo_url + '/A')
 
 _svnmucc_re = re.compile('^(r[0-9]+) committed by jrandom at (.*)$')
@@ -111,12 +112,14 @@ def basic_svnmucc(sbox):
   test_svnmucc(sbox.repo_url,
                ['A /foo'
                 ], # ---------
+               '-m', 'log msg',
                'mkdir', 'foo')
 
   # revision 3
   test_svnmucc(sbox.repo_url,
                ['A /z.c',
                 ], # ---------
+               '-m', 'log msg',
                'put', empty_file, 'z.c')
 
   # revision 4
@@ -124,6 +127,7 @@ def basic_svnmucc(sbox):
                ['A /foo/z.c (from /z.c:3)',
                 'A /foo/bar (from /foo:3)',
                 ], # ---------
+               '-m', 'log msg',
                'cp', '3', 'z.c', 'foo/z.c',
                'cp', '3', 'foo', 'foo/bar')
 
@@ -134,6 +138,7 @@ def basic_svnmucc(sbox):
                 'D /foo',
                 'A /zig/zag (from /foo:4)',
                 ], # ---------
+               '-m', 'log msg',
                'cp', '4', 'foo', 'zig',
                'rm',             'zig/bar',
                'mv',      'foo', 'zig/zag')
@@ -144,6 +149,7 @@ def basic_svnmucc(sbox):
                 'A /zig/zag/bar/y.c (from /z.c:5)',
                 'A /zig/zag/bar/x.c (from /z.c:3)',
                 ], # ---------
+               '-m', 'log msg',
                'mv',      'z.c', 'zig/zag/bar/y.c',
                'cp', '3', 'z.c', 'zig/zag/bar/x.c')
 
@@ -153,6 +159,7 @@ def basic_svnmucc(sbox):
                 'A /zig/zag/bar/y y.c (from /zig/zag/bar/y.c:6)',
                 'A /zig/zag/bar/y%20y.c (from /zig/zag/bar/y.c:6)',
                 ], # ---------
+               '-m', 'log msg',
                'mv',         'zig/zag/bar/y.c', 'zig/zag/bar/y%20y.c',
                'cp', 'HEAD', 'zig/zag/bar/y.c', 'zig/zag/bar/y%2520y.c')
 
@@ -163,6 +170,7 @@ def basic_svnmucc(sbox):
                 'A /zig/zag/bar/z%20z.c (from /zig/zag/bar/y%20y.c:7)',
                 'A /zig/zag/bar/z z2.c (from /zig/zag/bar/y y.c:7)',
                 ], #---------
+               '-m', 'log msg',
                'mv',         'zig/zag/bar/y%20y.c',   'zig/zag/bar/z z1.c',
                'cp', 'HEAD', 'zig/zag/bar/y%2520y.c', 'zig/zag/bar/z%2520z.c',
                'cp', 'HEAD', 'zig/zag/bar/y y.c',     'zig/zag/bar/z z2.c')
@@ -176,6 +184,7 @@ def basic_svnmucc(sbox):
                 'D /zig/foo/bar/z z2.c',
                 'R /zig/foo/bar/z z1.c (from /zig/zag/bar/x.c:6)',
                 ], #---------
+               '-m', 'log msg',
                'mv',      'zig/zag',         'zig/foo',
                'rm',                         'zig/foo/bar/z z1.c',
                'rm',                         'zig/foo/bar/z%20z2.c',
@@ -186,6 +195,7 @@ def basic_svnmucc(sbox):
   test_svnmucc(sbox.repo_url,
                ['R /zig/foo/bar (from /zig/z.c:9)',
                 ], #---------
+               '-m', 'log msg',
                'rm',                 'zig/foo/bar',
                'cp', '9', 'zig/z.c', 'zig/foo/bar')
 
@@ -194,6 +204,7 @@ def basic_svnmucc(sbox):
                ['R /zig/foo/bar (from /zig/foo/bar:9)',
                 'D /zig/foo/bar/z z1.c',
                 ], #---------
+               '-m', 'log msg',
                'rm',                     'zig/foo/bar',
                'cp', '9', 'zig/foo/bar', 'zig/foo/bar',
                'rm',                     'zig/foo/bar/z%20z1.c')
@@ -202,6 +213,7 @@ def basic_svnmucc(sbox):
   test_svnmucc(sbox.repo_url,
                ['R /zig/foo (from /zig/foo/bar:11)',
                 ], #---------
+               '-m', 'log msg',
                'rm',                        'zig/foo',
                'cp', 'head', 'zig/foo/bar', 'zig/foo')
 
@@ -214,6 +226,7 @@ def basic_svnmucc(sbox):
                 'D /foo/foo/bar',
                 'R /foo/foo/foo/bar (from /foo:4)',
                 ], #---------
+               '-m', 'log msg',
                'rm',             'zig',
                'cp', '4', 'foo', 'foo',
                'cp', '4', 'foo', 'foo/foo',
@@ -228,6 +241,7 @@ def basic_svnmucc(sbox):
                 'A /boozle/buz',
                 'A /boozle/buz/nuz',
                 ], #---------
+               '-m', 'log msg',
                'cp',    '4', 'foo', 'boozle',
                'mkdir',             'boozle/buz',
                'mkdir',             'boozle/buz/nuz')
@@ -238,6 +252,7 @@ def basic_svnmucc(sbox):
                 'A /boozle/guz (from /boozle/buz:14)',
                 'A /boozle/guz/svnmucc-test.py',
                 ], #---------
+               '-m', 'log msg',
                'put',      empty_file,   'boozle/buz/svnmucc-test.py',
                'cp', '14', 'boozle/buz', 'boozle/guz',
                'put',      empty_file,   'boozle/guz/svnmucc-test.py')
@@ -247,20 +262,25 @@ def basic_svnmucc(sbox):
                ['M /boozle/buz/svnmucc-test.py',
                 'R /boozle/guz/svnmucc-test.py',
                 ], #---------
+               '-m', 'log msg',
                'put', empty_file, 'boozle/buz/svnmucc-test.py',
                'rm',              'boozle/guz/svnmucc-test.py',
                'put', empty_file, 'boozle/guz/svnmucc-test.py')
 
   # revision 17
   test_svnmucc(sbox.repo_url,
-               ['R /foo/bar (from /foo/foo:16)'], #---------
+               ['R /foo/bar (from /foo/foo:16)'
+                ], #---------
+               '-m', 'log msg',
                'rm',                            'foo/bar',
                'cp', '16', 'foo/foo',           'foo/bar',
                'propset',  'testprop',  'true', 'foo/bar')
 
   # revision 18
   test_svnmucc(sbox.repo_url,
-               ['M /foo/bar'], #---------
+               ['M /foo/bar'
+                ], #---------
+               '-m', 'log msg',
                'propdel', 'testprop', 'foo/bar')
 
   # revision 19
@@ -268,6 +288,7 @@ def basic_svnmucc(sbox):
                ['M /foo/z.c',
                 'M /foo/foo',
                 ], #---------
+               '-m', 'log msg',
                'propset', 'testprop', 'true', 'foo/z.c',
                'propset', 'testprop', 'true', 'foo/foo')
 
@@ -276,6 +297,7 @@ def basic_svnmucc(sbox):
                ['M /foo/z.c',
                 'M /foo/foo',
                 ], #---------
+               '-m', 'log msg',
                'propsetf', 'testprop', empty_file, 'foo/z.c',
                'propsetf', 'testprop', empty_file, 'foo/foo')
 
@@ -283,6 +305,7 @@ def basic_svnmucc(sbox):
   xtest_svnmucc(sbox.repo_url,
                 ["svnmucc: E200004: 'a' is not a revision"
                  ], #---------
+                '-m', 'log msg',
                 'cp', 'a', 'b')
 
   # Expected cannot be younger error
@@ -290,18 +313,21 @@ def basic_svnmucc(sbox):
                 ['svnmucc: E205000: Copy source revision cannot be younger ' +
                  'than base revision',
                  ], #---------
+                '-m', 'log msg',
                 'cp', '42', 'a', 'b')
 
   # Expected already exists error
   xtest_svnmucc(sbox.repo_url,
                 ["svnmucc: E125002: 'foo' already exists",
                  ], #---------
+                '-m', 'log msg',
                 'cp', '17', 'a', 'foo')
 
   # Expected copy_src already exists error
   xtest_svnmucc(sbox.repo_url,
                 ["svnmucc: E125002: 'a/bar' (from 'foo/bar:17') already exists",
                  ], #---------
+                '-m', 'log msg',
                 'cp', '17', 'foo', 'a',
                 'cp', '17', 'foo/foo', 'a/bar')
 
@@ -309,12 +335,14 @@ def basic_svnmucc(sbox):
   xtest_svnmucc(sbox.repo_url,
                 ["svnmucc: E125002: 'a' not found",
                  ], #---------
+                '-m', 'log msg',
                 'cp', '17', 'a', 'b')
 
 
 def propset_root_internal(sbox, target):
   ## propset on ^/
   svntest.actions.run_and_verify_svnmucc(None, None, [],
+                                         '-m', 'log msg',
                                          'propset', 'foo', 'bar',
                                          target)
   svntest.actions.run_and_verify_svn(None, 'bar', [],
@@ -323,6 +351,7 @@ def propset_root_internal(sbox, target):
 
   ## propdel on ^/
   svntest.actions.run_and_verify_svnmucc(None, None, [],
+                                         '-m', 'log msg',
                                          'propdel', 'foo',
                                          target)
   svntest.actions.run_and_verify_svn(None, [], [],
@@ -338,12 +367,58 @@ def propset_root(sbox):
   propset_root_internal(sbox, sbox.repo_url + '/iota')
 
 
+def too_many_log_messages(sbox):
+  "test log message mutual exclusivity checks"
+
+  sbox.build() # would use read-only=True, but need a place to stuff msg_file
+  msg_file = sbox.ospath('svnmucc_msg')
+  svntest.main.file_append(msg_file, 'some log message')
+  err_msg = ["svnmucc: E205000: --message (-m), --file (-F), and "
+             "--with-revprop=svn:log are mutually exclusive"]
+             
+  xtest_svnmucc(sbox.repo_url, err_msg,
+                '--non-interactive',
+                '-m', 'log msg',
+                '-F', msg_file,
+                'mkdir', 'A/subdir')
+  xtest_svnmucc(sbox.repo_url, err_msg,
+                '--non-interactive',
+                '-m', 'log msg',
+                '--with-revprop', 'svn:log=proppy log message',
+                'mkdir', 'A/subdir')
+  xtest_svnmucc(sbox.repo_url, err_msg,
+                '--non-interactive',
+                '-F', msg_file,
+                '--with-revprop', 'svn:log=proppy log message',
+                'mkdir', 'A/subdir')
+  xtest_svnmucc(sbox.repo_url, err_msg,
+                '--non-interactive',
+                '-m', 'log msg',
+                '-F', msg_file,
+                '--with-revprop', 'svn:log=proppy log message',
+                'mkdir', 'A/subdir')
+  
+@Issues(3418)
+def no_log_msg_non_interactive(sbox):
+  "test non-interactive without a log message"
+  
+  sbox.build(create_wc=False)
+  xtest_svnmucc(sbox.repo_url,
+                ["svnmucc: E205001: Cannot invoke editor to get log message "
+                 "when non-interactive"
+                 ], #---------
+                '--non-interactive',
+                'mkdir', 'A/subdir')
+  
+
 ######################################################################
 
 test_list = [ None,
               reject_bogus_mergeinfo,
               basic_svnmucc,
               propset_root,
+              too_many_log_messages,
+              no_log_msg_non_interactive,
             ]
 
 if __name__ == '__main__':

Modified: subversion/trunk/subversion/tests/cmdline/svntest/main.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svntest/main.py?rev=1425140&r1=1425139&r2=1425140&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svntest/main.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Fri Dec 21 20:57:38 2012
@@ -651,13 +651,6 @@ def _with_auth(args):
   else:
     return args + ('--username', wc_author )
 
-def _with_log_message(args):
-
-  if '-m' in args or '--message' in args or '-F' in args:
-    return args
-  else:
-    return args + ('--message', 'default log message')
-
 # For running subversion and returning the output
 def run_svn(error_expected, *varargs):
   """Run svn with VARARGS; return exit code as int; stdout, stderr as
@@ -705,7 +698,7 @@ def run_svnmucc(*varargs):
   """Run svnmucc with VARARGS, returns exit code as int; stdout, stderr as
   list of lines (including line terminators).  Use binary mode for output."""
   return run_command(svnmucc_binary, 1, 1,
-                     *(_with_auth(_with_config_dir(_with_log_message(varargs)))))
+                     *(_with_auth(_with_config_dir(varargs))))
 
 def run_entriesdump(path):
   """Run the entries-dump helper, returning a dict of Entry objects."""