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

Eating peg revisions [was: [PATCH] Fix issue #3651 - "svn copy does not eat peg revision within copy target path"]

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Thu, 10 Jun 2010 13:57:16 +0100

On Wed, 2010-06-09 at 17:05 +0530, Senthil Kumaran S wrote:
> Hi,
>
> I am attaching a patch to fix issue #3651, just to check whether this is what
> stsp intended to say via
> http://subversion.tigris.org/issues/show_bug.cgi?id=3651#desc2
>
> Thank You.
> plain text document attachment (3651.patch.txt)
> [[[
> Fix issue #3651.

Hi Senthil.

Please could you quote the issue's Subject when you quote its issue
number in a log message or in an email subject line. That would make
life easier for me. Thanks.

> * subversion/svn/copy-cmd.c
> (svn_cl__copy): Eat peg revisions in copy target paths.
> ]]]
>
> Index: subversion/svn/copy-cmd.c
> ===================================================================
> --- subversion/svn/copy-cmd.c (revision 952908)
> +++ subversion/svn/copy-cmd.c (working copy)
> @@ -77,6 +77,8 @@
> APR_ARRAY_PUSH(sources, svn_client_copy_source_t *) = source;
> }
>
> + SVN_ERR(svn_opt_eat_peg_revisions(&targets, targets, pool));
> +

This fixes one missed case of the fix that was applied for all other
commands in r878062 for issue #3416 "Cannot add or commit
'dir/@file.txt'".

This reminds me that there is more to do.

Use of svn_opt_eat_peg_revisions() does indeed fix the cases where
"@HEAD" or "@@" is present on the command line, which is good. However,
if the user specifies a non-head revision:

  svn copy X $repos/trunk/oldfile_at_5

that should be an error (unless, perhaps, the latest revision of oldfile
is r5) but this patch hides the error.

The API svn_opt_eat_peg_revisions() checks that any supplied revision
peg specifier is syntactically valid, but it never gives the caller the
opportunity to check that the peg makes sense semantically.

For this reason, we should change all callers of
svn_opt_eat_peg_revisions() to parse and ignore "@@" endings (like
eat_peg_revisions() does) but also to throw an error if any non-empty
peg revision is specified (except perhaps HEAD, where that makes sense
for the particular command). For finer grained control, the caller
could use svn_opt_parse_path() instead, and should then verify that the
peg rev is either unspecified or a semantically acceptable value.

Then, the public name "svn_opt_eat_peg_revisions()" can be removed, as
it is still "new in 1.7", and the private name
"svn_opt_eat_peg_revisions()" can be deprecated or perhaps removed,
depending on 1.6-compatibility requirements.

- Julian

> /* Figure out which type of trace editor to use.
> If the src_paths are not homogeneous, setup_copy will return an error. */
> src_path = APR_ARRAY_IDX(targets, 0, const char *);
Received on 2010-06-10 14:57:56 CEST

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