On Fri, Jun 15, 2012 at 5:44 AM, Mat Booth <mat.bo...@wandisco.com> wrote:
> On 14 June 2012 13:12, Vladimir Berezniker <v...@hitechman.com> wrote: > > > > > > > > On Thu, Jun 14, 2012 at 4:37 AM, Hyrum K Wright < > hyrum.wri...@wandisco.com> wrote: > >> > >> On Thu, Jun 14, 2012 at 10:16 AM, Mat Booth <mat.bo...@wandisco.com> > wrote: > >> > On 12 June 2012 03:11, Vladimir Berezniker <v...@hitechman.com> > wrote: > >> >> > >> >> > >> >> On Mon, Jun 11, 2012 at 8:39 AM, 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? > >> >>> > >> >>>> > >> >>>> > >> >>>> 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@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@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.@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? > > > > To begin with, just the interrogation methods, like get_log, > get_file_revs, get_file, get_dir, etc. Did you have a plan of which > parts to implement first? > You can see the plan in the BRANCH-README http://svn.apache.org/viewvc/subversion/branches/javahl-ra/BRANCH-README?revision=HEAD&view=markup The items you describe are under the following item: * Implement reading of state and content of repository [TODO] I will keep in mind that you have expressed interest in this item in future planing. Regards, Vladimir > I look forward to giving a try when it starts reaching a usable state. > Maybe I will even be able to report bugs for you :-) > > -- > Mat Booth > Software Engineer > WANdisco, Inc. > http://www.wandisco.com >
