On 02/06/2012 03:21 PM, Hyrum K Wright wrote: > On Mon, Feb 6, 2012 at 1:26 PM, Greg Stein <gst...@gmail.com> wrote: >> On Mon, Feb 6, 2012 at 12:49, Hyrum K Wright <hyrum.wri...@wandisco.com> >> wrote: >>> ... >>>> Yeah, sounds like we're in a tough spot here. The checksums in Ev1 exist >>>> only as sanity checks -- they definitely are NOT the primary selection >>>> mechanism for the editor implementation's base text. >> >> Right. This is a key point, and Hyrum's earlier emails adopted a tone >> of using the base checksum as a key. The real semantic behind that >> checksum is, "I'm going to send you a delta against some text base >> that we've negotiated or assumed through an out-of-band communication, >> and to verify that our communication is correct, that text base should >> have CHECKSUM." >> >> Ev2 gets rid of all that hand-wave magic and leaves deltafication to >> other layers (notably, RA layers sending stuff over the wire; ra_local >> will not need to deltify at all). >> >>>> I assume we still have a valid checksum to pass to close_file() (the >>>> checksum of the post-edit fulltext for that file), right? It's just the >>>> pre-edit base checksum that's the problem? >>> >>> Correct. >>> >>> r1241097 relaxes the checks by special-casing the checksum of the >>> empty stream (as discussed elsethread). This addresses the immediate >>> issue, and I think the generalized case can be punted toward the >>> individual ra-layers long-term. >> >> Note that this will break third-party Ev1 receivers (since they assume >> something other than the empty stream). You happened to fix *ours*, >> but not theirs. > > Receivers (such as ours) are making unreasonable assumptions, in my > opinion. The docs for apply_textdelta() read thusly: > > * @a base_checksum is the hex MD5 digest for the base text against > * which the delta is being applied; it is ignored if NULL, and may > * be ignored even if not NULL. If it is not ignored, it must match > * the checksum of the base text against which svndiff data is being > * applied; if it does not, @c apply_textdelta or the @a *handler call > * which detects the mismatch will return the error > * SVN_ERR_CHECKSUM_MISMATCH (if there is no base text, there may > * still be an error if @a base_checksum is neither NULL nor the hex > * MD5 checksum of the empty string). > > All the docs guarantee is that this checksum must match the base > checksum against which the delta is being applied. They don't assume > that the receiver has the content, nor that it will even pay attention > to the checksums themselves. If the receiver has done some kind of > backdoor secret handshake that's outside the scope of the delta > editor, and it should (reasonably) not care. > > I maintain that it is still legal (though perhaps not very useful) for > the base checksum supplied by apply_textdelta() to be whatever > apply_textdelta() wants it to be.
Sorry, Hyrum, but I sincerely believe you are trying to bend this API to your will just to work around a tough engineering task. :-) If you read the whole docstring for apply_textdelta, you find that it begins like so: /** Apply a text delta, yielding the new revision of a file. * * @a file_baton indicates the file we're creating or updating, and the * ancestor file on which it is based; it is the baton set by some * prior @c add_file or @c open_file callback. Now, can you *really* convince yourself that anyone ever intended for the "base text" to be something other than the text associated with the "ancestor file on which [this new revision of the file] is based"? What value would there be in even passing along a base_checksum that doesn't match that of the file_baton's to-be-changed text? -- C. Michael Pilato <cmpil...@collab.net> CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
