On Thu, Jun 14, 2012 at 8:40 AM, Vladimir Berezniker <v...@hitechman.com>wrote:
> > > 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. >> >> >> >> >> >> >> 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. :) >> >> > 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. >> > > Glad to help. > > >> >> 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. >> > > I will take a look. > Looks like this project's main interests in ability to implement editors in java. This part is not part of my plan at the moment. Also it feels like it would make sense to target ev2 API for such use case. The remaining utility code looks very similar to what is already in JavaHL. I will keep this project in mind as I develop the RA bindings. Thank you, Vladimir > > >> * 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. >> > > I saw the emails and took a peek on the branch, it did look simpler to me. > I did not look at it in huge detail though. I figured I need to get > javahl-ra to a > stable point and then see what it takes to port it to ev2 branch, once the > editor > v2 API considered stable enough (which it could be already). > > >> Thanks for being patient with my somewhat sporadic responses. :) >> >> > We all busy, I haven't had huge amount of time for RA work either. > > Thank you for your help in regardless of time-frame or shape that it > takes. :) > > Vladimir >
