Let's not over complicate this people. get_commit_ev2() has cancellation
params for no other reason than I was unaware of the session variables.
On Jun 27, 2012 12:04 AM, "Vladimir Berezniker" <vmpn_at_hitechman.com> wrote:
> On Tue, Jun 26, 2012 at 10:21 PM, Hyrum K Wright <hyrum_at_hyrumwright.org>
> wrote:
> > On Tue, Jun 26, 2012 at 7:10 PM, Vladimir Berezniker <vmpn_at_hitechman.com>
> wrote:
> >> On Tue, Jun 26, 2012 at 3:17 PM, Hyrum K Wright
> >> <hyrum.wright_at_wandisco.com> wrote:
> >>> On Sun, Jun 24, 2012 at 6:09 PM, Vladimir Berezniker <
> vmpn_at_hitechman.com> wrote:
> >>>> Hi All,
> >>>>
> >>>> I have been taking a peek at ev2 code to see what it would take for
> JavaHL
> >>>> implementation and I have following questions that do not seem do be
> covered
> >>>> by the docs:
> >>>>
> >>>> * svn_ra__get_commit_ev2() takes svn_cancel_func_t as a parameter,
> however
> >>>> session already has a cancellation callback specified in the
> >>>> svn_ra_callbacks2_t structure. What is the relationship between
> the two?
> >>>> Is one of these going away?
> >>>
> >>> I've not looked at the session struct, but can explain a bit about the
> >>> reason get_commit_ev2() takes a cancellation func/baton.
> >>> Historically, we've had a number of places where we'd wrap an editor
> >>> with a cancellation editor, which was just a pain. Ev2 folds the
> >>> (optional) cancellation editor into the editor itself.
> >> Understood
> >>>
> >>> My guess is that in most places, the session cancellation callback and
> >>> the one provided to get_commit_ev2() will be the same, though they
> >>> will be invoked in different places.
> >>
> >> But how come other ra functions grab the cancellation callback from the
> ra
> >> session, but ev2 editor wants to get one passed directly to it rather
> >> than taking
> >> one from the ra session?
> >
> > Ev2 is a generic API interface used across an large number of layers
> > in Subversion. (There is an editor interface for committing changes
> > to the FS layer, for example.) In many cases, these layers don't have
> > the concept of an ra session, so it would be a mistake for the Ev2
> > interface to use the ra session directly.
>
> My brain was thinking Ev2 always used inside RA layer. False assumption.
> Now I know better, thanks.
>
> >
> > However, I think I see your point: svn_ra__get_commit_ev2() takes both
> > an ra_session_t *and* a cancellation func/baton. Since that API is
> > specific to the ra layer, I think we can drop the cancellation
> > func/baton from that interface, and just use those in the session
> > object when we create the Ev2 object.
>
> You got it. I will try to restate more clearly below what exactly I
> was thinking.
>
> > But looking at the definition
> > for svn_ra_session_t, it doesn't look like there is a cancellation
> > func/baton in there. Am I missing something?
>
> Proper explanation and context from me in place of high level abstract
> statement.
>
> * Logically (implementation neutral model in my brain):
>
> When RA session is opened it is given a cancellation callback as part of
> the
> svn_ra_open parameters. Then later svn_ra__get_commit_ev2 wants similar
> information. Two possibilities:
>
> 1) There is a need for separate cancellation callback, therefore
> Vladimir is missing
> the "why" such ability is needed
>
> 2) There is no need to keep it separate.
>
> * Literally:
>
> When opening a session with svn_ra_open caller passes svn_ra_callbacks2_t
> that
> contains a cancellation callback. Looking at local and svn RA
> implementation
> they stash it in RA specific structures svn_ra_local__session_baton_t and
> svn_ra_svn__session_baton_t that are accessible via priv member of the
> svn_ra_session_t structure. Therefore when svn_ra__get_commit_ev2
> calls the implementation (or the shim) it should be able to get at the
> cancellation
> callback. Of course this only makes sense if choice #2 applies from above
> and that shim has a way of getting to the callback. As it would have
> to intercept
> the svn_ra_open and have a place to stash the pointer to the callbacks.
> This
> might just add complexity that is not worth it.
>
> My biggest concern was if #1 applied I am missing possibly important piece
> of
> the puzzle. E.g. Do I need to give ability to JavaHL caller to specify
> two cancellation
> callbacks, etc.
>
> >
> >>>
> >>>> * svn_editor_add_directory() function requires that
> >>>> "const apr_array_header_t *children" and "apr_hash_t *props"
> parameters
> >>>> cannot be NULL, instead an empty structure should be passed.
> However,
> >>>> svn_editor_alter_directory() permits the same parameters to be
> NULL.
> >>>> What is the reason for this difference?
> >>>
> >>> In the case of add_directory(), callers MUST provide the list of
> >>> children and properties, since they are not known a priori. However,
> >>> in the case of alter_directory(), if there are no changes, a NULL
> >>> parameter may be used to indicate that. (It would be rather pointless
> >>> to require clients to fetch the list of children and then replay them
> >>> back to the server if there are no changes.)
> >>>
> >> I think I was not very clear in asking the question. Lets take
> >> following use cases.
> >>
> >> 1) Add new directory without any children, with 1 property
> >> 2) Alter a directory, by adding 1 property
> >>
> >> In both cases caller needs to indicate that there are 0 children at
> >> play. In call to
> >> add_directory() 0 size array means no affected children. In call to
> >> alter_directory()
> >> NULL means no affected children.
> >
> > In case (1), the caller needs to affirm that there are zero children
> > forthcoming, by providing a zero-length array. In case (2), since no
> > children are added or deleted, no array needs to be provided. The
> > difference is that in case (1), there isn't any existing list of
> > children, so one is required.
> >
> >> Or another way, what is the behavior of alter_directory() if passed a
> >> 0 size children
> >> array rather than NULL?
> >
> > If alter_directory() gets an array of size 0, it means "this directory
> > now has zero children; you should expect to see a delete() for each of
> > those children." If alter_directory() gets a NULL array it means
> > "none of the children are added or deleted." Big difference.
>
> This made things crystal clear.
>
> >
> >> Thank you for your time, as I might just being dense here.
> >
> > No worries; I am often quite dense myself.
> >
>
> Thank you very much,
>
> Vladimir
>
Received on 2012-06-27 11:58:34 CEST