You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pt...@gmail.com> on 2012/02/24 16:17:55 UTC

Re: svn commit: r1293229 - /subversion/trunk/subversion/libsvn_repos/log.c

On Fri, Feb 24, 2012 at 7:47 AM,  <ph...@apache.org> wrote:
> Author: philip
> Date: Fri Feb 24 12:47:37 2012
> New Revision: 1293229
>
> URL: http://svn.apache.org/viewvc?rev=1293229&view=rev
> Log:
> Fix an "svn log -g" bug that caused occasional missing "Merged via"
> notifications when a revision had changed paths both inside and
> outside the target.  This was exposed by the APR hash order changes
> and caused log_tests.py 32 to fail at random.
>
> * subversion/libsvn_repos/log.c
>  (send_log): Reduce scope of variable so it is reinitialised on each loop
>  iteration.
>
> Modified:
>    subversion/trunk/subversion/libsvn_repos/log.c
>
> Modified: subversion/trunk/subversion/libsvn_repos/log.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/log.c?rev=1293229&r1=1293228&r2=1293229&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/log.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/log.c Fri Feb 24 12:47:37 2012
> @@ -1152,7 +1152,6 @@ send_log(svn_revnum_t rev,
>       && log_target_history_as_mergeinfo
>       && apr_hash_count(log_target_history_as_mergeinfo))
>     {
> -      svn_boolean_t path_is_in_history = FALSE;
>       apr_hash_index_t *hi;
>       apr_pool_t *subpool = svn_pool_create(pool);
>
> @@ -1165,6 +1164,7 @@ send_log(svn_revnum_t rev,
>            hi;
>            hi = apr_hash_next(hi))
>         {
> +          svn_boolean_t path_is_in_history = FALSE;
>           const char *changed_path = svn__apr_hash_index_key(hi);
>           apr_hash_index_t *hi2;
>           apr_pool_t *inner_subpool = svn_pool_create(subpool);

Ah, nice find Philip!  I was looking at this last night with less success.

Re: svn commit: r1293229 - /subversion/trunk/subversion/libsvn_repos/log.c

Posted by Paul Burba <pt...@gmail.com>.
On Fri, Feb 24, 2012 at 12:08 PM, Philip Martin
<ph...@wandisco.com> wrote:
> Bert Huijben <be...@vmoo.com> writes:
>
>> Has this more impact than just svn log -g?
>>
>> Is this used internally in the merge calculation?

The merge tracking logic does make use of svn_ra_get_log2, which of
course means that svn_repos_get_logs4 and ultimately send_log is hit.
All of the merge calls however are with
svn_ra_get_log2(...include_merged_revisions=FALSE...), which means
that offending block fixed in r1293229 is never executed.

Paul

> I don't think so, only things that call svn_repos_get_logs4 are
> affected.
>
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com

Re: svn commit: r1293229 - /subversion/trunk/subversion/libsvn_repos/log.c

Posted by Philip Martin <ph...@wandisco.com>.
Bert Huijben <be...@vmoo.com> writes:

> Has this more impact than just svn log -g?
>
> Is this used internally in the merge calculation?

I don't think so, only things that call svn_repos_get_logs4 are
affected.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: svn commit: r1293229 - /subversion/trunk/subversion/libsvn_repos/log.c

Posted by Bert Huijben <be...@vmoo.com>.
On Fri, Feb 24, 2012 at 4:17 PM, Paul Burba <pt...@gmail.com> wrote:
> On Fri, Feb 24, 2012 at 7:47 AM,  <ph...@apache.org> wrote:
>> Author: philip
>> Date: Fri Feb 24 12:47:37 2012
>> New Revision: 1293229
>>
>> URL: http://svn.apache.org/viewvc?rev=1293229&view=rev
>> Log:
>> Fix an "svn log -g" bug that caused occasional missing "Merged via"
>> notifications when a revision had changed paths both inside and
>> outside the target.  This was exposed by the APR hash order changes
>> and caused log_tests.py 32 to fail at random.
>>
>> * subversion/libsvn_repos/log.c
>>  (send_log): Reduce scope of variable so it is reinitialised on each loop
>>  iteration.
>>
>> Modified:
>>    subversion/trunk/subversion/libsvn_repos/log.c
>>
>> Modified: subversion/trunk/subversion/libsvn_repos/log.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/log.c?rev=1293229&r1=1293228&r2=1293229&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_repos/log.c (original)
>> +++ subversion/trunk/subversion/libsvn_repos/log.c Fri Feb 24 12:47:37 2012
>> @@ -1152,7 +1152,6 @@ send_log(svn_revnum_t rev,
>>       && log_target_history_as_mergeinfo
>>       && apr_hash_count(log_target_history_as_mergeinfo))
>>     {
>> -      svn_boolean_t path_is_in_history = FALSE;
>>       apr_hash_index_t *hi;
>>       apr_pool_t *subpool = svn_pool_create(pool);
>>
>> @@ -1165,6 +1164,7 @@ send_log(svn_revnum_t rev,
>>            hi;
>>            hi = apr_hash_next(hi))
>>         {
>> +          svn_boolean_t path_is_in_history = FALSE;
>>           const char *changed_path = svn__apr_hash_index_key(hi);
>>           apr_hash_index_t *hi2;
>>           apr_pool_t *inner_subpool = svn_pool_create(subpool);
>
> Ah, nice find Philip!  I was looking at this last night with less success.
+1 on the nice find :)

Has this more impact than just svn log -g?

Is this used internally in the merge calculation?

   Bert