[prev in list] [next in list] [prev in thread] [next in thread] 

List:       subversion-dev
Subject:    Re: svn commit: r1807225 - in /subversion/branches/better-pristines/subversion/libsvn_wc: lock.c upg
From:       Branko_Čibej <brane () apache ! org>
Date:       2017-09-05 18:52:57
Message-ID: ee983afc-c365-f3df-7178-d71dd87f9562 () apache ! org
[Download RAW message or body]

On 05.09.2017 15:58, Daniel Shahaf wrote:
> brane@apache.org wrote on Mon, 04 Sep 2017 13:58 +0000:
>> Author: brane
>> Date: Mon Sep  4 13:58:26 2017
>> New Revision: 1807225
>>
>> URL: http://svn.apache.org/viewvc?rev=1807225&view=rev
>> Log:
>> Begin adding support for multiple working copy formats.
>>
>> Instead of supporting just one format, introduce a current formaat
>> (the default for new working copies) and a lowest supported format,
>> and change the way new working copies are created: instead of the
>> base schema defining the current format, it defines the lowest
>> supporting format and a series of format updates are performed
>> to bring it to the current shape.
>> +++ subversion/branches/better-pristines/subversion/libsvn_wc/lock.c Mon Sep  4 13:58:26 2017
>> @@ -566,10 +566,12 @@ open_single(svn_wc_adm_access_t **adm_ac
>>      }
>>    SVN_ERR(err);
>>  
>> -  /* The format version must match exactly. Note that wc_db will perform
>> -     an auto-upgrade if allowed. If it does *not*, then it has decided a
>> -     manual upgrade is required and it should have raised an error.  */
>> -  SVN_ERR_ASSERT(wc_format == SVN_WC__VERSION);
>> +  /* The format version must be in the supported version range. Note
>> +     that wc_db will perform an auto-upgrade if allowed. If it does
>> +     *not*, then it has decided a manual upgrade is required and it
>> +     should have raised an error.  */
> This is a useful comment for people who work on the upgrade logic.  It
> might, therefore, be useful to copy/reference it from svn_wc*upgrade*()?

Ack, thanks.

>> +  SVN_ERR_ASSERT(SVN_WC__SUPPORTED_VERSION <= wc_format
>> +                 && wc_format <= SVN_WC__VERSION);
>> +++ subversion/branches/better-pristines/subversion/libsvn_wc/upgrade.c Mon Sep  4 13:58:26 2017
>> @@ -2202,6 +2155,102 @@ svn_wc__upgrade_sdb(int *result_format,
>> +svn_error_t *
>> +svn_wc__update_schema(int *result_format,
>> +                      const char *wcroot_abspath,
>> +                      svn_sqlite__db_t *sdb,
>> +                      int start_format,
>> +                      int target_format,
>> +                      apr_pool_t *scratch_pool)
>> +{
>> +  struct bump_baton bb;
>> +  bb.wcroot_abspath = wcroot_abspath;
>> +
>> +  /* Repeatedly upgrade until the target format version is reached. */
>> +  for (*result_format = start_format;
>> +       *result_format < target_format;)
>> +    {
>> +      switch (*result_format)
>> +        {
>> +          case 19:
>> +            SVN_ERR(svn_sqlite__with_lock(sdb, bump_to_20, &bb, scratch_pool));
>> +            *result_format = 20;
>> +            break;
>> +
>> +          case 20:
>> +            SVN_ERR(svn_sqlite__with_lock(sdb, bump_to_21, &bb, scratch_pool));
>> +            *result_format = 21;
>> +            break;
>> +
>> +          case 21:
>> +            SVN_ERR(svn_sqlite__with_lock(sdb, bump_to_22, &bb, scratch_pool));
>> +            *result_format = 22;
>> +            break;
> This block is very repetitive.  I wonder if we should use:
>
>     #define FOO(twenty_two) \
>       case (twenty_two)-1: \
>         SVN_ERR(svn_sqlite__with_lock(sdb, bump_to_##twenty_two, &bb, scratch_pool)); \
>         *result_format = twenty_two; \
>         break;
>
>     switch (*result_format)
>       {
>         FOO(20);
>         FOO(21);
>         FOO(22);
>         ⋮
>       }

It wasn't originally; before my changes, these cases were all
fall-through and the stats1 table insertion was within the switch block
... which was just a bit too much of a good thing. :)

I think I might take your advice and add the boilerplate macro, if only
to avoid future typos.

-- Brane
[prev in list] [next in list] [prev in thread] [next in thread]