Hey, Paul. Overall, I think the change makes sense. I've given some inline review below. (This is based mostly on a reading of the patch itself, not a reading of the patched source files.)
On 01/18/2011 04:44 PM, Paul Burba wrote: > Index: subversion/mod_dav_svn/reports/update.c > =================================================================== > --- subversion/mod_dav_svn/reports/update.c (revision 1060528) > +++ subversion/mod_dav_svn/reports/update.c (working copy) [...] > @@ -413,8 +407,16 @@ > return SVN_NO_ERROR; > > /* ### ack! binary names won't float here! */ > - /* If this is a copied file/dir, we can have removed props. */ > - if (baton->removed_props && (! baton->added || baton->copyfrom)) > + /* If this is a copied file/dir, we can have removed props. > + > + Old features never die: 1.7+ clients don't require this block because > + they never ask for copyfrom information from the server when adding > + files created by a copy, but 1.5-1.6 clients will ask for it so we > + have to keep sending it. > + > + See http://svn.haxx.se/dev/archive-2010-09/0265.shtml and > + http://subversion.tigris.org/issues/show_bug.cgi?id=3711. */ 1.7's RA interface still allows clients to request copyfrom information, and we never know if we might later use this codepath again. So I'm not sure it's accurate to claim that "1.7+ clients don't require this block". Maybe TortoiseSVN is using it? Maybe our repos diff code will use it (I seem to recall someone talking about doing this)? I think this whole comment change can be reverted. > - SVN_ERR(dav_svn__brigade_puts(baton->uc->bb, baton->uc->output, > "<S:prop>")); > - > - /* Both modern and non-modern clients need the checksum... */ > - if (baton->text_checksum) > - { > - SVN_ERR(dav_svn__brigade_printf(baton->uc->bb, baton->uc->output, > - "<V:md5-checksum>%s</V:md5-checksum>", > - baton->text_checksum)); > - } > - This removal doesn't seem right. There are two kinds of checksum sent over the wire in this REPORT response: 1. a "base-checksum", which is the checksum of the file against which a content delta is being transmitted (so the client can verify that it's about to apply that delta against the right base), and 2. a "text-checksum", which is the checksum of final text content (either as retrieved via fulltext or via delta application to a base. Maybe I'm overlooking it, but it seems you're no longer transmitting the text-checksum any longer. -- C. Michael Pilato <cmpil...@collab.net> CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
