On Wed, Nov 03, 2010 at 03:32:25PM +0530, Noorul Islam K M wrote:
>
> Stefan Sperling <[email protected]> writes:
> >
> > Noorul, if you're looking for more tasks, you could take a look at
> > issue #3620 which is about a similar problem:
> > http://subversion.tigris.org/issues/show_bug.cgi?id=3620
> > I'll happily review and commit patches for issue #3620.
> >
> > Thanks,
> > Stefan
>
> Here is the patch for revert command. I will look into other commands as
> and when time permits.
>
> Log
>
> [[[
> Make 'svn revert' verify that none of its targets are URLs.
>
> * subversion/libsvn_client/revert.c,
> subversion/svn/revert-cmd.c
> (svn_client_revert2, svn_cl__revert): Raise an error if any targets
> look like URLs.
>
> * subversion/tests/cmdline/input_validation_tests.py
> (invalid_revert_targets, test_list): New test.
>
> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
> ]]]
>
> Thanks and Regards
> Noorul
>
> Index: subversion/tests/cmdline/input_validation_tests.py
> ===================================================================
> --- subversion/tests/cmdline/input_validation_tests.py (revision
> 1030340)
> +++ subversion/tests/cmdline/input_validation_tests.py (working copy)
> @@ -173,6 +173,12 @@
> run_and_verify_svn_in_wc(sbox, "svn:.*is not a local path", 'upgrade',
> target, target)
>
> +def invalid_revert_targets(sbox):
> + "non-working copy paths for 'revert'"
> + sbox.build(read_only=True)
> + for target in _invalid_wc_path_targets:
> + run_and_verify_svn_in_wc(sbox, "svn:.*is not a local path", 'revert',
> + target)
>
> ########################################################################
> # Run the tests
> @@ -192,6 +198,7 @@
> invalid_log_targets,
> invalid_merge_args,
> invalid_wcpath_upgrade,
> + invalid_revert_targets,
> ]
>
> if __name__ == '__main__':
> Index: subversion/svn/revert-cmd.c
> ===================================================================
> --- subversion/svn/revert-cmd.c (revision 1030340)
> +++ subversion/svn/revert-cmd.c (working copy)
> @@ -27,6 +27,7 @@
>
> /*** Includes. ***/
>
> +#include "svn_path.h"
> #include "svn_client.h"
> #include "svn_error_codes.h"
> #include "svn_error.h"
> @@ -47,6 +48,7 @@
> svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
> apr_array_header_t *targets = NULL;
> svn_error_t *err;
> + int i;
>
> SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
> opt_state->targets,
> @@ -63,6 +65,19 @@
>
> SVN_ERR(svn_cl__eat_peg_revisions(&targets, targets, scratch_pool));
>
> + /* Don't even attempt to modify the working copy if any of the
> + * targets look like URLs. URLs are invalid input. */
> + for (i = 0; i < targets->nelts; i++)
> + {
> + const char *target = APR_ARRAY_IDX(targets, i, const char *);
> +
> + if (svn_path_is_url(target))
> + return
> svn_error_return(svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR,
> + NULL,
> + _("'%s' is not a local
> path"),
> + target));
> + }
> +
> err = svn_client_revert2(targets, opt_state->depth,
> opt_state->changelists, ctx, scratch_pool);
>
> Index: subversion/libsvn_client/revert.c
> ===================================================================
> --- subversion/libsvn_client/revert.c (revision 1030340)
> +++ subversion/libsvn_client/revert.c (working copy)
> @@ -27,6 +27,7 @@
>
> /*** Includes. ***/
>
> +#include "svn_path.h"
> #include "svn_wc.h"
> #include "svn_client.h"
> #include "svn_dirent_uri.h"
> @@ -37,6 +38,7 @@
> #include "client.h"
> #include "private/svn_wc_private.h"
>
> +#include "svn_private_config.h"
>
>
> /*** Code. ***/
> @@ -121,6 +123,19 @@
> svn_boolean_t use_commit_times;
> struct revert_with_write_lock_baton baton;
>
> + /* Don't even attempt to modify the working copy if any of the
> + * targets look like URLs. URLs are invalid input. */
> + for (i = 0; i < paths->nelts; i++)
> + {
> + const char *path = APR_ARRAY_IDX(paths, i, const char *);
> +
> + if (svn_path_is_url(path))
> + return
> svn_error_return(svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR,
The SVN_ERR_CL_* codes are for use by the command line client, I think.
So I've used SVN_ERR_ILLEGAL_TARGET for errors thrown from libsvn_client.
But I can fix the error code locally, you don't need to send another patch.
Looks great otherwise. I will test and quite likely commit this later today.
Thanks,
Stefan
> + NULL,
> + _("'%s' is not a local
> path"),
> + path));
> + }
> +
> cfg = ctx->config ? apr_hash_get(ctx->config, SVN_CONFIG_CATEGORY_CONFIG,
> APR_HASH_KEY_STRING) : NULL;
>
