You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Neels J Hofmeyr <ne...@elego.de> on 2010/07/20 23:55:25 UTC
Re: svn commit: r965943 - in /subversion/trunk/subversion: svn/commit-cmd.c
tests/cmdline/input_validation_tests.py
On 2010-07-20 20:23, stsp@apache.org wrote:
> Author: stsp
> Date: Tue Jul 20 18:23:11 2010
> New Revision: 965943
>
> URL: http://svn.apache.org/viewvc?rev=965943&view=rev
> Log:
> Turns out that 'svn commit' is already pretty good at rejecting URL targets.
> Add a test for it anyway and do some nitpicking:
>
> * subversion/svn/commit-cmd.c
> (svn_cl__commit): For consistency with other commands, use
> SVN_ERR_CL_ARG_PARSING_ERROR when rejecting URL targets.
> Also, use the same error message as svn_client_commit4(), and mark
> the error message for translation.
>
> * subversion/tests/cmdline/input_validation_tests.py
> (invalid_wcpath_commit, test_list): New test.
What about commit_tests.py 59 (from r870124, which you broke)?
I fixed it but we could remove it from commit_tests completely. The
advantage of keeping both is that depending on the ra method used,
commit_test 59 also uses svn:// and http:// URLs (_invalid_wc_path_targets
in input_validation_tests only has 'file:///' and '^/').
I tried to find something that is not a URL and still an invalid wc_path, to
tickle your error message to say " 'non-URL' is a URL ... ". I didn't manage
but I found this unrelated curiosity:
(in a WC of subversion/tests/cmdline)
$ svn ci not/a/path
subversion/svn/commit-cmd.c:140: (apr_err=155004)
subversion/libsvn_client/commit.c:1150: (apr_err=155004)
subversion/libsvn_client/commit.c:1003: (apr_err=155004)
svn: Are all targets part of the same working copy?
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
$ svn st -v | grep "^ L"
AND
$ svn ci no/path
subversion/svn/commit-cmd.c:140: (apr_err=155004)
subversion/libsvn_client/commit.c:1150: (apr_err=155004)
subversion/libsvn_client/commit.c:1003: (apr_err=155004)
svn: Are all targets part of the same working copy?
svn: run 'svn cleanup' to remove locks (type 'svn help cleanup' for details)
$ svn st -v | grep "^ L"
L 966056 966048 neels .
L 966056 859568 lundblad update_tests_data
L 966056 965912 neels svntest
L 966056 939329 pburba svndumpfilter_tests_data
L 966056 864101 dlr special_tests_data
L 966056 938999 pburba svnadmin_tests_data
L 966056 959807 rhuijben upgrade_tests_data
L 966056 965930 stsp svnsync_tests_data
L 966056 965531 stsp getopt_tests_data
L 966056 873704 danielsh log_tests_data
Pretty weird stuff. Why it even locks those dirs in the first place, where
clearly none of the paths are addressed by the target... some odd WC-1
behaviour? Maybe I'll get around to look at it tomorrow.
Good night!
~Neels
>
> Modified:
> subversion/trunk/subversion/svn/commit-cmd.c
> subversion/trunk/subversion/tests/cmdline/input_validation_tests.py
>
> Modified: subversion/trunk/subversion/svn/commit-cmd.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/commit-cmd.c?rev=965943&r1=965942&r2=965943&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svn/commit-cmd.c (original)
> +++ subversion/trunk/subversion/svn/commit-cmd.c Tue Jul 20 18:23:11 2010
> @@ -38,6 +38,7 @@
> #include "svn_config.h"
> #include "cl.h"
>
> +#include "svn_private_config.h"
>
>
> /* This implements the `svn_opt_subcommand_t' interface. */
> @@ -66,9 +67,10 @@ svn_cl__commit(apr_getopt_t *os,
> {
> const char *target = APR_ARRAY_IDX(targets, i, const char *);
> if (svn_path_is_url(target))
> - return svn_error_create(SVN_ERR_WC_BAD_PATH, NULL,
> - "Must give local path (not URL) as the "
> - "target of a commit");
> + return svn_error_return(
> + svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("'%s' is a URL, but URLs cannot be "
> + "commit targets"), target));
> }
>
> /* Add "." if user passed 0 arguments. */
>
> Modified: subversion/trunk/subversion/tests/cmdline/input_validation_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/input_validation_tests.py?rev=965943&r1=965942&r2=965943&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/input_validation_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/input_validation_tests.py Tue Jul 20 18:23:11 2010
> @@ -83,6 +83,13 @@ def invalid_wcpath_cleanup(sbox):
> run_and_verify_svn_in_wc(sbox, "svn:.*is not a local path", 'cleanup',
> target)
>
> +def invalid_wcpath_commit(sbox):
> + "non-working copy paths for 'commit'"
> + sbox.build()
> + for target in _invalid_wc_path_targets:
> + run_and_verify_svn_in_wc(sbox, "svn: '.*' is a URL, but URLs cannot be " +
> + "commit targets", 'commit', target)
> +
> ########################################################################
> # Run the tests
>
> @@ -91,6 +98,7 @@ test_list = [ None,
> invalid_wcpath_add,
> invalid_wcpath_changelist,
> invalid_wcpath_cleanup,
> + invalid_wcpath_commit,
> ]
>
> if __name__ == '__main__':
>
>