On 2010-07-20 20:23, s...@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__': > >
signature.asc
Description: OpenPGP digital signature
