[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] JavaHL: Factor out common context to be shared between SVNClient and SVNRa classes

From: Vladimir Berezniker <vmpn_at_hitechman.com>
Date: Mon, 11 Jun 2012 09:53:12 -0400

On Mon, Jun 11, 2012 at 9:09 AM, Hyrum K Wright
<hyrum.wright_at_wandisco.com>wrote:

> On Mon, Jun 11, 2012 at 2:58 PM, Vladimir Berezniker <vmpn_at_hitechman.com>
> wrote:
> >
> >
> > On Mon, Jun 11, 2012 at 6:56 AM, Hyrum K Wright <
> hyrum.wright_at_wandisco.com>
> > wrote:
> >>
> >> On Thu, Jun 7, 2012 at 5:09 AM, Vladimir Berezniker <vmpn_at_hitechman.com
> >
> >> wrote:
> >> > Greetings,
> >> >
> >> > This patch signifies a point post which I can merge the existing logic
> >> > on
> >> > the
> >> > javahl-ra branch with starting parts of my proposed enhancements. So I
> >> > hope
> >> > people still have some patience left to review them.
> >> >
> >> > This patch moves parts of client context that are shared with ra
> context
> >> > into
> >> > common classes. As I cannot get svn to generate patches that deal
> with
> >> > copies
> >> > of existing file, before applying below the following svn copy command
> >> > have
> >> > to
> >> > be issued:
> >> >
> >> > svn cp
> >> >
> >> >
> subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java
> >> >
> >> >
> subversion/bindings/javahl/src/org/apache/subversion/javahl/CommonContext.java
> >> > svn cp subversion/bindings/javahl/native/ClientContext.h
> >> > subversion/bindings/javahl/native/CommonContext.h
> >> > svn cp subversion/bindings/javahl/native/ClientContext.cpp
> >> > subversion/bindings/javahl/native/CommonContext.cpp
> >> >
> >> >
> >> > Also please note that java part of this patch is already applied in
> >> > r1343452
> >> > & r1347345
> >> > on javahl-ra branch, due to my earlier mistake.
> >>
> >> If this work is specific to the things happening on the branch, I
> >> think you're fine committing it directly there.
> >
> >
> > Similar to the other patches so far, while the logic is used by RA
> > layer. The re factoring is applicable to the trunk on its own. Though
> > not as useful as it does not take care of the additional flexibility on
> > its own. It does lean more towards RA specific enhancements than the
> > other patches.
> >
> >>
> >>
> >> One question, though. The ClientContext object directly mirrors the
> >> svn_client_context_t struct in the C layer (the analogs are probably
> >> obvious). To my knowledge, we don't have a similar struct for the C
> >> ra layer. I'm just a little confused why we need to break this clear
> >> correlation with the C layer in Java, when we don't need to in C.
> >> I'll need to think on this a bit...
> >
> >
> > The refactoring represents the common elements between
> > the svn_client_ctx_t,
> > svn_ra_callbacks2_t and svn_commit_callback2_t needed by the
> > (svn_ra_get_commit_editor3) function. This overlap is demonstrated by the
> > existing code in the (svn_client__open_ra_session_internal) function that
> > goes
> > between the client context and ra session. Specifically client and ra
> layers
> > share
> > the following objects:
> >
> > * svn_auth_baton_t
> > * apr_hash_t (config) needed to create svn_auth_baton_t
> > * svn_ra_progress_notify_func_t
> >
> > Hope this helps,
>
> It helps a lot! Should we then call the new object something a bit
> more descriptive? Maybe CommitContext?
>

How about RASharedContext? It is RA layer data that is used across client
and
ra functions.

Also, I have not correctly described the role of the svn_commit_callback2_t.
While
its JNI implementation is shared between client and ra layers. It is not
part of the
svn_client_ctx_t nor svn_ra_callbacks2_t nor their JNI counterparts.
Rather, it is
passed directly to the svn_client_commit6 and svn_ra_get_commit_editor3
functions.

Sorry for the confusion, my brain was still thinking of patch series, where
one of the
patches factors this out to common code, when I mentioned it.

Vladimir

>
> -Hyrum
>
>
>
> --
>
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com/
>
Received on 2012-06-11 15:53:46 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.