On Thu, Jun 14, 2012 at 4:37 AM, Hyrum K Wright
<hyrum.wright_at_wandisco.com>wrote:
> On Thu, Jun 14, 2012 at 10:16 AM, Mat Booth <mat.booth_at_wandisco.com>
> wrote:
> > On 12 June 2012 03:11, Vladimir Berezniker <vmpn_at_hitechman.com> wrote:
> >>
> >>
> >> On Mon, Jun 11, 2012 at 8:39 AM, Vladimir Berezniker <
> vmpn_at_hitechman.com>
> >> wrote:
> >>>
> >>>
> >>>
> >>> On Mon, Jun 11, 2012 at 6:20 AM, Hyrum K Wright
> >>> <hyrum.wright_at_wandisco.com> wrote:
> >>>>
> >>>> On Thu, Jun 7, 2012 at 4:03 AM, Vladimir Berezniker <
> vmpn_at_hitechman.com>
> >>>> wrote:
> >>>> > Hi All,
> >>>> >
> >>>> > The intention if this patch is to introduce reusable logic for
> creating
> >>>> > java
> >>>> > objects from within C++. This keeps object creation logic fully
> within
> >>>> > C++
> >>>> > while leaving to java the decision as to when they will be
> destroyed.
> >>>> > It
> >>>> > will be used by RA code to allocate container object for items like
> >>>> > SVNRa,
> >>>> > Editor, Directory and File.
> >>>> >
> >>>> > Thank you,
> >>>> >
> >>>> > Vladimir
> >>>> >
> >>>> > [[[
> >>>> > JavaHL: New method for creating java objects linked to their C++
> >>>> > counterpart
> >>>> >
> >>>> > [ in subversion/bindings/javahl/native ]
> >>>> >
> >>>> > * SVNBase.cpp, SVNBase.h
> >>>> > (createCppBoundObject): New method for creating java objects
> linked
> >>>> > to
> >>>> > their
> >>>> > C++ counterpart
> >>>> >
> >>>> > [ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]
> >>>> >
> >>>> > * JNIObject.java: Base class for JNI linked java objects
> >>>> > ]]]
> >>>>
> >>>> Looks good. Is there a way to mark the new Java class as private?
> >>>
> >>>
> >>> The original plan is to put RA java classes in a ra/ sub package, so
> >>> package visible
> >>> would not work in such case. It is an abstract class, even if someone
> >>> instantiates
> >>> it will be harmless, as it will not create any C++ instances. Would
> >>> putting a good
> >>> javadoc explaining it is not part of the public API address your
> concerns?
> >>>
> >>>>
> >>>>
> >>>> How do you plan to use this functionality?
> >>>
> >>>
> >>> As compared to what SVNClient class does, where caller creates the java
> >>> object which
> >>> in turn calls native method call to create a matching native object.
> With
> >>> RA layer,
> >>> consumer calls a factory method:
> >>>
> >>> /**
> >>> * Crates RA session for a given url with provided context
> >>> * @param url to connect to
> >>> * @param uuid of the remote repository, can be null if uuid check is
> not
> >>> desired
> >>> * @param config configuration to use for the session.
> >>> * @return RA session
> >>> */
> >>> public static native ISVNRa createRaSession(String url, String uuid,
> >>> ISVNRaConfig config);
> >>> }
> >>>
> >>> that method is responsible for instantiation of the related C++ and
> Java
> >>> classes.
> >>> I think it is cleaner because:
> >>>
> >>> It better encapsulates implementation class: User only sees factory
> >>> and ISVNRa interface
> >>> Instantiation logic resides in a single language
> >>> It avoids chicken and the egg with not year having cppAddr when java
> >>> object is constructed: Allows enforcement that parameter is supplied
> via
> >>> constructor argument
> >>>
> >>> Relatedly, do you have a
> >>>>
> >>>> high-level plan for this series of patches?
> >>>
> >>>
> >>
> >> More detailed plan, full version is in the BRANCH-README on javahl-ra
> >> branch:
> >>
> >> * Prepare existing code for merging of SVNRa editor [IN
> PROGRESS]
> >>
> >> 01. JavaHL: Changed return value from the
> >> java svn_stream_t read function to be compatible
> >> with the txdelta_next_window function
> [trunk_at_1342720]
> >>
> >> 02. JavaHL: Explicitly pass jobject jthis when
> >> processing dispose() call rather than stashing a
> >> reference in the SVNBase class where it can be
> >> misused later
> [trunk_at_1342810]
> >>
> >> 03. JavaHL: Add SVN_JNI_STRING macro to reduce amount
> >> of code necessary to declare JNIStringHolder and
> >> check for exceptions [IN REVIEW]
> >>
> >> 04. JavaHL: New method for creating java objects
> >> linked to their C++ counterpart [IN REVIEW]
> >>
> >> 05. JavaHL: Factored out common context for later use
> >> by SVNRa class [IN REVIEW]
> >>
> >> 06. JavaHL: Minimal implementation of SVNRa [TODO]
> >>
> >> 07. JavaHL: Merge existing SVNRepoAccess into SVNRA [TODO]
> >>
> >>
> >> * Implement other ra functions not requiring addition code
> >> refactoring [TODO]
> >>
> >>
> >> * Apply editor support patches [TODO]
> >> xx. JavaHL: Support returning non const, empty rather
> >> than NULL hash as required by
> >> (svn_ra_get_commit_editor3)
> >> apr_hash_t *revprop_table parameter
> [br._at_1343456
> >> TBR]
> >>
> >> xx. JavaHL: Support keeping global reference to the
> >> callback java object as required by the RA API due
> >> to callback being used across method calls [TODO]
> >>
> >> xx. JavaHL: Added support for creating of svn_string_t
> >> from JNIByteArray [TODO]
> >>
> >> xx. JavaHL: Added SVN_JNI_BYTE_ARRAY macro to reduce
> >> amount of duplicate code dealing with jbyteArray
> >> wrapper and checking for exceptions [TODO]
> >>
> >> xx. JavaHL: Factor out common java string map
> >> processing into StringsTable class from
> >> svn_string_t specific processing in the
> >> RevpropTable class [TODO]
> >>
> >>
> >
> > Hi Vladimir,
> >
> > When you start implementing JavaHL RA, what would be the best way for
> > me to try it out? Will the work be on this branch first:
> > http://svn.apache.org/repos/asf/subversion/branches/javahl-ra/ ?
>
>
Are there any particular features of the RA JavaHL layer that you would
be interested in trying out?
> I think that's the goal. We're encouraging him to put as much "common
> functionality" type stuff on trunk, though.
>
Yep, that is the approach I am following.
To give more context to the way this has been progressing: I submit
few (0 to ~3) patches to make it easier to review while developing few steps
ahead on my local PC so that if/when the patches are signed off I have the
next batch ready, without getting too much ahead; as I keep realizing better
ways to design the code due to the received feedback.
I try to keep the plan reflective of my local progress, so where you see
more
detail that means I have at least partial code locally, and where the plan
has less
detail that means the code to back that step is not there yet.
Regards,
Vladimir
Received on 2012-06-14 14:13:02 CEST