[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r37988 - in trunk/subversion: include include/private libsvn_subr svn tests/cmdline

From: Bert Huijben <rhuijben_at_sharpsvn.net>
Date: Thu, 11 Jun 2009 17:00:50 +0200

> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: donderdag 11 juni 2009 16:46
> To: svn_at_subversion.tigris.org
> Subject: svn commit: r37988 - in trunk/subversion: include include/private
> libsvn_subr svn tests/cmdline
>
> Author: stsp
> Date: Thu Jun 11 07:45:47 2009
> New Revision: 37988
>
> Log:
> Fix issue #3416, "Cannot add or commit 'dir/@file.txt'".
>
> Make every subcommand parse peg revisions which previously didn't
> parse them. The Book documents this behaviour as follows:
>
> After all, how does svn know whether news_at_11 is the name of a
> directory in my tree or just a syntax for “revision 11 of news”?
> Thankfully, while svn will always assume the latter, [...]
>
> Regrettably, svn didn't always assume the latter.
>
> We only ever parsed peg revisions for commands where they make sense.
> Escaping '@' characters in filenames was impossible for subcommands
> which didn't parse peg revisions. Subversion would always interpret
> 'abc_at_abc' as a complete filename for those commands, instead of
> as the nonsense 'revision abc of abc' it is.
>
> Even our unit tests didn't agree with The Book, and were merrily
> adding and committing 'abc_at_abc' without escaping.
>
> To add insult to injury, when splitting peg revisions from target
> paths we never ensured that the target path still had a non-zero
> length after the split. The code then went on to form a canonicalised
> target path with attached peg revision by canonicalising the empty
> path and then concatenating the empty canonicalisation result and
> the peg revision.
>
> This led to confusing behaviour were 'svn add @file' ('@file' is an
> actual filename) was accidentally working, but 'svn add @file@' was
> not -- it resulted in "svn add: Warning: '@file@' not found"
>
> In the former situation we now throw an error, because as far as svn
> can tell only a peg revision was specified, but no target path.
> 'svn add @file@' is the correct way to add a file called @file, and
> because 'svn add' will now parse and discard the peg revision specifier,
> this now works.
>
> This problem didn't just affect add, but every subcommand which
> didn't interpret peg revisions (commit, lock, etc.)
>
> * subversion/libsvn_subr/opt.c
> (svn_opt__split_arg_at_peg_revision): Require target path to have
> a non-zero length after splitting a peg revision from it.
> Allow callers to pass NULL for the parsed peg revision, so they can
> easily discard the parsed peg revision if they don't need it.
> (svn_opt_eat_peg_revisions): New function. Returns a copy of a list
> of targets with all peg revision specifiers removed.
>
> * subversion/tests/cmdline/basic_tests.py
> (basic_peg_revision): Consistently escape '@' characters in filenames,
> and check for regression into issue #3416.
>
> * subversion/svn/patch-cmd.c, subversion/svn/propdel-cmd.c,
> subversion/svn/mkdir-cmd.c, subversion/svn/move-cmd.c,
> subversion/svn/revert-cmd.c, subversion/svn/changelist-cmd.c,
> subversion/svn/update-cmd.c, subversion/svn/resolved-cmd.c,
> subversion/svn/upgrade-cmd.c, subversion/svn/cleanup-cmd.c,
> subversion/svn/commit-cmd.c, subversion/svn/add-cmd.c,
> subversion/svn/propset-cmd.c, subversion/svn/delete-cmd.c,
> subversion/svn/resolve-cmd.c, subversion/svn/status-cmd.c,
> subversion/svn/propedit-cmd.c, subversion/svn/lock-cmd.c,
> subversion/svn/unlock-cmd.c
> (svn_cl__patch, svn_cl__propdel, svn_cl__mkdir, svn_cl__move,
> svn_cl__revert, svn_cl__changelist, svn_cl__update, svn_cl__resolved,
> svn_cl__upgrade, svn_cl__cleanup, svn_cl__commit, svn_cl__add,
> svn_cl__propset, svn_cl__delete, svn_cl__resolve, svn_cl__status,
> svn_cl__propedit, svn_cl__lock, svn_cl__unlock): Remove peg revision
> specifiers from all targets before passing them to the client library,
> using the new function svn_opt_eat_peg_revisions().
>
> * subversion/include/private/svn_opt_private.h
> (svn_opt__split_arg_at_peg_revision): Document changes in behaviour
> described above (see entry for subversion/libsvn_subr/opt.c).
>
> * subversion/include/svn_opt.h
> (svn_opt_parse_path): Document a few additional examples of how peg
> revisions are parsed, all pertaining to issue #3416.
>
> Modified:
> trunk/subversion/include/private/svn_opt_private.h
> trunk/subversion/include/svn_opt.h
> trunk/subversion/libsvn_subr/opt.c
> trunk/subversion/svn/add-cmd.c
> trunk/subversion/svn/changelist-cmd.c
> trunk/subversion/svn/cleanup-cmd.c
> trunk/subversion/svn/commit-cmd.c
> trunk/subversion/svn/delete-cmd.c
> trunk/subversion/svn/lock-cmd.c
> trunk/subversion/svn/mkdir-cmd.c
> trunk/subversion/svn/move-cmd.c
> trunk/subversion/svn/patch-cmd.c
> trunk/subversion/svn/propdel-cmd.c
> trunk/subversion/svn/propedit-cmd.c
> trunk/subversion/svn/propset-cmd.c
> trunk/subversion/svn/resolve-cmd.c
> trunk/subversion/svn/resolved-cmd.c
> trunk/subversion/svn/revert-cmd.c
> trunk/subversion/svn/status-cmd.c
> trunk/subversion/svn/unlock-cmd.c
> trunk/subversion/svn/update-cmd.c
> trunk/subversion/svn/upgrade-cmd.c
> trunk/subversion/tests/cmdline/basic_tests.py
>
> Modified: trunk/subversion/include/private/svn_opt_private.h
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/include/private/svn_opt_priv
> ate.h?pathrev=37988&r1=37987&r2=37988
> ==============================================================================
> --- trunk/subversion/include/private/svn_opt_private.h Thu Jun 11 05:58:46
> 2009 (r37987)
> +++ trunk/subversion/include/private/svn_opt_private.h Thu Jun 11 07:45:47
> 2009 (r37988)
> @@ -32,15 +32,23 @@
> extern "C" {
> #endif /* __cplusplus */
>
> -/* Extract the peg revision, if any, from UTF8_TARGET. Return the peg
> - * revision in *PEG_REVISION and the true target portion in *TRUE_TARGET.
> +/* Extract the peg revision, if any, from UTF8_TARGET.
> + *
> + * If PEG_REVISION is not NULL, return the peg revision in *PEG_REVISION.
> * *PEG_REVISION will be an empty string if no peg revision is found.
> + * Return the true target portion in *TRUE_TARGET.
> *
> * UTF8_TARGET need not be canonical. *TRUE_TARGET will not be canonical
> * unless UTF8_TARGET is.
> *
> + * It is an error if *TRUE_TARGET results in the empty string after the
> + * split, which happens in case UTF8_TARGET has a leading '@' character
> + * with no additional '@' characters to escape the first '@'.
> + *
> * Note that *PEG_REVISION will still contain the '@' symbol as the first
> - * character if a peg revision was found.
> + * character if a peg revision was found. If a trailing '@' symbol was
> + * used to escape other '@' characters in UTF8_TARGET, *PEG_REVISION will
> + * point to the string "@", containing only a single character.
> *
> * All allocations are done in POOL.
> */
>
> Modified: trunk/subversion/include/svn_opt.h
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_opt.h?pathrev=37
> 988&r1=37987&r2=37988
> ==============================================================================
> --- trunk/subversion/include/svn_opt.h Thu Jun 11 05:58:46 2009 (r37987)
> +++ trunk/subversion/include/svn_opt.h Thu Jun 11 07:45:47 2009 (r37988)
> @@ -621,11 +621,14 @@ svn_opt_parse_all_args(apr_array_header_
> * "foo/bar_at_1:2" -> error
> * "foo/bar_at_baz" -> error
> * "foo/bar@" -> "foo/bar", (base)
> + * "foo/@bar@" -> "foo/@bar", (base)
> * "foo/bar/@13" -> "foo/bar/", (number, 13)
> * "foo/bar@@13" -> "foo/bar@", (number, 13)
> * "foo/@bar_at_HEAD" -> "foo/@bar", (head)
> * "foo@/bar" -> "foo@/bar", (unspecified)
> * "foo_at_HEAD/bar" -> "foo_at_HEAD/bar", (unspecified)
> + * "@foo/bar" -> error
> + * "@foo/bar@" -> "@foo/bar", (unspecified)
> *
> * [*] Syntactically valid but probably not semantically useful.
> *
> @@ -734,6 +737,27 @@ svn_opt_print_help(apr_getopt_t *os,
> const char *footer,
> apr_pool_t *pool);
>
> +/* Return, in @a *true_targets_p, a copy of @a targets with peg revision
> + * specifiers snipped off the end of each element.
> + *
> + * This function is useful for subcommands for which peg revisions
> + * do not make any sense. Such subcommands still need to allow peg
> + * revisions to be specified on the command line so that users of
> + * the command line client can consistently escape '@' characters
> + * in filenames by appending an '@' character, regardless of the
> + * subcommand being used.
> + *
> + * If a peg revision is present but cannot be parsed, an error is thrown.
> + * The user has likely forgotten to escape an '@' character in a filename.
> + *
> + * It is safe to pass the address of @a targets as @a true_targets_p.
> + *
> + * Do all allocations in @a pool. */
> +svn_error_t *
> +svn_opt_eat_peg_revisions(apr_array_header_t **true_targets_p,
> + apr_array_header_t *targets,
> + apr_pool_t *pool);
> +

This new function needs a @since doxygen comment.

> #ifdef __cplusplus
> }
> #endif /* __cplusplus */

        Bert

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2361316
Received on 2009-06-11 17:01:20 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.