Stefan Fuhrmann wrote on Sun, Oct 21, 2012 at 19:43:16 +0200: > On Sun, Oct 21, 2012 at 5:55 PM, Daniel Shahaf <d...@daniel.shahaf.name>wrote: > > > Stefan Fuhrmann wrote on Sat, Oct 20, 2012 at 15:14:11 +0200: > > > On Thu, Oct 18, 2012 at 10:22 PM, Daniel Shahaf <d...@daniel.shahaf.name > > >wrote: > > > > > > > Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 02:56:15 +0200: > > > > > On Tue, Oct 16, 2012 at 12:55 AM, Daniel Shahaf < > > d...@daniel.shahaf.name > > > > >wrote: > > > > > > > > > > > stef...@apache.org wrote on Sun, Jun 10, 2012 at 21:34:22 -0000: > > > > > > > Author: stefan2 > > > > > > > Date: Sun Jun 10 21:34:21 2012 > > > > > > > New Revision: 1348666 > > > > > > > > > > > > > > URL: http://svn.apache.org/viewvc?rev=1348666&view=rev > > > > > > > Log: > > > > > > > When handing out node contents, the delta streams don't need > > > > > > > to calculate MD5 checksums as the result will not be used and the > > > > > > > check would be redundant even if it were made. > > > > > > > > > > > > > > Thus, rev the svn_txdelta API to calculate the checksum only > > > > > > > upon specific request and update all callers to use the new API. > > > > > > > > > > > > > ... > > > > > > > Modified: subversion/trunk/subversion/include/svn_delta.h > > > > > > > URL: > > > > > > > > > > > > http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_delta.h?rev=1348666&r1=1348665&r2=1348666&view=diff > > > > > > > > > > > > > > > > > > > ============================================================================== > > > > > > > --- subversion/trunk/subversion/include/svn_delta.h (original) > > > > > > > +++ subversion/trunk/subversion/include/svn_delta.h Sun Jun 10 > > > > 21:34:21 > > > > > > 2012 > > > > > > > @@ -367,10 +367,26 @@ svn_txdelta_md5_digest(svn_txdelta_strea > > > > > > > + * If @a calculate_checksum > > > > > > > + * is set, you may call @ref svn_txdelta_md5_digest to get an > > MD5 > > > > > > checksum > > > > > > > + * for @a target. > > > > > > > * > > > > > > > * Do any necessary allocation in a sub-pool of @a pool. > > > > > > > + * > > > > > > > + * @since New in 1.8. > > > > > > > + */ > > > > > > > +void > > > > > > > +svn_txdelta2(svn_txdelta_stream_t **stream, > > > > > > > + svn_stream_t *source, > > > > > > > + svn_stream_t *target, > > > > > > > + svn_boolean_t calculate_checksum, > > > > > > > > > > > > As of today, only svn_txdelta() and window-test.c pass TRUE for @a > > > > > > calculate_checksum. Do we anticipate needing to pass TRUE For it > > in > > > > any > > > > > > new code? > > > > > > > > > > > > > > > > No, I don't anticipate extra need for that parameter > > > > > neither do I feel that it is useless. How would you > > > > > implement svn_txdelta() without that parameter? > > > > > > > > > > Hm .. wrong question :/ > > > > > > I would remove the parameter from the public API only. Internally we > > > > will define both svn_txdelta() and svn_txdelta2() as one-line wrappers > > > > around some third function which does take an 'svn_boolean_t > > > > calculate_checksum' parameter. > > > > > > > > > > That would only be o.k., if that internal function was internal > > > to libsvn_delta. But we would need to call it from other libs > > > which I see as a strong indicator that others might want this > > > functionality, too. > > > > We wouldn't need to call the internal function from other libs. > > > > static void > > foo_internal(svn_txdelta_stream_t **stream, > > svn_stream_t *source, > > svn_stream_t *target, > > svn_boolean_t calculate_checksum, > > apr_pool_t *pool); > > > > void > > svn_txdelta2(stream, source, target, pool) { > > foo_internal(stream, source, target, FALSE, pool); > > } > > > > void > > svn_txdelta(stream, source, target, pool) { > > foo_internal(stream, source, target, TRUE, pool); > > } > > > > We seem to talk past each other here. My problem > is that the BDB / FSFS libs would need to call the > *deprecated* svn_txdelta because they need the MD5 > checksum.
Odd. I thought that there were no calls to svn_txdelta() left in the codebase, and that all calls to svn_txdelta2() passed FALSE for the new boolean parameter. Anyway, shrug. I think this thread is too long for the prospective benefit (one fewer boolean parameter on a 5-ary function). Cheers Daniel