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" <v...@hitechman.com> wrote:
> On Tue, Jun 26, 2012 at 10:21 PM, Hyrum K Wright <hy...@hyrumwright.org> > wrote: > > On Tue, Jun 26, 2012 at 7:10 PM, Vladimir Berezniker <v...@hitechman.com> > wrote: > >> On Tue, Jun 26, 2012 at 3:17 PM, Hyrum K Wright > >> <hyrum.wri...@wandisco.com> wrote: > >>> On Sun, Jun 24, 2012 at 6:09 PM, Vladimir Berezniker < > v...@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 >
