You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2006/10/11 11:55:32 UTC
[PATCH][merge-tracking] get_merge_info_for_path eliding fails when
grand parent has mergeinfo or grand parent has null mergeinfo.
Hi All,
Find the attached patch and log.
With regards
Kamesh Jayachandran
Re: [PATCH][merge-tracking] get_merge_info_for_path eliding fails
when grand parent has mergeinfo or grand parent has null mergeinfo.
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Kamesh Jayachandran wrote:
> Today I reposted this patch with almost same subject(with Take2
> appended) which has one small correction to this one.
Thanks. I've got the new patch, I just hadn't made it that far down the
queue.
> Hyrum K. Wright wrote:
>> Kamesh Jayachandran wrote:
>>
>>> Hi All,
>>> Find the attached patch and log.
>>>
>>
>> Ping...
>>
>> Has anybody taken a look at Kamesh's patch? If no one comments in the
>> next few days, I'll file it as an issue.
>>
>> Well, I'll probably give is a little longer than normal, given the
>> Summit and all. Just wanted to make sure this wasn't forgotten.
>>
>> Thanks,
>> -Hyrum
>>
>>
>>> ------------------------------------------------------------------------
>>>
>>> [[[
>>> Patch by: Kamesh Jayachandran <ka...@collab.net>
>>>
>>> get_merge_info_path fails in the following cases,
>>> a)if the path is '/a/b/c/d/e' and the repo has the mergeinfo only
>>> for '/a/b/c' and no direct mergeinfo for '/a/b/c/d'.
>>> b)if the path is '/a/b/c/d/e' and the repo has null
>>> mergeinfo(meaning record in mergeinfo_changed no record in mergeinfo)
>>> for '/a/b/c' and no direct mergeinfo for '/a/b/c/d'. The moment
>>> 'null mergeinfo' is encountered eliding has to stop.
>>>
>>> * subversion/libsvn_fs_util/merge-info-sqlite-index.c
>>> (get_merge_info_for_path): No need to flag null mergeinfo with
>>> 'has_no_mergeinfo' they can be better signalled with
>>> NEGATIVE_CACHE_RESULT for the 'path' in the cache hash.
>>> No need of 'pathresult' variable as we can use 'cacheresult' itself.
>>> When we have null merge info signal the same by setting 'cache'
>>> hash for the
>>> key 'path' with NEGATIVE_CACHE_RESULT.
>>> As long as we have a record in 'mergeinfo_changed' for this path
>>> and revision<=rev return no need to recurse further.
>>> Irrespective of 'result' translate the hash keys of 'elided
>>> parent->path'
>>> and set it to cache, so that we don't loose the 'mergeinfo' of
>>> '/a/b/c' during unwinding phase of recursion with path '/a/b/c/d'.
>>> ]]]
>>>
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> Index: subversion/libsvn_fs_util/merge-info-sqlite-index.c
>>> ===================================================================
>>> --- subversion/libsvn_fs_util/merge-info-sqlite-index.c (revision
>>> 21879)
>>> +++ subversion/libsvn_fs_util/merge-info-sqlite-index.c (working
>>> copy)
>>> @@ -400,7 +400,6 @@
>>> sqlite3_stmt *stmt;
>>> int sqlite_result;
>>> sqlite_int64 count;
>>> - svn_boolean_t has_no_mergeinfo = FALSE;
>>>
>>> cacheresult = apr_hash_get(cache, path, APR_HASH_KEY_STRING);
>>> if (cacheresult != 0)
>>> @@ -430,26 +429,23 @@
>>> mergeinfo hash */
>>> if (count > 0)
>>> {
>>> - apr_hash_t *pathresult = NULL;
>>> -
>>> - SVN_ERR(parse_mergeinfo_from_db(db, path, rev, &pathresult,
>>> pool));
>>> - if (pathresult)
>>> + SVN_ERR(parse_mergeinfo_from_db(db, path, rev, &cacheresult,
>>> pool));
>>> + if (cacheresult)
>>> {
>>> if (result)
>>> - apr_hash_set(result, path, APR_HASH_KEY_STRING,
>>> pathresult);
>>> - apr_hash_set(cache, path, APR_HASH_KEY_STRING, pathresult);
>>> + apr_hash_set(result, path, APR_HASH_KEY_STRING,
>>> cacheresult);
>>> + apr_hash_set(cache, path, APR_HASH_KEY_STRING, cacheresult);
>>> }
>>> else
>>> - has_no_mergeinfo = TRUE;
>>> + apr_hash_set(cache, path, APR_HASH_KEY_STRING,
>>> NEGATIVE_CACHE_RESULT);
>>> + return SVN_NO_ERROR;
>>> }
>>>
>>> /* If this path has no mergeinfo, and we are asked to, check our
>>> parent */
>>> - if ((count == 0 || has_no_mergeinfo) && include_parents)
>>> + if ((count == 0) && include_parents)
>>> {
>>> svn_stringbuf_t *parentpath;
>>>
>>> - apr_hash_set(cache, path, APR_HASH_KEY_STRING,
>>> NEGATIVE_CACHE_RESULT);
>>> -
>>> /* It is possible we are already at the root. */
>>> if (strcmp(path, "") == 0)
>>> return SVN_NO_ERROR;
>>> @@ -465,23 +461,20 @@
>>> SVN_ERR(get_merge_info_for_path(db, parentpath->data, rev,
>>> NULL, cache, include_parents,
>>> pool));
>>> - if (result)
>>> + cacheresult = apr_hash_get(cache, parentpath->data,
>>> APR_HASH_KEY_STRING);
>>> + if (cacheresult == NEGATIVE_CACHE_RESULT)
>>> + apr_hash_set(cache, path, APR_HASH_KEY_STRING, NULL);
>>> + else if (cacheresult)
>>> {
>>> /* Now translate the result for our parent to our path */
>>> - cacheresult = apr_hash_get(cache, parentpath->data,
>>> - APR_HASH_KEY_STRING);
>>> - if (cacheresult == NEGATIVE_CACHE_RESULT)
>>> - apr_hash_set(result, path, APR_HASH_KEY_STRING, NULL);
>>> - else if (cacheresult)
>>> - {
>>> - apr_hash_t *translatedhash;
>>> - const char *toappend = &path[parentpath->len + 1];
>>> + apr_hash_t *translatedhash;
>>> + const char *toappend = &path[parentpath->len + 1];
>>>
>>> - append_component_to_paths(&translatedhash, cacheresult,
>>> - toappend, pool);
>>> - apr_hash_set(result, path, APR_HASH_KEY_STRING,
>>> - translatedhash);
>>> - }
>>> + append_component_to_paths(&translatedhash, cacheresult,
>>> + toappend, pool);
>>> + apr_hash_set(cacheresult, path, APR_HASH_KEY_STRING,
>>> translatedhash);
>>> + if (result)
>>> + apr_hash_set(result, path, APR_HASH_KEY_STRING,
>>> translatedhash);
>>> }
>>> }
>>> return SVN_NO_ERROR;
Re: [PATCH][merge-tracking] get_merge_info_for_path eliding fails
when grand parent has mergeinfo or grand parent has null mergeinfo.
Posted by Kamesh Jayachandran <ka...@collab.net>.
Today I reposted this patch with almost same subject(with Take2
appended) which has one small correction to this one.
Please ignore this one.
With regards
Kamesh Jayachandran
Hyrum K. Wright wrote:
> Kamesh Jayachandran wrote:
>
>> Hi All,
>> Find the attached patch and log.
>>
>
> Ping...
>
> Has anybody taken a look at Kamesh's patch? If no one comments in the
> next few days, I'll file it as an issue.
>
> Well, I'll probably give is a little longer than normal, given the
> Summit and all. Just wanted to make sure this wasn't forgotten.
>
> Thanks,
> -Hyrum
>
>
>> ------------------------------------------------------------------------
>>
>> [[[
>> Patch by: Kamesh Jayachandran <ka...@collab.net>
>>
>> get_merge_info_path fails in the following cases,
>> a)if the path is '/a/b/c/d/e' and the repo has the mergeinfo only for
>> '/a/b/c' and no direct mergeinfo for '/a/b/c/d'.
>> b)if the path is '/a/b/c/d/e' and the repo has
>> null mergeinfo(meaning record in mergeinfo_changed no record in mergeinfo)
>> for '/a/b/c' and no direct mergeinfo for '/a/b/c/d'. The moment
>> 'null mergeinfo' is encountered eliding has to stop.
>>
>> * subversion/libsvn_fs_util/merge-info-sqlite-index.c
>> (get_merge_info_for_path):
>> No need to flag null mergeinfo with 'has_no_mergeinfo' they can be better
>> signalled with NEGATIVE_CACHE_RESULT for the 'path' in the cache hash.
>> No need of 'pathresult' variable as we can use 'cacheresult' itself.
>> When we have null merge info signal the same by setting 'cache' hash for the
>> key 'path' with NEGATIVE_CACHE_RESULT.
>> As long as we have a record in 'mergeinfo_changed' for this path and
>> revision<=rev return no need to recurse further.
>> Irrespective of 'result' translate the hash keys of 'elided parent->path'
>> and set it to cache, so that we don't loose the 'mergeinfo' of '/a/b/c'
>> during unwinding phase of recursion with path '/a/b/c/d'.
>>
>> ]]]
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: subversion/libsvn_fs_util/merge-info-sqlite-index.c
>> ===================================================================
>> --- subversion/libsvn_fs_util/merge-info-sqlite-index.c (revision 21879)
>> +++ subversion/libsvn_fs_util/merge-info-sqlite-index.c (working copy)
>> @@ -400,7 +400,6 @@
>> sqlite3_stmt *stmt;
>> int sqlite_result;
>> sqlite_int64 count;
>> - svn_boolean_t has_no_mergeinfo = FALSE;
>>
>> cacheresult = apr_hash_get(cache, path, APR_HASH_KEY_STRING);
>> if (cacheresult != 0)
>> @@ -430,26 +429,23 @@
>> mergeinfo hash */
>> if (count > 0)
>> {
>> - apr_hash_t *pathresult = NULL;
>> -
>> - SVN_ERR(parse_mergeinfo_from_db(db, path, rev, &pathresult, pool));
>> - if (pathresult)
>> + SVN_ERR(parse_mergeinfo_from_db(db, path, rev, &cacheresult, pool));
>> + if (cacheresult)
>> {
>> if (result)
>> - apr_hash_set(result, path, APR_HASH_KEY_STRING, pathresult);
>> - apr_hash_set(cache, path, APR_HASH_KEY_STRING, pathresult);
>> + apr_hash_set(result, path, APR_HASH_KEY_STRING, cacheresult);
>> + apr_hash_set(cache, path, APR_HASH_KEY_STRING, cacheresult);
>> }
>> else
>> - has_no_mergeinfo = TRUE;
>> + apr_hash_set(cache, path, APR_HASH_KEY_STRING, NEGATIVE_CACHE_RESULT);
>> + return SVN_NO_ERROR;
>> }
>>
>> /* If this path has no mergeinfo, and we are asked to, check our parent */
>> - if ((count == 0 || has_no_mergeinfo) && include_parents)
>> + if ((count == 0) && include_parents)
>> {
>> svn_stringbuf_t *parentpath;
>>
>> - apr_hash_set(cache, path, APR_HASH_KEY_STRING, NEGATIVE_CACHE_RESULT);
>> -
>> /* It is possible we are already at the root. */
>> if (strcmp(path, "") == 0)
>> return SVN_NO_ERROR;
>> @@ -465,23 +461,20 @@
>> SVN_ERR(get_merge_info_for_path(db, parentpath->data, rev,
>> NULL, cache, include_parents,
>> pool));
>> - if (result)
>> + cacheresult = apr_hash_get(cache, parentpath->data, APR_HASH_KEY_STRING);
>> + if (cacheresult == NEGATIVE_CACHE_RESULT)
>> + apr_hash_set(cache, path, APR_HASH_KEY_STRING, NULL);
>> + else if (cacheresult)
>> {
>> /* Now translate the result for our parent to our path */
>> - cacheresult = apr_hash_get(cache, parentpath->data,
>> - APR_HASH_KEY_STRING);
>> - if (cacheresult == NEGATIVE_CACHE_RESULT)
>> - apr_hash_set(result, path, APR_HASH_KEY_STRING, NULL);
>> - else if (cacheresult)
>> - {
>> - apr_hash_t *translatedhash;
>> - const char *toappend = &path[parentpath->len + 1];
>> + apr_hash_t *translatedhash;
>> + const char *toappend = &path[parentpath->len + 1];
>>
>> - append_component_to_paths(&translatedhash, cacheresult,
>> - toappend, pool);
>> - apr_hash_set(result, path, APR_HASH_KEY_STRING,
>> - translatedhash);
>> - }
>> + append_component_to_paths(&translatedhash, cacheresult,
>> + toappend, pool);
>> + apr_hash_set(cacheresult, path, APR_HASH_KEY_STRING, translatedhash);
>> + if (result)
>> + apr_hash_set(result, path, APR_HASH_KEY_STRING, translatedhash);
>> }
>> }
>> return SVN_NO_ERROR;
>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][merge-tracking] get_merge_info_for_path eliding fails
when grand parent has mergeinfo or grand parent has null mergeinfo.
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Kamesh Jayachandran wrote:
> Hi All,
> Find the attached patch and log.
Ping...
Has anybody taken a look at Kamesh's patch? If no one comments in the
next few days, I'll file it as an issue.
Well, I'll probably give is a little longer than normal, given the
Summit and all. Just wanted to make sure this wasn't forgotten.
Thanks,
-Hyrum
> ------------------------------------------------------------------------
>
> [[[
> Patch by: Kamesh Jayachandran <ka...@collab.net>
>
> get_merge_info_path fails in the following cases,
> a)if the path is '/a/b/c/d/e' and the repo has the mergeinfo only for
> '/a/b/c' and no direct mergeinfo for '/a/b/c/d'.
> b)if the path is '/a/b/c/d/e' and the repo has
> null mergeinfo(meaning record in mergeinfo_changed no record in mergeinfo)
> for '/a/b/c' and no direct mergeinfo for '/a/b/c/d'. The moment
> 'null mergeinfo' is encountered eliding has to stop.
>
> * subversion/libsvn_fs_util/merge-info-sqlite-index.c
> (get_merge_info_for_path):
> No need to flag null mergeinfo with 'has_no_mergeinfo' they can be better
> signalled with NEGATIVE_CACHE_RESULT for the 'path' in the cache hash.
> No need of 'pathresult' variable as we can use 'cacheresult' itself.
> When we have null merge info signal the same by setting 'cache' hash for the
> key 'path' with NEGATIVE_CACHE_RESULT.
> As long as we have a record in 'mergeinfo_changed' for this path and
> revision<=rev return no need to recurse further.
> Irrespective of 'result' translate the hash keys of 'elided parent->path'
> and set it to cache, so that we don't loose the 'mergeinfo' of '/a/b/c'
> during unwinding phase of recursion with path '/a/b/c/d'.
>
> ]]]
>
>
>
> ------------------------------------------------------------------------
>
> Index: subversion/libsvn_fs_util/merge-info-sqlite-index.c
> ===================================================================
> --- subversion/libsvn_fs_util/merge-info-sqlite-index.c (revision 21879)
> +++ subversion/libsvn_fs_util/merge-info-sqlite-index.c (working copy)
> @@ -400,7 +400,6 @@
> sqlite3_stmt *stmt;
> int sqlite_result;
> sqlite_int64 count;
> - svn_boolean_t has_no_mergeinfo = FALSE;
>
> cacheresult = apr_hash_get(cache, path, APR_HASH_KEY_STRING);
> if (cacheresult != 0)
> @@ -430,26 +429,23 @@
> mergeinfo hash */
> if (count > 0)
> {
> - apr_hash_t *pathresult = NULL;
> -
> - SVN_ERR(parse_mergeinfo_from_db(db, path, rev, &pathresult, pool));
> - if (pathresult)
> + SVN_ERR(parse_mergeinfo_from_db(db, path, rev, &cacheresult, pool));
> + if (cacheresult)
> {
> if (result)
> - apr_hash_set(result, path, APR_HASH_KEY_STRING, pathresult);
> - apr_hash_set(cache, path, APR_HASH_KEY_STRING, pathresult);
> + apr_hash_set(result, path, APR_HASH_KEY_STRING, cacheresult);
> + apr_hash_set(cache, path, APR_HASH_KEY_STRING, cacheresult);
> }
> else
> - has_no_mergeinfo = TRUE;
> + apr_hash_set(cache, path, APR_HASH_KEY_STRING, NEGATIVE_CACHE_RESULT);
> + return SVN_NO_ERROR;
> }
>
> /* If this path has no mergeinfo, and we are asked to, check our parent */
> - if ((count == 0 || has_no_mergeinfo) && include_parents)
> + if ((count == 0) && include_parents)
> {
> svn_stringbuf_t *parentpath;
>
> - apr_hash_set(cache, path, APR_HASH_KEY_STRING, NEGATIVE_CACHE_RESULT);
> -
> /* It is possible we are already at the root. */
> if (strcmp(path, "") == 0)
> return SVN_NO_ERROR;
> @@ -465,23 +461,20 @@
> SVN_ERR(get_merge_info_for_path(db, parentpath->data, rev,
> NULL, cache, include_parents,
> pool));
> - if (result)
> + cacheresult = apr_hash_get(cache, parentpath->data, APR_HASH_KEY_STRING);
> + if (cacheresult == NEGATIVE_CACHE_RESULT)
> + apr_hash_set(cache, path, APR_HASH_KEY_STRING, NULL);
> + else if (cacheresult)
> {
> /* Now translate the result for our parent to our path */
> - cacheresult = apr_hash_get(cache, parentpath->data,
> - APR_HASH_KEY_STRING);
> - if (cacheresult == NEGATIVE_CACHE_RESULT)
> - apr_hash_set(result, path, APR_HASH_KEY_STRING, NULL);
> - else if (cacheresult)
> - {
> - apr_hash_t *translatedhash;
> - const char *toappend = &path[parentpath->len + 1];
> + apr_hash_t *translatedhash;
> + const char *toappend = &path[parentpath->len + 1];
>
> - append_component_to_paths(&translatedhash, cacheresult,
> - toappend, pool);
> - apr_hash_set(result, path, APR_HASH_KEY_STRING,
> - translatedhash);
> - }
> + append_component_to_paths(&translatedhash, cacheresult,
> + toappend, pool);
> + apr_hash_set(cacheresult, path, APR_HASH_KEY_STRING, translatedhash);
> + if (result)
> + apr_hash_set(result, path, APR_HASH_KEY_STRING, translatedhash);
> }
> }
> return SVN_NO_ERROR;