Re: svn commit: r40473 - trunk/subversion/libsvn_wc

On Nov 19, 2009, at 6:07 AM, Bert Huijben wrote:

>
>
>> -----Original Message-----
>> From: Hyrum K. Wright [mailto:hyrum_at_hyrumwright.org]
>> Sent: donderdag 12 november 2009 7:10
>> To: Greg Stein; Bert Huijben
>> Cc: dev_at_subversion.tigris.org
>> Subject: Re: svn commit: r40473 - trunk/subversion/libsvn_wc
>>
>> On Nov 11, 2009, at 11:56 PM, Hyrum K. Wright wrote:
>>
>>> Author: hwright
>>> Date: Wed Nov 11 21:56:37 2009
>>> New Revision: 40473
>>>
>>> Log:
>>> Add a working copy upgrade migration routine for moving properties
>> from disk
>>> to the wc_db.
>>>
>>> This commit does *not* change the current version number of the
>> working copy,
>>> so this code will never be run. In fact, even with the current
>> version number
>>> bumped, I still can't get the code to run, for reasons I'll
>> pontificate on
>>> at a later date. I'm committing this now to get some review.
>>
>> Greg,
>> Although this code is called from svn_wc__upgrade_sdb(), that function
>> never gets called. We're not making any schema changes in the 15 -> 16
>> bump, and the sqlite version updater just goes right along with that.
>> By the time we get to the call to svn_wc__upgrade_sdb() inside of
>> create_wcroot(), it turns out that the sqlite format has already been
>> bumped to 16, so we don't even make the call. Is this the intended
>> behavior? If so, how can I get that function called so we can migrate
>> the properties correctly?
>>
>>> Once we feel confident that the upgrade works as intended, I plan to
>> commit
>>> the rest of the patch: bumping the working copy version and using in-
>> database
>>> properties exclusively.
>>
>> Bert,
>> I think this does the props migration correctly. I've got the rest of
>> the cutover patch queued up in my working copy, just waiting
>> verification of this patch. When you get a minute, could you take a
>> look at the migrate_props() function and tell me if it matches what
>> you'd expect?
>>
>> Thanks,
>> -Hyrum
>>
>>>
>>> * subversion/libsvn_wc/upgrade.c
>>> (PROPS_SUBDIR, PROP_BASE_SUBDIR, TEMP_DIR): Some useful defines.
>>> (migrate_props): New.
>>> (bump_database_to_16): Renamed to...
>>> (bump_database_to_17): ...this.
>>> (bump_to_16): Renamed to...
>>> (bump_to_17): ...this.
>>> (svn_wc__upgrade_sdb): Call the new properties migration code when
>> moving
>>> from format 15 -> 16.
>>>
>>> Modified:
>>> trunk/subversion/libsvn_wc/upgrade.c
>>>
>>> Modified: trunk/subversion/libsvn_wc/upgrade.c
>>> URL:
>> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/upgrade.c?p
>> athrev=40473&r1=40472&r2=40473
>>>
>> =======================================================================
>> =======
>>> --- trunk/subversion/libsvn_wc/upgrade.c Wed Nov 11 14:53:19 2009
>> (r40472)
>>> +++ trunk/subversion/libsvn_wc/upgrade.c Wed Nov 11 21:56:37 2009
>> (r40473)
>>> @@ -49,6 +49,12 @@
>>> #define WCPROPS_FNAME_FOR_DIR "dir-wcprops"
>>> #define WCPROPS_ALL_DATA "all-wcprops"
>>>
>>> +/* Old property locations. */
>>> +#define PROPS_SUBDIR "props"
>>> +#define PROP_BASE_SUBDIR "prop-base"
>>> +
>>> +#define TEMP_DIR "tmp"
>>> +
>>> /* Old data files that we no longer need/use. */
>>> #define ADM_README "README.txt"
>>> #define ADM_EMPTY_FILE "empty-file"
>>> @@ -725,10 +731,141 @@ migrate_locks(const char *wcroot_abspath
>>> }
>>>
>>>
>>> +static svn_error_t *
>>> +migrate_props(const char *wcroot_abspath,
>>> + svn_sqlite__db_t *sdb,
>>> + apr_pool_t *scratch_pool)
>>> +{
>>> + /* General logic here: iterate over all the immediate children of
>> the root
>>> + (since we aren't yet in a centralized system), and for any
>> properties that
>>> + exist, map them as follows:
>>> +
>>> + if (node is replaced):
>>> + revert -> BASE
>>> + working -> ACTUAL
>
> In this specific case the base properties go to WORKING. This is the only
> case where we had three sets of properties
>
> In this specific state reverting can be two things. Undoing the add, or
> undoing the delete.

Thanks for taking a look, I've updated this in r882164.

>>> + else if (prop pristine is working [as defined in props.c] ):
>>> + base -> WORKING
>>> + working -> ACTUAL
>>> + else:
>>> + base -> BASE
>>> + working -> ACTUAL
>>> + */
>>> + const apr_array_header_t *children;
>>> + apr_pool_t *iterpool;
>>> + const char *props_dirpath;
>>> + const char *props_base_dirpath;
>>> + svn_wc__db_t *db;
>>> + int i;
>>> +
>>> + /* *sigh* We actually want to use wc_db APIs to read data, but we
>> aren't
>>> + provided a wc_db, so open one. */
>>> + SVN_ERR(svn_wc__db_open(&db, svn_wc__db_openmode_default, NULL,
>> FALSE, TRUE,
>>> + scratch_pool, scratch_pool));
>
>
> Note from the future: This call could be one of the cases where the database
> is upgraded unexpectedly.

So, this could lead to possible infinite recursion? Yikes.

> Sorry, my head is not clear enough for a full review right now. (Code looks
> ok, but don't fully trust my judgment until I'm off the medicines)
>
> Bert
>>> +
>>> + /* Go find all the children of the wcroot. */
>>> + SVN_ERR(svn_wc__db_read_children(&children, db, wcroot_abspath,
>>> + scratch_pool, scratch_pool));
>>> +
>>> + /* Set up some data structures */
>>> + iterpool = svn_pool_create(scratch_pool);
>>> + props_dirpath = svn_wc__adm_child(wcroot_abspath, PROPS_SUBDIR,
>> scratch_pool);
>>> + props_base_dirpath = svn_wc__adm_child(wcroot_abspath,
>> PROP_BASE_SUBDIR,
>>> + scratch_pool);
>>> +
>>> + /* Iterate over the children, as described above */
>>> + for (i = 0; i < children->nelts; i++)
>>> + {
>>> + const char *child_relpath = APR_ARRAY_IDX(children, i, const
>> char *);
>>> + const char *child_abspath;
>>> + const char *prop_base_path, *prop_working_path,
>> *prop_revert_path;
>>> + svn_boolean_t pristine_is_working;
>>> + svn_boolean_t replaced;
>>> + apr_hash_t *working_props;
>>> + svn_stream_t *stream;
>>> +
>>> + svn_pool_clear(iterpool);
>>> +
>>> + /* Several useful paths. */
>>> + child_abspath = svn_dirent_join(wcroot_abspath, child_relpath,
>> iterpool);
>>> + prop_base_path = svn_dirent_join(props_base_dirpath,
>>> + apr_psprintf(iterpool, "%s"
>> SVN_WC__BASE_EXT,
>>> + child_relpath),
>>> + iterpool);
>>> + prop_working_path = svn_dirent_join(props_dirpath,
>>> + apr_psprintf(iterpool, "%s"
>> SVN_WC__WORK_EXT,
>>> + child_relpath),
>>> + iterpool);
>>> + prop_revert_path = svn_dirent_join(props_base_dirpath,
>>> + apr_psprintf(iterpool, "%s"
>> SVN_WC__REVERT_EXT,
>>> + child_relpath),
>>> + iterpool);
>>> +
>>> + /* if node is replaced ... */
>>> + SVN_ERR(svn_wc__internal_is_replaced(&replaced, db,
>> child_abspath,
>>> + iterpool));
>>> + if (replaced)
>>> + {
>>> + apr_hash_t *revert_props;
>>> +
>>> + SVN_ERR(svn_stream_open_readonly(&stream,
>> prop_revert_path, iterpool,
>>> + iterpool));
>>> + SVN_ERR(svn_hash_read2(revert_props, stream,
>> SVN_HASH_TERMINATOR,
>>> + iterpool));
>>> +
>>> + SVN_ERR(svn_wc__db_temp_op_set_pristine_props(db,
>> child_abspath,
>>> +
>> revert_props, FALSE,
>>> + iterpool));
>>> + }
>>> + else
>>> + {
>>> + apr_hash_t *base_props;
>>> +
>>> + SVN_ERR(svn_stream_open_readonly(&stream, prop_base_path,
>> iterpool,
>>> + iterpool));
>>> + SVN_ERR(svn_hash_read2(base_props, stream,
>> SVN_HASH_TERMINATOR,
>>> + iterpool));
>>> +
>>> +
>> SVN_ERR(svn_wc__prop_pristine_is_working(&pristine_is_working, db,
>>> + child_abspath,
>> iterpool));
>>> + SVN_ERR(svn_wc__db_temp_op_set_pristine_props(db,
>> child_abspath,
>>> + base_props,
>>> +
>> pristine_is_working,
>>> + iterpool));
>>> + }
>>> +
>>> + SVN_ERR(svn_stream_open_readonly(&stream, prop_working_path,
>> iterpool,
>>> + iterpool));
>>> + SVN_ERR(svn_hash_read2(working_props, stream,
>> SVN_HASH_TERMINATOR,
>>> + iterpool));
>>> +
>>> + SVN_ERR(svn_wc__db_op_set_props(db, child_abspath,
>> working_props,
>>> + iterpool));
>>> + }
>>> +
>>> + /* Now delete the old directories. */
>>> + SVN_ERR(svn_io_remove_dir2(props_dirpath, TRUE, NULL, NULL,
>> iterpool));
>>> + SVN_ERR(svn_io_remove_dir2(props_base_dirpath, TRUE, NULL, NULL,
>>> + iterpool));
>>> + SVN_ERR(svn_io_remove_dir2(svn_dirent_join_many(iterpool,
>> wcroot_abspath,
>>> + ".svn", TEMP_DIR,
>>> + PROPS_SUBDIR,
>> NULL),
>>> + TRUE, NULL, NULL, iterpool));
>>> + SVN_ERR(svn_io_remove_dir2(svn_dirent_join_many(iterpool,
>> wcroot_abspath,
>>> + ".svn", TEMP_DIR,
>>> + PROP_BASE_SUBDIR,
>> NULL),
>>> + TRUE, NULL, NULL, iterpool));
>>> +
>>> + SVN_ERR(svn_wc__db_close(db));
>>> + svn_pool_destroy(iterpool);
>>> +
>>> + return SVN_NO_ERROR;
>>> +}
>>> +
>>> +
>>> #if 0
>>> /* This implements svn_sqlite__transaction_callback_t */
>>> static svn_error_t *
>>> -bump_database_to_16(void *baton,
>>> +bump_database_to_17(void *baton,
>>> svn_sqlite__db_t *sdb,
>>> apr_pool_t *scratch_pool)
>>> {
>>> @@ -742,15 +879,15 @@ bump_database_to_16(void *baton,
>>> }
>>>
>>> static svn_error_t *
>>> -bump_to_16(const char *wcroot_abspath,
>>> +bump_to_17(const char *wcroot_abspath,
>>> svn_sqlite__db_t *sdb,
>>> apr_pool_t *scratch_pool)
>>> {
>>> /* ### migrate disk bits here. */
>>>
>>> /* Perform the database upgrade. The last thing this does is to
>> bump
>>> - the recorded version to 15. */
>>> - SVN_ERR(svn_sqlite__with_transaction(sdb, bump_database_to_15,
>> NULL, scratch_pool));
>>> + the recorded version to 17. */
>>> + SVN_ERR(svn_sqlite__with_transaction(sdb, bump_database_to_17,
>> NULL, scratch_pool));
>>>
>>> return SVN_NO_ERROR;
>>> }
>>> @@ -790,8 +927,13 @@ svn_wc__upgrade_sdb(int *result_format,
>>> SVN_ERR(svn_sqlite__set_schema_version(sdb, 15,
>> scratch_pool));
>>> ++start_format;
>>>
>>> -#if 0
>>> case 15:
>>> + SVN_ERR(migrate_props(wcroot_abspath, sdb, scratch_pool));
>>> + SVN_ERR(svn_sqlite__set_schema_version(sdb, 16,
>> scratch_pool));
>>> + ++start_format;
>>> +
>>> +#if 0
>>> + case 16:
>>> SVN_ERR(bump_to_16(wcroot_abspath, sdb, scratch_pool));
>>> ++start_format;
>>> #endif
>>>
>>> ------------------------------------------------------
>>>
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
>> d=2416883
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageI
>> d=2416884
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2420068
Received on 2009-11-19 15:53:19 CET