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/oldf...@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 *);