On 1/28/2011 12:59 AM, Daniel Shahaf wrote:
> Is this a fix or a work-around? i.e., if you set MAX_OPEN_HISTORIES to,
> say, 2, can you then reproduce the failure you saw before?
>
> It's just not clear to me whether MAX_OPEN_HISTORIES is in fact an
> assumption that "no file will be 'svn mv'd more than 32 times" or
> similar.
Previous to r1028108, the check for error status was outside the whole
get_path_histories function. I simply added it back in to the other
place in this function that can return errors. It does appear to be
trying to open the wrong file name after MAX_OPEN_HISTORIES has been
reached. So you are correct, this is more of a "restore to previous
behavior" patch instead of a true "fix"... However, previously log -g
completed successfully, and currently it aborts the connection with a
chuck size error. I verified the same data is now returned as old
1.6.13 versions but did not validate the data was not missing any merge
information or if my test case merge information is corrupted somehow.
(I do know the merge info has not been hand modified.)
I did not try reducing MAX_OPEN_HISTORIES to make this easier to
reproduce, since I already had a complex, but always failing test case.
Doing so may make it easier for someone more familiar to this code to
figure out more details if needed.
Kevin R.
> Kevin Radke wrote on Wed, Jan 26, 2011 at 16:59:54 -0600:
>> This also needs a back-port to the 1.6.x branch.
>>
>> [[[
>> Fix issue #3789: Correctly ignore missing locations when a renamed file
>> has more than MAX_OPEN_HISTORIES.
>>
>> * subversion/libsvn_repos/log.c
>> (get_path_histories): Ignore more bogus repository locations to restore
>> pre-1.6.15 log -g behavior. This fixes client chunk
>> errors introduced by r1028108.
>> ]]]
>>
>>
>> Index: subversion/libsvn_repos/log.c
>> ===================================================================
>> --- subversion/libsvn_repos/log.c (revision 1063904)
>> +++ subversion/libsvn_repos/log.c (working copy)
>> @@ -1052,6 +1052,7 @@
>> {
>> svn_fs_root_t *root;
>> apr_pool_t *iterpool;
>> + svn_error_t *err;
>> int i;
>>
>> /* Create a history object for each path so we can walk through
>> @@ -1093,7 +1094,6 @@
>>
>> if (i< MAX_OPEN_HISTORIES)
>> {
>> - svn_error_t *err;
>> err = svn_fs_node_history(&info->hist, root, this_path, pool);
>> if (err
>> && ignore_missing_locations
>> @@ -1115,10 +1115,20 @@
>> info->newpool = NULL;
>> }
>>
>> - SVN_ERR(get_history(info, fs,
>> - strict_node_history,
>> - authz_read_func, authz_read_baton,
>> - hist_start, pool));
>> + err = get_history(info, fs,
>> + strict_node_history,
>> + authz_read_func, authz_read_baton,
>> + hist_start, pool);
>> + if (err
>> +&& ignore_missing_locations
>> +&& (err->apr_err == SVN_ERR_FS_NOT_FOUND ||
>> + err->apr_err == SVN_ERR_FS_NOT_DIRECTORY ||
>> + err->apr_err == SVN_ERR_FS_NO_SUCH_REVISION))
>> + {
>> + svn_error_clear(err);
>> + continue;
>> + }
>> + SVN_ERR(err);
>> APR_ARRAY_PUSH(*histories, struct path_info *) = info;
>> }
>> svn_pool_destroy(iterpool);
Received on 2011-01-28 14:46:25 CET