Re: Bugs in new JavaHL merge tracking API's

On Mon, 04 Jun 2007, Hyrum K. Wright wrote:

> Mark Phippard wrote:
> > Here is a test you can use locally to produce the crash in #1 below.
> > Just paste it into BasicTests.java.
> >
> > /**
> > * Test the basic functionality of {@link
> > * org.tigris.subversion.javahl.SVNClientInterface#getMergeInfo(}).
> > * @throws Throwable
> > * @since 1.5
> > */
> > public void testMergeInfoCrash() throws Throwable
> > {
> > OneTest thisTest = new OneTest();
> > File localPath = new File(thisTest.getWCPath(), "notesWC");
> > client.checkout("http://svn.collab.net/repos/svn/trunk/notes",
> > localPath.toString(),
> > Revision.HEAD, Revision.HEAD, 1, true, true);
> > MergeInfo mergeInfo =
> > client.getMergeInfo(localPath.toString(), Revision.HEAD);
> > System.out.println(mergeInfo.toString());
> > }
>
> I've tracked the problem down to a NULL mergeinfo hash which gets passed
> back through the get_mergeinfo() apis. The following patch fixes the
> symptom, but I'm not familiar enough with the purpose of the code to
> determine if it is correct or not.
...

Thanks Hyrum, that was a big help in understanding the problem. We
recently change the get_mergeinfo() C APIs to return NULL mergeinfo to
represent "no merge info", and an empty hash to represent "explicitly
empty merge info on this and child paths". Looks like the JavaHL APIs
didn't catch up with this yet.

Mark, try this patch, and let me know:

Index: subversion/bindings/javahl/native/SVNClient.cpp
===================================================================
--- subversion/bindings/javahl/native/SVNClient.cpp (revision 25306)
+++ subversion/bindings/javahl/native/SVNClient.cpp (working copy)
@@ -633,6 +633,8 @@
                                          rev.revision(), ctx,
                                          requestPool.pool()),
                 NULL);
+ if (mergeinfo == NULL)
+ return NULL;
 
     // Transform mergeinfo into Java MergeInfo object.
     jclass clazz = env->FindClass(JAVA_PACKAGE "/MergeInfo");

 
> > On 6/4/07, Mark Phippard <markphip@gmail.com> wrote:
> >> Ran into a couple of bugs in some of the new merge-tracking related
> >> API's in JavaHL. I imagine some of these are not just in JavaHL.
> >>
> >> 1) getMergeInfo(localPath, Revision.HEAD) crashes the JVM when called
> >> on a WC from an http repository. In this case, it was tigris.org (SVN
> >> 1.3.x). To recreate, checkout:
> >>
> >> svn co http://subclipse.tigris.org/svn/subclipse/trunk/subclipse/feature
> >>
> >> Chosen because it is small and demonstrates the problem.
> >>
> >> Recreated this problem on Windows and OSX.
> >>
> >> 2) getCopySource(localPath, Revision.HEAD) when called on a local
> >> path that has never been copied gives these errors:
> >>
> >> Exception in thread "main" java.lang.IllegalArgumentException: Invalid
> >> (negative) revision number: -1
> >> at
> >> org.tigris.subversion.javahl.Revision$Number.<init>(Revision.java:216)
> >> at
> >> org.tigris.subversion.javahl.Revision.getInstance(Revision.java:127)
> >> at org.tigris.subversion.javahl.SVNClient.getCopySource(Native
> >> Method)
> >> at
> >> org.tigris.subversion.svnclientadapter.javahl.AbstractJhlClientAdapter.getCopySource(AbstractJhlClientAdapter.java:2034)
> >>
> >> at
> >> org.tigris.subversion.svnclientadapter.javahl.AbstractJhlClientAdapter.getCopySource(AbstractJhlClientAdapter.java:2025)
> >>
> >>
> >>
> >> I think the problem is that the C++ code is trying to create null
> >> revision objects for the path by using a value of -1. But that is not
> >> a value the JavaHL Revision class will accept. This problem does not
> >> appear to cause any additional problems other than the exception being
> >> logged.

Not certain if the above patch addresses this as well.