Sorry missed bunch of inline responses first time around. not used to this
in gmail.
On Thu, Jun 14, 2012 at 8:13 AM, Hyrum K Wright
<hyrum.wright_at_wandisco.com>wrote:
> On Mon, Jun 11, 2012 at 2:39 PM, 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?
>
> Yes, just documenting that it is private should work. My primary
> concern is backward compat, rather than technical issues.
>
>
That is what I have done in the latest version of the patch. Java class
scoping is
pretty limiting, there is no equivalent to C# "internal" classifier as far
as I know,
and package scope forces you to have all related classes in one folder
which gets
in the way of clean folder structure.
> >>
> >>
> >> 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
>
> If you think that's cleaning, I'm happy to support it. It's been a
> while since I've been in that part of the code, and I don't think many
> folks around here have spent much time reasoning about it lately.
> You're probably the domain expert. :)
>
Thanks, this is how I shall proceed then.
>
> > Relatedly, do you have a
> >>
> >> high-level plan for this series of patches?
> >
> >
> > Let me get the up to date version published. But high level summary for
> now:
> >
> > 1. Have the following 3 patches reviewed, so that minimal version of
> SVNRa
> > implementation
> > can be committed to the javahl-ra branch and merged with the existing
> code
> > base.
> >
> > [PATCH] JavaHL: Add SVN_JNI_STRING macro to reduce amount of code
> necessary
> > to declare JNIStringHolder and check for exceptions
> > [PATCH] JavaHL: New method for creating java objects linked to their C++
> > counterpart
> > [PATCH] JavaHL: Factor out common context to be shared between SVNClient
> and
> > SVNRa classes
> >
> > 2. Commit additional RA function calls that do not require additional
> > enhancements to the
> > common code to the branch. These are mostly pass-through calls to the C
> > functions.
> >
> > 3. Submit another ~6 patches for review om shared code that will support
> RA
> > Editor code
> >
> > 4. Commit revised RA Editor implementation to the branch
> >
> > 5. Present proposal on making RA layer use Runtime exceptions rather than
> > checked ones
>
> Thanks for that, and updating the branch.
>
> A couple more things, now that I'm thinking about it:
> * There is a nascent project on google code called SVNJ
> (http://code.google.com/p/svnj/) which had some kind of editor
> implementation. That code is Apache licensed, so we should be able to
> use it. You may want to see if anything there is useful.
> * We're in the middle of a rather large effort to move from the
> current delta editor to something called Ev2. Ev2 should be simpler
> from a bindings perspective, but it won't be fully implemented for a
> while. When it is implemented, it's something you'll want to support,
> but we don't have a concrete timeline for it, yet.
>
> Thanks for being patient with my somewhat sporadic responses. :)
Vladimir
Received on 2012-06-14 14:49:16 CEST