Re: Ev2 design questions
On Tue, Jun 26, 2012 at 10:21 PM, Hyrum K Wright wrote: > On Tue, Jun 26, 2012 at 7:10 PM, Vladimir Berezniker > wrote: >> On Tue, Jun 26, 2012 at 3:17 PM, Hyrum K Wright >> wrote: >>> On Sun, Jun 24, 2012 at 6:09 PM, Vladimir Berezniker >>> 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
Re: Ev2 design questions
On Tue, Jun 26, 2012 at 7:10 PM, Vladimir Berezniker wrote: > On Tue, Jun 26, 2012 at 3:17 PM, Hyrum K Wright > wrote: >> On Sun, Jun 24, 2012 at 6:09 PM, Vladimir Berezniker >> 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. 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. 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? >> >>> * 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. > Thank you for your time, as I might just being dense here. No worries; I am often quite dense myself. -Hyrum
Re: [RFC] JavaHL: Moving some of the C++ object address logic into Java
On Tue, Jun 26, 2012 at 3:18 PM, Hyrum K Wright
wrote:
> On Sun, Jun 24, 2012 at 9:20 PM, Vladimir Berezniker
> wrote:
>> Hi All,
>>
>> In the current JavaHL code the C++ objects are attached via pointer stored in
>> the long cppAddr field in java object. The way C++ code gets hold of the
>> cppAddr value is to read this field using the GetLongField(). In summary
>>
>> Java:
>>
>> class JHLClass
>> {
>> long cppAddr;
>> public native method();
>> }
>>
>>
>> JNI Stub:
>>
>> Java_method(JNIEnv *env, jobject jthis)
>> {
>> JHLClass cppObj = JHLClass::getCppObject(jthis);
>> }
>>
>> C++:
>>
>> class JHLClass
>> {
>> JHLClass getCppObject(jobject jthis) { }
>> }
>>
>>
>> I was thinking why not simplify this by doing all object->jlong lookup in the
>> java land. What takes about 20 lines of C++ takes 1 line in java, at a cost
>> of
>> implementing 3 line java wrapper method that converts from caller visible
>> method signature to native method signature. As follows:
>>
>> class JHLClass
>> {
>> long cppAddr;
>>
>> private native static method(long cppAddr);
>> method() {
>> method(cppAddr);
>> }
>> }
>>
>> JNI Stub:
>>
>> Java_method(JNIEnv *env, jlong cppAdder)
>> {
>> JHLClass cppObj = reinterpret_cast(fileCppAddr);
>> }
>>
>> C++: No additional code necessary
>>
>> This will require a related change in JNIStackElement, as it won't have jthis
>> anymore. But this logic also can move up to java code in a similar manner.
>>
>> What do others think? Any objections at least of doing this in RA functions?
>
> While I can't comment on the specific implications of your plan,
> generally the more we can write in Java in the JavaHL bindings, the
> better. Hopefully that nugget of wisdom gives you some insight. :)
>
That answers it. I will put a patch together to illustrate the above
on real code.
Thank you,
Vladimir
Re: Ev2 design questions
On Tue, Jun 26, 2012 at 3:17 PM, Hyrum K Wright wrote: > On Sun, Jun 24, 2012 at 6:09 PM, Vladimir Berezniker > 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? > >> * 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. Or another way, what is the behavior of alter_directory() if passed a 0 size children array rather than NULL? Thank you for your time, as I might just being dense here. Vladimir
Re: svn commit: r1354029 - in /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c
On Tue, Jun 26, 2012 at 06:00:06PM +0200, Bert Huijben wrote: > > > The intent of resolving conflicts during an operation for 1.5 API users > > > won't work anyway, since our backwards compat wrappers will always pass > > > a 1.7-style conflict_func2 which wraps a 1.5 conflict_func. > > > > I'm unclear on this. It seems that if no conflict2_func is passed, > > then we can pass a wrapper for 1.7-style resolution during the > > operation. ie. stop hard-coding NULL and possibly provide a wrapper > > for the 1.5 API. > > > > (not sure what we'd do for 1.6 and 1.7, but it seems pretty clear we > > could support 1.5 and before) > > For the libsvn_client api we already wrap the 1.7 api, by setting a default > wrapper in the svn_client_ctx_t that calls the conflict_func. > (Before 1.5 we didn't use any conflict callbacks) > > I think the behavior difference Stephan talks about is that he can ignore an > older style conflict handler without a newer one. Yes, it seems with our libsvn_client APIs callers will always pass in a conflict_func2 even if a 1.5 API users passes a conflict_func. So we cannot detect what API the caller is really using by inspecting the conflict_func2 pointer. And even if we could, it seems that trying to keep the exact 1.5 behaviour for 1.5 API users is more trouble than it is worth. It's not as if the new behaviour will fundamentally break things. The callback will just be invoked at a different time. And if we're willing to expose code compiled against the 1.7 API to this, why treat code compiled against 1.5 differently?
Re: [PATCH] JavaHL: Support commit callback across method invocations
On Sun, Jun 24, 2012 at 8:31 PM, Vladimir Berezniker wrote: > In case of delta editor, commit callback is provided at the time of the editor > creation and used during the close_edit() call. For that there is a need to > keep a global reference to the underlying java object so that it does not get > GCed meanwhile. > > Attached please find the patch that adds such capabilities. > > Thank you, > > Vladimir > > [[[ > JavaHL: Support keeping global reference to the callback java object for cases > when callback is being used across method calls > > [ in subversion/bindings/javahl/native ] > > * CommitCallback.cpp, > CommitCallback.h > (CommitCallback, ~CommitCallback): Add handling of additional parameter and > state when requesting a global reference to be kept > ]]] I don't like putting the burden on the caller that constructs the Commit callback to define whether a global reference should be held or not. Can't the thing consuming the callback reference make that determination? (Alternatively, could we have a subclass which keeps a global reference around, and let the caller instantiate that if needed?) -Hyrum -- uberSVN: Apache Subversion Made Easy http://www.uberSVN.com/
Re: [RFC] JavaHL: Moving some of the C++ object address logic into Java
On Sun, Jun 24, 2012 at 9:20 PM, Vladimir Berezniker wrote:
> Hi All,
>
> In the current JavaHL code the C++ objects are attached via pointer stored in
> the long cppAddr field in java object. The way C++ code gets hold of the
> cppAddr value is to read this field using the GetLongField(). In summary
>
> Java:
>
> class JHLClass
> {
> long cppAddr;
> public native method();
> }
>
>
> JNI Stub:
>
> Java_method(JNIEnv *env, jobject jthis)
> {
> JHLClass cppObj = JHLClass::getCppObject(jthis);
> }
>
> C++:
>
> class JHLClass
> {
> JHLClass getCppObject(jobject jthis) { }
> }
>
>
> I was thinking why not simplify this by doing all object->jlong lookup in the
> java land. What takes about 20 lines of C++ takes 1 line in java, at a cost of
> implementing 3 line java wrapper method that converts from caller visible
> method signature to native method signature. As follows:
>
> class JHLClass
> {
> long cppAddr;
>
> private native static method(long cppAddr);
> method() {
> method(cppAddr);
> }
> }
>
> JNI Stub:
>
> Java_method(JNIEnv *env, jlong cppAdder)
> {
> JHLClass cppObj = reinterpret_cast(fileCppAddr);
> }
>
> C++: No additional code necessary
>
> This will require a related change in JNIStackElement, as it won't have jthis
> anymore. But this logic also can move up to java code in a similar manner.
>
> What do others think? Any objections at least of doing this in RA functions?
While I can't comment on the specific implications of your plan,
generally the more we can write in Java in the JavaHL bindings, the
better. Hopefully that nugget of wisdom gives you some insight. :)
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: Ev2 design questions
On Sun, Jun 24, 2012 at 6:09 PM, Vladimir Berezniker 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. 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. > * 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.) -Hyrum -- uberSVN: Apache Subversion Made Easy http://www.uberSVN.com/
Re: Bigger --deltas dump with 1.7.5 than with 1.6.17
On Sat, Jun 23, 2012 at 9:15 AM, Branko Čibej wrote: > On 23.06.2012 00:23, Vincent Lefevre wrote: >> Thanks for the explanations. >> >> On 2012-06-22 00:18:50 +0200, Stefan Fuhrmann wrote: >>> xdelta uses fixed-size 100kByte deltification windows. >>> The Changelog file in question is >400k, i.e. 4+ windows. >>> You insert about 2k at the beginning of the file, moving >>> the older parts by a similar distance. At the beginning >>> of each delta window, those 2+k don't have deltification >>> partner. Expected delta size: > 4 x 2kBytes. >> Wouldn't it be possible to change this size of deltification windows, >> with a command-line option (for svnadmin dump) and/or an option in >> the config file? >> >> The gain would be quite important on some kinds of changes (typically >> ChangeLog update), and a dump of a revision is something that could >> be done once and for all, so that it would be worth to spend time on >> optimizing it. > > It's not that simple, unfortunately. Currently both the delta generator > and especially the delta combiner expect the windows to be a known, > constant size. You can't just change them without affecting a /lot/ of code. > > Frankly I don't know how to make all the delta code paths gracefully > deal with variable window sizes. I'm sure it can be done, but the ROI at > this point is not good at all. It's a shame that we didn't include the window size in the encoding of xdelta windows back in the day. Oh, well, a lesson for the future, I suppose. -Hyrum -- uberSVN: Apache Subversion Made Easy http://www.uberSVN.com/
RE: svn commit: r1354029 - in /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c
> -Original Message- > From: Greg Stein [mailto:gst...@gmail.com] > Sent: dinsdag 26 juni 2012 17:48 > To: dev@subversion.apache.org > Subject: Re: svn commit: r1354029 - in > /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c > > On Tue, Jun 26, 2012 at 10:21 AM, wrote: > > Author: stsp > > Date: Tue Jun 26 14:21:18 2012 > > New Revision: 1354029 > > > > URL: http://svn.apache.org/viewvc?rev=1354029&view=rev > > Log: > > Remove some redundant local variables. We're now effectively always > postponing > > conflict resolution until after an update/switch/merge has completed. > > > > The intent of resolving conflicts during an operation for 1.5 API users > > won't work anyway, since our backwards compat wrappers will always pass > > a 1.7-style conflict_func2 which wraps a 1.5 conflict_func. > > I'm unclear on this. It seems that if no conflict2_func is passed, > then we can pass a wrapper for 1.7-style resolution during the > operation. ie. stop hard-coding NULL and possibly provide a wrapper > for the 1.5 API. > > (not sure what we'd do for 1.6 and 1.7, but it seems pretty clear we > could support 1.5 and before) For the libsvn_client api we already wrap the 1.7 api, by setting a default wrapper in the svn_client_ctx_t that calls the conflict_func. (Before 1.5 we didn't use any conflict callbacks) I think the behavior difference Stephan talks about is that he can ignore an older style conflict handler without a newer one. Bert > > >... > > Cheers, > -g
Re: svn commit: r1354029 - in /subversion/trunk/subversion/libsvn_client: merge.c switch.c update.c
On Tue, Jun 26, 2012 at 10:21 AM, wrote: > Author: stsp > Date: Tue Jun 26 14:21:18 2012 > New Revision: 1354029 > > URL: http://svn.apache.org/viewvc?rev=1354029&view=rev > Log: > Remove some redundant local variables. We're now effectively always postponing > conflict resolution until after an update/switch/merge has completed. > > The intent of resolving conflicts during an operation for 1.5 API users > won't work anyway, since our backwards compat wrappers will always pass > a 1.7-style conflict_func2 which wraps a 1.5 conflict_func. I'm unclear on this. It seems that if no conflict2_func is passed, then we can pass a wrapper for 1.7-style resolution during the operation. ie. stop hard-coding NULL and possibly provide a wrapper for the 1.5 API. (not sure what we'd do for 1.6 and 1.7, but it seems pretty clear we could support 1.5 and before) >... Cheers, -g
Conflict storage for 1.8 (Re: svn commit: r1348513 - /subversion/trunk/notes/wc-ng/conflict-storage-2.0)
On Sat, Jun 09, 2012 at 10:09:13PM -, rhuij...@apache.org wrote:
> Author: rhuijben
> Date: Sat Jun 9 22:09:13 2012
> New Revision: 1348513
>
> URL: http://svn.apache.org/viewvc?rev=1348513&view=rev
> Log:
> * notes/wc-ng/conflict-storage-2.0
> Add some initial design of a simple but extensible skel based conflict
> store,
> written down over the last few days.
Hi Bert,
I've skimmed this proposal, and I do think it would be a good way
forward without any obvious drawbacks. What are your plans on getting
this implemented in time for 1.8? Do you need help with this?
>
> Added:
> subversion/trunk/notes/wc-ng/conflict-storage-2.0 (with props)
>
> Added: subversion/trunk/notes/wc-ng/conflict-storage-2.0
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/notes/wc-ng/conflict-storage-2.0?rev=1348513&view=auto
> ==
> --- subversion/trunk/notes/wc-ng/conflict-storage-2.0 (added)
> +++ subversion/trunk/notes/wc-ng/conflict-storage-2.0 Sat Jun 9 22:09:13 2012
> @@ -0,0 +1,172 @@
> +-*- Text
> -*-
> +
> +Conflict storage 2.0/NG
> +===
> +
> +For WC-NG we tried to introduce a new storage model for conflicts, but we
> +didn't get this proposal completed for Subversion 1.7. I would like to
> +revive the new conflict storage topic with a new simple model that can
> +be extended later to allow the more advanced scenarios.
> +
> +I'm going to simplify the model described in the 'conflict-storage' document
> +a bit in an attempt to allow fitting this in the Subversion 1.8.0 release.
> +
> +
> +Current Situation
> +-
> +
> +We currently support three kinds of conflicts.
> +
> + * Text conflicts
> + * Property conflicts
> + * Tree conflicts
> +
> +These conflicts all have their own storage model.
> +
> +Text conflicts are stored in the working copy in marker files. This allows
> +resolving later using the --accept argument of 'svn resolve'.
> +While resolving non-interactively we just know where these files are.
> +
> +Property conflicts are stored in a 'write only' marker file. Only --accept
> +working is really supported.
> +While resolving non-interactively we just know that there is/was some
> conflict.
> +
> +Tree conflicts are just stored in a skel in the ACTUAL table of wc.db. Many
> +details are available while resolving both interactively and
> non-interatively,
> +but we don't have the necessary logic to provide help in common user
> scenarios.
> +
> +In our current code a node can be tree conflicted or 'node' conflicted, but
> not
> +both. A 'node' conflicted node can have either text or property conflicts, or
> +both.
> +
> +Once a node is tree or 'node' conflicted, it is skipped by future update,
> +switch and merge operations so we (currently) don't have to allow layering
> +multiple conflicts of the same type while creating conflicts.
> +
> +(BH: My guess would be that resolvers might encounter such situations on
> multi
> + layer moves, but in that case multiple tree locations are involved. So maybe
> + we can work around that)
> +
> +Plans for the initial version
> +-
> +
> +What I would like to introduce for 1.8 would be a unified extensible storage
> +model that allows the interactive and non-interactive resolvers access to the
> +same information. In most cases this can be accomplished by collecting the
> +information and storing it in a conflict-ng skel that can be stored in the
> +ACTUAL table, like we do for tree conflicts now.
> +
> +The Skel I would like to propose for the intial version would be
> +
> +(WHY (CONFLICT*) ...)
> +
> +Where ... is currently undefined, but explicitly free for future extension.
> +
> +
> +Where 'WHY' would tell why the conflict was introduced
> +WHY = (OPERATION (PATH_REV*) ...)
> +
> +OPERATION = "update" | "switch" | "merge" | ...
> +PATH_REV = ("subversion" repos_id repos_relpath revision kind ...) | () | ...
> +
> +repos_id, repos_relpath, revision and kind are defined as in wc-metadata.sql.
> +
> +"update" and "switch" will have 1 PATH_REV item, containing the original BASE
> +path from before the update/switch. The new location is already available in
> +BASE so doesn't have to be duplicated.
> +
> +Merge will have 2 items: the left and right paths. These can come from a
> +different repository.
> +
> +An empty skel specifies that there is no location. (Tree conflicts and/or
> +upgrade scenarios). Future versions may introduce other origins.
> +
> +CONFLICT =
> + ("text" MARKERS ...) |
> + ("prop" MARKERS (PROPS-OLD PROPS-NEW PROPS-WORKING) ...) |
> + ("tree" () LOCAL-STATE INCOMING-ACTION ...) |
> + ("..." MARKERS ...)
> +
> +A node can have more than one conflict, so this is defined to be a list.
> +Currently this will be either a tree conflict, or a 'node' conflict, that
> might
> +be text, prop or both.
> +
> +Every confl
Re: svn commit: r1353676 - in /subversion/trunk/subversion: include/private/svn_wc_private.h include/svn_ra.h libsvn_client/ra.c libsvn_ra_serf/update.c libsvn_wc/adm_ops.c
On 06/25/2012 02:24 PM, Bert Huijben wrote: >> Ewww... yeah, table scan is no good. So what's involved in adding, >> populating, and maintaining such a new index? > > Something like: > (I only tested the queries with the query test tool, not via the test suite) [patch snipped] Wow. This WC-NG thing might just pan out. Thanks for the patch, Bert. I'll give it a run through the test suite before committing. -- C. Michael Pilato CollabNet <> www.collab.net <> Enterprise Cloud Development signature.asc Description: OpenPGP digital signature
Re: Format bump for 1.8?
On Mon, Jun 25, 2012 at 05:53:46PM -0400, Mark Phippard wrote: > What about the move feature? What happens when 1.7 client commits a move or > partial move that was made with 1.8? I've written the 1.8 code in a way that copes with broken moves (where only one half of the move exists) and treats those as a normal copy/delete. So this shouldn't really be an issue, apart from the fact that you're falling back to 1.7 behaviour. I believe Philip mentioned he didn't like the idea of allowing partially recorded moves but off-hand I don't know whether he changed the code in this regard.
Re: Format bump for 1.8?
On Tue, Jun 26, 2012 at 07:47:16AM +0100, Daniel Shahaf wrote: > Stefan Sperling wrote on Mon, Jun 25, 2012 at 23:27:37 +0200: > > On Mon, Jun 25, 2012 at 10:23:38PM +0100, Daniel Shahaf wrote: > > > From the peanut gallery: as long as svn1.7 errors out, but doesn't > > > corrupt any of the data or metadata, perhaps that's something we're > > > willing to live with? > > > > This approach might be suboptimal for GUI apps that use status to > > refresh icons etc. > > I guess we didn't use a special error code for the "New value in enum" > case... (so GUIs can recognize it and treat it as ETHISSVNISTOOOLD) They could catch the error, but existing GUIs based on 1.7 code won't catch it right now because they don't need to. Requiring the error to be caught is still a breaking change for existing code. And I don't like the idea of pushing responsibility for backwards compat onto the shoulders of users of our APIs.
Re: Format bump for 1.8?
On Mon, Jun 25, 2012 at 07:51:59PM -0400, Greg Stein wrote: > Historically, whenever we have changed the format, then the working > copy is auto-upgraded the first time we (re)wrote the .svn/entries > file. The upgrade was invisible and fast, so it Just Happened. At that > moment, older versions of Subversion could no longer use the working > copy. > > The 1.7 upgrade was considered too time-consuming and invasive to > continue with the auto-upgrade mechanism. We required people to > manually upgrade the working copy. I would prefer to by default keep working copy upgrades manual from now on. A silent auto-upgrade generates many problems and help desk cases in large organisations. I've seen people complain about "Eclipse stopped working" when what really happened is that JavaHL suddenly used a different set of Subversion libs and eclipse dropped all working copies from the GUI. Which can take them a day or so to figure out -- especially if the user who ran into the problem doesn't have a good understanding about what's really happening beneath the shiny icons in the GUI, which is more often the case than not because these GUIs are designed with the goal of hiding complexity. This and other issues are likely to happen when people mix TortoiseSVN and eclipse, which is a common setup in many organisations I've walked into. It's much better if they get an error saying "your working copy is too old, please upgrade". They can handle that because they're being told what the problem is. We could add a config switch that says "please auto-upgrade" for those who really need it (i.e. folks who have a million old and rotten working copies which they may some day want to keep using with some newer svn release without running 'svn upgrade' on each).
Re: svn commit: r1353748 - /subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
On Tue, Jun 26, 2012 at 12:15:00AM +0200, Stephen Butler wrote: > > On Jun 25, 2012, at 23:25 , s...@apache.org wrote: > > > Author: stsp > > Date: Mon Jun 25 21:25:47 2012 > > New Revision: 1353748 > > > > URL: http://svn.apache.org/viewvc?rev=1353748&view=rev > > Log: > > Following up to r1353730, fix a test failure due to passing the wrong number > > of arguments to a helper function. > > Thank for catching my mistake, Stefan. I should be more careful when > splitting > up a big ugly patch into readable commits. > > Thanks, > Steve No problem!
Re: svn commit: r1353738 - /subversion/trunk/subversion/libsvn_client/switch.c
On Mon, Jun 25, 2012 at 07:54:39PM -0400, Greg Stein wrote: > On Mon, Jun 25, 2012 at 4:50 PM, wrote: > >... > > +++ subversion/trunk/subversion/libsvn_client/switch.c Mon Jun 25 20:50:17 > > 2012 > > @@ -94,8 +94,6 @@ switch_internal(svn_revnum_t *result_rev > > : NULL; > > /* Resolve conflicts post-switch for 1.7 and above API users. */ > > svn_boolean_t resolve_conflicts_post_switch = (ctx->conflict_func2 != > > NULL); > > Note the above line... > > >... > > @@ -252,7 +239,10 @@ switch_internal(svn_revnum_t *result_rev > > server_supports_depth, > > diff3_cmd, preserved_exts, > > svn_client__dirent_fetcher, &dfb, > > - ctx->conflict_func2, > > ctx->conflict_baton2, > > + resolve_conflicts_post_switch ? > > + NULL : ctx->conflict_func2, > > + resolve_conflicts_post_switch ? > > + NULL : ctx->conflict_baton2, > > You may as well just pass NULL unconditionally. If > resolve_conflicts_post_switch is FALSE, the the func is NULL. > > >... > > Cheers, > -g Right. I did so for update but forgot about it while modifying switch.
Re: svn commit: r1351792 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_tests.py
On Mon, Jun 25, 2012 at 08:00:03PM -0400, Greg Stein wrote: > On Tue, Jun 19, 2012 at 2:26 PM, wrote: > > Author: stsp > > Date: Tue Jun 19 18:26:30 2012 > > New Revision: 1351792 > > > > URL: http://svn.apache.org/viewvc?rev=1351792&view=rev > > Log: > > Teach 'svn merge' to invoke the interactive conflict resolution callback > > after the merge, not during the merge. > > Don't forget these changes, too... > > Thanks > -g Yup, they're on my list.
