On Mon, Mar 29, 2010 at 13:10, Julian Foad <julian.f...@wandisco.com> wrote: > The attached patch is an attempt to close the gap between "transmit text > deltas" and commit post-processing, by passing the temporary new text > base file paths around. It succeeds in that, at least it looks right > and passes the test suite.
except for that printf() :-P > The part of this patch that I haven't finished is with back-compat of > svn_wc__process_committed_internal(), and what is the difference between > its 'queue' parameter and its checksum/recurse/no_unlock/etc. > parameters, being values which are alternatively available in the queue. > > svn_wc__process_committed_internal() is called by > svn_wc_process_committed_queue2() which passes the 'queue' param, and > also by the deprecated svn_wc_process_committed4() with QUEUE=NULL. I > had been assuming that if QUEUE==NULL then all the parameters that are > available in the queue (checksum for one) are not available, but that's > not how the back-compat wrapper wants to work. I'll need to fix that. > I think the right thing to do is to re-write the wrapper > (svn_wc_process_committed4()) to construct a new queue with one item and > pass that, and stop having the other parameters (checksum etc.) passed > as separate parameters. I'll look at that tomorrow. I may already have > committed changes that break that back-compat; I'll check. I think the answer for above may be to call internal functions with a single cqi, rather than a full queue. Would that work well? I do agree that passing QUEUE==NULL is troublesome, given all the checks in this patch. > The internal recursion of svn_wc__process_committed_internal() is > confusing me: it passes NO_UNLOCK=TRUE to itself when recursing, > regardless of what no_unlock value was passed in or what is in the queue > item. Yet it passes the received value of KEEP_CHANGELIST. For the > NEW_DAV_CACHE and CHECKSUM arguments it passes NULL, which is half > explained by them only having meaning in connection with a single node: > the same ones cannot be applicable to a child node. Right. I have no idea why the NO_UNLOCK=TRUE is present. I would suggest a bit of archeology to try and determine the reason. The cache/checksum as NULL are valid, tho I don't know how we *do* get a valid checksum for those committed children (since we don't, necessarily, transmit any text for them). We kinda need a checksum... >... Cheers, -g
