On Wed, 2010-11-03 at 14:46 +0100, Stefan Sperling wrote:
> 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.
-0 on duplicating this check inside libsvn_client.
- Julian
> 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;
> >
>
