[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1222521 - /subversion/trunk/subversion/libsvn_wc/wc_db.c

From: Konstantin Kolinko <knst.kolinko_at_gmail.com>
Date: Fri, 23 Dec 2011 10:40:59 +0400

2011/12/23 Hyrum K Wright <hyrum.wright_at_wandisco.com>:
> On Thu, Dec 22, 2011 at 6:31 PM,  <stsp_at_apache.org> wrote:
>> Author: stsp
>> Date: Fri Dec 23 00:31:51 2011
>> New Revision: 1222521
>>
>> URL: http://svn.apache.org/viewvc?rev=1222521&view=rev
>> Log:
>> Turn another wc-db assertion people are reporting into a normal error.
>>
>> * subversion/libsvn_wc/wc_db.c
>>  (read_children_info): If a node from an unexpected repository is found,
>>   don't assert but print an informative error message.
>>
>> Modified:
>>    subversion/trunk/subversion/libsvn_wc/wc_db.c
>>
>> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1222521&r1=1222520&r2=1222521&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
>> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Fri Dec 23 00:31:51 2011
>> @@ -7231,7 +7231,7 @@ read_children_info(void *baton,
>>   svn_boolean_t have_row;
>>   const char *repos_root_url = NULL;
>>   const char *repos_uuid = NULL;
>> -  apr_int64_t last_repos_id;
>> +  apr_int64_t last_repos_id = INVALID_REPOS_ID;
>>   apr_hash_t *nodes = rci->nodes;
>>   apr_hash_t *conflicts = rci->conflicts;
>>   apr_pool_t *result_pool = rci->result_pool;
>> @@ -7299,20 +7299,54 @@ read_children_info(void *baton,
>>             }
>>           else
>>             {
>> +              const char *last_repos_root_url = NULL;
>> +              const char *last_repos_uuid = NULL;
>> +
>>               apr_int64_t repos_id = svn_sqlite__column_int64(stmt, 1);
>> -              if (!repos_root_url)
>> +              if (!repos_root_url ||
>> +                  (last_repos_id != INVALID_REPOS_ID &&
>> +                   repos_id != last_repos_id))
>>                 {
>> +                  last_repos_root_url = repos_root_url;
>> +                  last_repos_uuid = repos_uuid;
>>                   err = fetch_repos_info(&repos_root_url, &repos_uuid,
>>                                          wcroot->sdb, repos_id, result_pool);
>>                   if (err)
>>                     SVN_ERR(svn_error_compose_create(err,
>> -                                                     svn_sqlite__reset(stmt)));
>> -                  last_repos_id = repos_id;
>> +                                                 svn_sqlite__reset(stmt)));
>>                 }
>>
>> +              if (last_repos_id == INVALID_REPOS_ID)
>> +                last_repos_id = repos_id;
>> +
>>               /* Assume working copy is all one repos_id so that a
>>                  single cached value is sufficient. */
>> -              SVN_ERR_ASSERT(repos_id == last_repos_id);
>> +              if (repos_id != last_repos_id)
>> +                return svn_error_createf(
>> +                         SVN_ERR_WC_DB_ERROR, NULL,
>> +                         _("The node '%s' comes from unexpected repository "
>> +                           "(ID '%" APR_INT64_T_FMT "'%s%s, expected ID '%"
>> +                           APR_INT64_T_FMT"'%s%s); this could be a "
>> +                           "misconfigured file-external which points to a "
>> +                           "foreign repository"),
>> +                         child_relpath, repos_id,
>> +                         repos_root_url
>> +                           ? apr_psprintf(scratch_pool, ", URL '%s'",
>> +                                          repos_root_url)
>> +                           : "",
>> +                         repos_uuid
>> +                           ? apr_psprintf(scratch_pool, ", UUID '%s'",
>> +                                          repos_uuid)
>> +                           : "",
>> +                         last_repos_id,
>> +                         last_repos_root_url
>> +                           ? apr_psprintf(scratch_pool, ", URL '%s'",
>> +                                          last_repos_root_url)
>> +                           : "",
>> +                         last_repos_uuid
>> +                           ? apr_psprintf(scratch_pool, ", UUID '%s'",
>> +                                          last_repos_uuid)
>> +                           : "");
>
> Yikes!  I think this wins the prize for "longest function call".
> Perhaps a helper function or at least some temporary variables would
> be in order?
>

I would say that the message is not friendly to translators.

Those repos_id, last_repos_id -- are they needed in a message
displayed to an user? I would say that URL and UUID should be enough.

I think it would be better to put them into the main message template string.

                         _("The node '%s' comes from unexpected repository "
                           "(URL '%s', UUID '%s', expected URL '%s',
UUID '%s'); "
                           "this could be a "
                           "misconfigured file-external which points to a "
                           "foreign repository"),
    (...)
                         repos_root_url ? repos_root_url : "",

>>               child->repos_root_url = repos_root_url;
>>               child->repos_uuid = repos_uuid;
>>             }
>>

Best regards,
Konstantin Kolinko
Received on 2011-12-23 07:41:38 CET

This is an archived mail posted to the Subversion Dev mailing list.