On 05.09.2017 15:58, Daniel Shahaf wrote:
> br...@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
