Re: [patch] Fix for JavaHL copy sources bug reported by Alexander Kitaev

Thanks for getting back to us, Alex. I've updated the patch as you suggested,
passing Revision.BASE for WC paths and Revision.HEAD for URLs in the Subversion
1.0 overload of the SVNClient.copy() API:

[[[
JavaHL: Fix bug in the CopySources::array() JNI method, where the peg
revision was set incorrectly, and in the Java SVNClient.copy() method,
where the HEAD revision was used as the peg revision for a WC path.

[ in subversion/bindings/javahl/ ]

* native/CopySources.cpp
  (array): Invoke the CopySource.getPegRevision() method when
    extracting the peg revision of a copy source from a Java object.

* src/org/tigris/subversion/javahl/SVNClient.java
  (copy): Use a peg revision of HEAD for a copy source that's a URL,
    and WORKING for one that's a WC path.

Reported by: Alexander Kitaev <Alexander.Kitaev_at_svnkit.com>
]]]

The above patch requires r31993 from Subversion's trunk, and passes all
existing tests (we don't actually have an explicit test for this deprecated
API).

However, when I went to look at the corresponding test code, I noticed that
BasicTests.testCopy() currently passes a null peg rev (which should default to
HEAD) to the 1.5-era copy() API in conjunction with a WC path (see line 849).
It's curious that there's no problem in this case, since both Java copy()
overloads call through to the same underlying JNI and svn_client_copy4() APIs.
When I try something similar with a command-line client built from trunk,
it also works fine:

$ svn cp README_at_HEAD foo
A foo

Would you mind providing some more details on the failure you're seeing?
Some runnable code snippets would be especially helpful.

Thanks!
- Dan

p.s. In the meanwhile, I'll go ahead and commit the change to CopySources.cpp.

On Wed, 25 Jun 2008, Alexander Kitaev wrote:

> Dear Daniel,
>
> I'm sorry for delay with the answer. I've built subversion from trunk
> and applied your patch. Unfortunately it doesn't work as expected:
>
> peg revision specified in CopySource is used now, but passing "null" in
> place of peg revision in deprecated copy method results in the same
> error as before (working copy path required for that operation).
>
> So, I assume, we should either left this method as it is (as it is
> deprecated) or pass Revision.BASE or Revision.HEAD basing on the path kind.
>
> Alexander Kitaev,
> TMate Software,
> http://svnkit.com/ - Java [Sub]Versioning Library!
>
> Daniel L. Rall wrote:
> >oka reported this today on IRC. He mentioned that he'd have a chance
> >to test it out over the weekend. We could use some improved test coverage
> >here, not necessarily for the deprecated API in this patch, but for the
> >various permutations of CopySource usage in general.
> >
> >oka, if this patch works out for you, please expand on the summary of my
> >change log message, as it doesn't properly capture the use cases involved
> >here. Assuming this works out, I'll propose it for backport to 1.5.1.