I can't seem to reach tigris.org right now, so I'm posting this
preliminary patch here, in case anyone wants to review. I'm still
testing it ('make check' takes a while), and there may be one
refactorization in the test suite I want to do before committing.
[[[
Fix issue #2285: Non-commit operations should error if given a log message.
### Not ready for commit yet, still being tested. ###
* subversion/include/svn_error_codes.h
(SVN_ERR_CL_UNNECESSARY_LOG_MESSAGE): New error code.
* subversion/libsvn_client/client.h, subversion/libsvn_client/log.c
(svn_client__ensure_no_log_message): New helper function.
* subversion/libsvn_client/copy.c
(wc_to_wc_copy, repos_to_wc_copy): Call new helper function.
* subversion/libsvn_client/delete.c
(svn_client_delete): Call new helper function.
* subversion/libsvn_client/add.c
(svn_client_mkdir): Call new helper function.
* subversion/tests/clients/cmdline/commit_tests.py
(local_mods_are_not_commits): New test.
(test_list): Run it.
* subversion/tests/clients/cmdline/svntest/actions.py
(match_or_fail): New helper, does regexp matching.
(display_lines): New parameter 'expected_is_regexp', conditionally
adjust display accordingly.
(run_and_verify_svn): Treat string type in 'expected' parameter as a
regexp to match.
]]]
Index: subversion/include/svn_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h (revision 14483)
+++ subversion/include/svn_error_codes.h (working copy)
@@ -1008,6 +1008,10 @@
SVN_ERR_CL_CATEGORY_START + 8,
"Something is wrong with the log message's contents")
+ SVN_ERRDEF (SVN_ERR_CL_UNNECESSARY_LOG_MESSAGE,
+ SVN_ERR_CL_CATEGORY_START + 9,
+ "A log message was given where none was necessary")
+
SVN_ERROR_END
Index: subversion/libsvn_client/delete.c
===================================================================
--- subversion/libsvn_client/delete.c (revision 14483)
+++ subversion/libsvn_client/delete.c (working copy)
@@ -260,6 +260,8 @@
svn_pool_clear (subpool);
parent_path = svn_path_dirname (path, subpool);
+ SVN_ERR (svn_client__ensure_no_log_message (ctx, pool));
+
/* See if the user wants us to stop. */
if (ctx->cancel_func)
SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 14483)
+++ subversion/libsvn_client/client.h (working copy)
@@ -176,6 +176,13 @@
apr_pool_t *pool);
+/* Try to extract a log message from CTX, and if succeed, return
+ SVN_ERR_CL_UNNECESSARY_LOG_MESSAGE. Use POOL for temporary
+ allocation only. */
+svn_error_t *
+svn_client__ensure_no_log_message (svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
/* ---------------------------------------------------------------- */
/*** RA callbacks ***/
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 14483)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -74,6 +74,8 @@
svn_wc_adm_access_t *adm_access, *src_access;
svn_error_t *err;
+ SVN_ERR (svn_client__ensure_no_log_message (ctx, pool));
+
/* Verify that SRC_PATH exists. */
SVN_ERR (svn_io_check_path (src_path, &src_kind, pool));
if (src_kind == svn_node_none)
@@ -760,6 +762,8 @@
svn_boolean_t same_repositories;
svn_opt_revision_t revision;
+ SVN_ERR (svn_client__ensure_no_log_message (ctx, pool));
+
/* Open a repository session to the given URL. We do not (yet) have a
working copy, so we don't have a corresponding path and tempfiles
cannot go into the admin area. */
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 14483)
+++ subversion/libsvn_client/log.c (working copy)
@@ -37,9 +37,30 @@
#include "svn_private_config.h"
-/*** Getting update information ***/
+/*** Helper used by commands other than log. ***/
+svn_error_t *
+svn_client__ensure_no_log_message (svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ if (ctx->log_msg_func)
+ {
+ const char *message, *tmp_file;
+ apr_array_header_t *commit_items_decoy =
+ apr_array_make (pool, 0, sizeof (svn_client_commit_item_t *));
+ SVN_ERR ((*ctx->log_msg_func) (&message, &tmp_file, commit_items_decoy,
+ ctx->log_msg_baton, pool));
+ if (tmp_file || (message && (message[0] != '\0')))
+ return svn_error_createf
+ (SVN_ERR_CL_UNNECESSARY_LOG_MESSAGE, NULL,
+ _("Local, non-commit operations do not take a log message"));
+ }
+
+ return SVN_NO_ERROR;
+}
+
+
/*** Public Interface. ***/
Index: subversion/libsvn_client/add.c
===================================================================
--- subversion/libsvn_client/add.c (revision 14483)
+++ subversion/libsvn_client/add.c (working copy)
@@ -622,6 +622,8 @@
svn_error_t *err;
int i;
+ SVN_ERR (svn_client__ensure_no_log_message (ctx, pool));
+
for (i = 0; i < paths->nelts; i++)
{
const char *path = APR_ARRAY_IDX (paths, i, const char *);
Index: subversion/tests/clients/cmdline/commit_tests.py
===================================================================
--- subversion/tests/clients/cmdline/commit_tests.py (revision 14483)
+++ subversion/tests/clients/cmdline/commit_tests.py (working copy)
@@ -1902,7 +1902,59 @@
source_url, tab_url)
match_bad_tab_path(tab_dir, errlines)
+#----------------------------------------------------------------------
+def local_mods_are_not_commits(sbox):
+ "local ops should not be treated like commits"
+
+ # For issue #2285.
+ #
+ # Some commands can run on either a URL or a local path. These
+ # commands take a log message, intended for the URL case.
+ # Therefore, they should make sure that getting a log message for
+ # a local operation errors (because not committing).
+ #
+ # This is in commit_tests.py because the unifying theme is that
+ # commits are *not* happening. And because there was no better
+ # place to put it :-).
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+ expected_error = '.*Local, non-commit operations do not take a log message.*'
+
+ # copy wc->wc
+ svntest.actions.run_and_verify_svn(None, None, expected_error,
+ 'cp', '-m', 'log msg',
+ os.path.join(wc_dir, 'iota'),
+ os.path.join(wc_dir, 'iota2'))
+
+ # copy repos->wc
+ svntest.actions.run_and_verify_svn(None, None, expected_error,
+ 'cp', '-m', 'log msg',
+ '--username',
+ svntest.main.wc_author,
+ '--password',
+ svntest.main.wc_passwd,
+ svntest.main.current_repo_url + "/iota",
+ os.path.join(wc_dir, 'iota2'))
+
+ # delete
+ svntest.actions.run_and_verify_svn(None, None, expected_error,
+ 'rm', '-m', 'log msg',
+ os.path.join(wc_dir, 'A', 'D', 'gamma'))
+
+ # mkdir
+ svntest.actions.run_and_verify_svn(None, None, expected_error,
+ 'mkdir', '-m', 'log msg',
+ os.path.join(wc_dir, 'newdir'))
+
+ # rename
+ svntest.actions.run_and_verify_svn(None, None, expected_error,
+ 'cp', '-m', 'log msg',
+ os.path.join(wc_dir, 'A', 'mu'),
+ os.path.join(wc_dir, 'A', 'yu'))
+
+
########################################################################
# Run the tests
@@ -1941,6 +1993,7 @@
from_wc_top_with_bad_editor,
mods_in_schedule_delete,
Skip(tab_test, (os.name != 'posix' or sys.platform == 'cygwin')),
+ local_mods_are_not_commits,
]
if __name__ == '__main__':
Index: subversion/tests/clients/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/clients/cmdline/svntest/actions.py (revision 14483)
+++ subversion/tests/clients/cmdline/svntest/actions.py (working copy)
@@ -171,6 +171,8 @@
if type(expected_stdout) is type([]):
compare_and_display_lines(message, 'STDOUT', expected_stdout, out)
+ elif type(expected_stdout) is type(''):
+ match_or_fail(message, 'STDOUT', expected_stdout, out)
elif expected_stdout == SVNAnyOutput:
if len(out) == 0:
if message is not None: print message
@@ -180,6 +182,8 @@
if type(expected_stderr) is type([]):
compare_and_display_lines(message, 'STDERR', expected_stderr, err)
+ elif type(expected_stderr) is type(''):
+ match_or_fail(message, 'STDERR', expected_stderr, err)
elif expected_stderr == SVNAnyOutput:
if len(err) == 0:
if message is not None: print message
@@ -698,13 +702,18 @@
tree.dump_tree(actual)
-def display_lines(message, label, expected, actual):
+def display_lines(message, label, expected, actual, expected_is_regexp=None):
'Print two sets of output lines, expected and actual.'
if message is not None:
print message
if expected is not None:
- print 'EXPECTED', label + ':'
+ if expected_is_regexp:
+ print 'EXPECTED', label + ' (regexp):'
+ else:
+ print 'EXPECTED', label + ':'
map(sys.stdout.write, expected)
+ if expected_is_regexp:
+ map(sys.stdout.write, '\n')
if actual is not None:
print 'ACTUAL', label + ':'
map(sys.stdout.write, actual)
@@ -721,6 +730,17 @@
display_lines(message, label, expected, actual)
raise main.SVNLineUnequal
+def match_or_fail(message, label, expected, actual):
+ """Make sure that regexp EXPECTED matches at least one line in list ACTUAL.
+ If no match, then print MESSAGE (if it's not None), followed by
+ EXPECTED and ACTUAL, both labeled with LABEL, and raise SVNLineUnequal."""
+ matched = None
+ for line in actual:
+ if re.match(expected, line):
+ matched = 1
+ if not matched:
+ display_lines(message, label, expected, actual, 1)
+ raise main.SVNLineUnequal
######################################################################
# Other general utilities
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 28 01:20:34 2005