On Thu, Apr 30, 2009 at 17:01, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_wc/upgrade.c Thu Apr 30 08:01:58 2009 (r37513)
>...
> +/* Helper for converting wcprops from the one-file-per-file model.
> + This implements svn_io_walk_func_t(). */
> +static svn_error_t *
> +convert_props_walker(void *baton,
> + const char *path,
> + const apr_finfo_t *finfo,
> + apr_pool_t *pool)
> +{
> + svn_wc__db_t *db = baton;
> + apr_hash_t *proplist;
> + apr_file_t *file;
> + svn_stream_t *stream;
> + const char *local_abspath;
> + int len;
> +
> + /* Skip the directory. */
> + if (finfo->filetype == APR_DIR)
> + return SVN_NO_ERROR;
> +
> + proplist = apr_hash_make(pool);
> + SVN_ERR(svn_io_file_open(&file, path, APR_READ | APR_BUFFERED,
> + APR_OS_DEFAULT, pool));
> + stream = svn_stream_from_aprfile2(file, FALSE, pool);
> + SVN_ERR(svn_hash_read2(proplist, stream, SVN_HASH_TERMINATOR, pool));
> + SVN_ERR(svn_stream_close(stream));
> +
> + /* The filename will be something like foo.c.svn-work. From it, determine
> + the local_abspath of the node. The magic number of 9 below is basically
> + strlen(".svn-work"). */
> + len = strlen(path);
> + local_abspath = apr_pstrndup(pool, local_abspath, len - 9);
This won't work. local_abspath is never set. And even then, you're
talking about a resulting abspath that looks like
"/some/path/.svn/foo.c" ... you really don't want the .svn in there.
Might I also suggest adding a test case for this to upgrade_tests.py?
>...
> +static svn_error_t *
> +convert_wcprops(svn_wc_adm_access_t *adm_access,
> + int old_format,
> + apr_pool_t *scratch_pool)
> +{
> + svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
> +
> + if (old_format <= SVN_WC__WCPROPS_MANY_FILES_VERSION)
> + {
> + svn_stream_t *stream;
> + svn_error_t *err;
> + const char *dir_abspath;
> + apr_hash_t *proplist;
> +
> + /* First, look at dir-wcprops. */
> + SVN_ERR(svn_dirent_get_absolute(&dir_abspath,
> + svn_wc_adm_access_path(adm_access),
> + scratch_pool));
dir_abspath = svn_wc__adm_access_abspath(adm_access);
> + err = svn_wc__open_adm_stream(&stream, dir_abspath,
> + SVN_WC__ADM_DIR_WCPROPS,
> + scratch_pool, scratch_pool);
> + if (err)
> + {
> + /* If the file doesn't exist, it means no wcprops. */
> + if (APR_STATUS_IS_ENOENT(err->apr_err))
> + svn_error_clear(err);
> + else
> + return svn_error_return(err);
> + }
> + else
> + {
> + proplist = apr_hash_make(scratch_pool);
> + SVN_ERR(svn_hash_read2(proplist, stream, SVN_HASH_TERMINATOR,
> + scratch_pool));
> + SVN_ERR(svn_wc__db_base_set_dav_cache(db, dir_abspath, proplist,
> + scratch_pool));
> + }
> +
> + /* Now walk the wcprops directory. */
> + SVN_ERR(svn_io_dir_walk(svn_wc__adm_child(svn_wc_adm_access_path(
> + adm_access),
> + SVN_WC__ADM_WCPROPS,
> + scratch_pool),
> + 0 /* wanted */,
> + convert_props_walker,
> + db, scratch_pool));
I understand this was the original/old code, but why walk rather than
simply fetching the dirents?
> + }
> + else
> + {
> + apr_hash_t *allprops;
> + apr_hash_index_t *hi;
> + apr_pool_t *iterpool;
> +
> + /* Read the all-wcprops file. */
> + SVN_ERR(read_wcprops(&allprops, db, svn_wc_adm_access_path(adm_access),
> + scratch_pool, scratch_pool));
> +
> + /* Iterate over all the wcprops, writing each one to the wc_db. */
> + iterpool = svn_pool_create(scratch_pool);
> + for (hi = apr_hash_first(scratch_pool, allprops); hi;
> + hi = apr_hash_next(hi))
> + {
> + const void *key;
> + void *val;
> + const char *name;
> + apr_hash_t *proplist;
> + const char *local_abspath;
> +
> + svn_pool_clear(iterpool);
> +
> + apr_hash_this(hi, &key, NULL, &val);
> + name = key;
> + proplist = val;
> +
> + SVN_ERR(svn_dirent_get_absolute(&local_abspath,
> + svn_wc_adm_access_path(adm_access),
> + iterpool));
I'd call this dir_abspath, locate it outside the loop, and use
svn_wc__adm_access_abspath() to get it.
> + local_abspath = svn_dirent_join(local_abspath, name, iterpool);
> + SVN_ERR(svn_wc__db_base_set_dav_cache(db, local_abspath, proplist,
> + iterpool));
> + }
> + svn_pool_destroy(iterpool);
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +
> +svn_error_t *
> +svn_wc__upgrade_format(svn_wc_adm_access_t *adm_access,
> + apr_pool_t *scratch_pool)
> +{
> + int wc_format;
> + svn_boolean_t cleanup_required;
> + svn_stringbuf_t *log_accum;
> + const svn_wc_entry_t *this_dir;
> +
> + SVN_ERR(svn_wc__adm_wc_format(&wc_format, adm_access, scratch_pool));
> +
> + if (wc_format > SVN_WC__VERSION)
> + {
> + return svn_error_createf
> + (SVN_ERR_WC_UNSUPPORTED_FORMAT, NULL,
> + _("This client is too old to work with working copy '%s'. You need\n"
> + "to get a newer Subversion client, or to downgrade this working "
> + "copy.\n"
> + "See "
> + "http://subversion.tigris.org/faq.html#working-copy-format-change\n"
> + "for details."
> + ),
> + svn_path_local_style(svn_wc_adm_access_path(adm_access),
> + scratch_pool));
> + }
> +
> + /* We can upgrade all formats that are accepted by check_format(). */
> + if (wc_format >= SVN_WC__VERSION)
> + return SVN_NO_ERROR;
This test makes no sense. You already checked for greater-than above.
Further, the comment *totally* doesn't describe what is going on here.
> +
> + /* Don't try to mess with the WC if there are old log files left. */
> + SVN_ERR(svn_wc__adm_is_cleanup_required(&cleanup_required,
> + adm_access, scratch_pool));
> +
> + if (cleanup_required)
> + return svn_error_create(SVN_ERR_WC_UNSUPPORTED_FORMAT, NULL,
> + _("Cannot upgrade with existing logs; please "
> + "run 'svn cleanup' with Subversion 1.6"));
> +
> + /* What's going on here?
> + *
> + * We're attempting to upgrade an older working copy to the new wc-ng format.
> + * The sematics and storage mechanisms between the two are vastly different,
> + * so it's going to be a bit painful. Here's a plan for the operation:
> + *
> + * 1) The 'entries' file needs to be moved to the new format. Ideally, we'd
> + * read it using svn_wc__entries_read_old(), and then translate the
> + * current state of the file into a series of wc_db commands to duplicate
> + * that state in WC-NG. We're not quite there yet, so we just use
> + * the same loggy process as we always have, relying on the lower layers
> + * to take care of the translation, and remembering to remove the old
> + * entries file when were're done. ### This isn't a long-term solution.
> + *
> + * 2) Convert wcprop to the wc-ng format
> + *
> + * ### (fill in other bits as they are implemented)
> + */
> +
> + /***** ENTRIES *****/
> + /* Create an empty sqlite database for this directory. */
> + SVN_ERR(svn_wc_entry(&this_dir, svn_wc_adm_access_path(adm_access),
> + adm_access, FALSE, scratch_pool));
> + SVN_ERR(svn_wc__entries_init(svn_wc_adm_access_path(adm_access),
> + this_dir->uuid, this_dir->url,
> + this_dir->repos, this_dir->revision,
> + this_dir->depth, scratch_pool));
> +
> + /* Migrate the entries over to the new database.
> + ### We need to thing about atomicity here. */
> + SVN_ERR(svn_wc__entries_upgrade(adm_access, SVN_WC__VERSION, scratch_pool));
> + svn_error_clear(svn_io_remove_file(svn_wc__adm_child(svn_wc_adm_access_path(
> + adm_access),
> + SVN_WC__ADM_FORMAT,
> + scratch_pool),
> + scratch_pool));
> + SVN_ERR(svn_io_remove_file(svn_wc__adm_child(svn_wc_adm_access_path(
> + adm_access),
> + SVN_WC__ADM_ENTRIES,
> + scratch_pool),
> + scratch_pool));
> +
> + /* ### Note that lots of this content is cribbed from the old format updater.
> + ### The following code will change as the wc-ng format changes and more
> + ### stuff gets migrated to the sqlite format. */
> +
> + log_accum = svn_stringbuf_create("", scratch_pool);
> +
> + /***** WC PROPS *****/
> + SVN_ERR(convert_wcprops(adm_access, wc_format, scratch_pool));
> +
> + SVN_ERR(svn_wc__write_log(adm_access, 0, log_accum, scratch_pool));
eh? what is this write_log doing here?
> +
> + if (wc_format <= SVN_WC__WCPROPS_MANY_FILES_VERSION)
> + {
> + /* Remove wcprops directory, dir-props, README.txt and empty-file
> + files.
> + We just silently ignore errors, because keeping these files is
> + not catastrophic. */
> +
> + svn_error_clear(svn_io_remove_dir2(
> + svn_wc__adm_child(svn_wc_adm_access_path(adm_access),
> + SVN_WC__ADM_WCPROPS, scratch_pool),
> + FALSE, NULL, NULL, scratch_pool));
> + svn_error_clear(svn_io_remove_file(
> + svn_wc__adm_child(svn_wc_adm_access_path(adm_access),
> + SVN_WC__ADM_DIR_WCPROPS, scratch_pool),
> + scratch_pool));
> + svn_error_clear(svn_io_remove_file(
> + svn_wc__adm_child(svn_wc_adm_access_path(adm_access),
> + SVN_WC__ADM_EMPTY_FILE, scratch_pool),
> + scratch_pool));
> + svn_error_clear(svn_io_remove_file(
> + svn_wc__adm_child(svn_wc_adm_access_path(adm_access),
> + SVN_WC__ADM_README, scratch_pool),
> + scratch_pool));
> + }
> + else
> + {
> + svn_error_clear(svn_io_remove_file(
> + svn_wc__adm_child(svn_wc_adm_access_path(adm_access),
> + SVN_WC__ADM_ALL_WCPROPS, scratch_pool),
> + scratch_pool));
> + }
> +
> + return svn_wc__run_log(adm_access, NULL, scratch_pool);
and what is this? you didn't put anything into log_accum.
>...
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2070805
Received on 2009-05-05 18:41:41 CEST