On Thu, Feb 9, 2012 at 12:24 PM, Daniel Shahaf <danielsh_at_elego.de> wrote:
> hwright_at_apache.org wrote on Wed, Feb 08, 2012 at 01:55:55 -0000:
>> Author: hwright
>> Date: Wed Feb 8 01:55:54 2012
>> New Revision: 1241733
>>
>> URL: http://svn.apache.org/viewvc?rev=1241733&view=rev
>> Log:
>> Ev2 shims: Make sure that if we are providing content to alter_file(), we
>> are also providing a checksum.
>>
>> * subversion/libsvn_delta/compat.c
>> (path_checksum_args): Remove checksum member.
>> (process_actions): Checksum the content file.
>> (ev2_apply_textdelta): Don't checksum the output stream.
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_delta/compat.c
>>
>> Modified: subversion/trunk/subversion/libsvn_delta/compat.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/compat.c?rev=1241733&r1=1241732&r2=1241733&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_delta/compat.c (original)
>> +++ subversion/trunk/subversion/libsvn_delta/compat.c Wed Feb 8 01:55:54 2012
>> @@ -203,7 +203,6 @@ struct path_checksum_args
>> {
>> const char *path;
>> svn_revnum_t base_revision;
>> - svn_checksum_t *checksum;
>> };
>>
>> static svn_error_t *
>> @@ -340,9 +339,10 @@ process_actions(void *edit_baton,
>> /* We can only set text on files. */
>> kind = svn_node_file;
>>
>> + SVN_ERR(svn_io_file_checksum2(&checksum, pca->path,
>> + svn_checksum_sha1, scratch_pool));
>> SVN_ERR(svn_stream_open_readonly(&contents, pca->path,
>> scratch_pool, scratch_pool));
>
> So, you first open the file and read it through to checksum it, and then
> open it again to read it through as a stream.
>
> That's because right now add_file()/alter_file() require the checksum to
> be provided up front.
>
> Would it make sense to relax that requirement? On the one hand,
> many receivers don't need to know the checksum until after they've
> received the whole file (eg, the fs txn process and the pristine store);
> on the other hand, having the checksum will allow in some cases not
> transmitting the file over the wire at all, and will allow for
> error-checking the transmission in other cases.
Agreed that the current approach taken by the shim is wasteful, though
other attempts were a bit dodgy, and this one Just Works. If there
are better ways to get this checksum, let's use 'em.
I'm not certain about relaxing the requirement just yet. I'll have to
think about the design implications.
(I'll also note that we actually *do* have a checksum by this point,
only it is the md5 provided by close_file(), and Ev2 uses sha1s
exclusively, so we have to recalculate. I suspect this will be a
somewhat common issue while we use a mixed shim environment.)
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Received on 2012-02-09 19:55:22 CET