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.wri...@wandisco.com>wrote: > On Mon, Jun 11, 2012 at 2:39 PM, Vladimir Berezniker <v...@hitechman.com> > wrote: > > > > > > On Mon, Jun 11, 2012 at 6:20 AM, Hyrum K Wright < > hyrum.wri...@wandisco.com> > > wrote: > >> > >> On Thu, Jun 7, 2012 at 4:03 AM, Vladimir Berezniker <v...@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