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

RE: svn commit: r1404856 - in /subversion/trunk/subversion: libsvn_wc/ tests/cmdline/

From: Bert Huijben <bert_at_qqmail.nl>
Date: Fri, 2 Nov 2012 12:22:29 +0100

> -----Original Message-----
> From: stsp_at_apache.org [mailto:stsp_at_apache.org]
> Sent: vrijdag 2 november 2012 02:53
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1404856 - in /subversion/trunk/subversion: libsvn_wc/
> tests/cmdline/
>
> Author: stsp
> Date: Fri Nov 2 01:53:23 2012
> New Revision: 1404856
>
> URL: http://svn.apache.org/viewvc?rev=1404856&view=rev
> Log:
> Disable automatic working copy upgrades. This has been discussed over and
> over, with many people in the community indicating they prefer manual
> upgrades.
>
> For now, this is a hard-coded default. There is no way to enable auto-
> upgrade.
> And unfortunately there is no single knob to globally switch auto-upgrade on
> and off in the code. Rather, function parameters have to be tweaked in
> various
> places where a working copy database is opened.
>
> Some parts of the upgrade code were written and tested exclusively for
> upgrading from 1.6 and earlier working copy formats to wc-ng, and thus
> needed small fixes to allow 'svn upgrade' to run wc-ng -> wn-ng format
> bumps without crashing.
>
> * subversion/libsvn_wc/adm_files.c
> (svn_wc_create_tmp_file2): Don't auto-upgrade working copies.
>
> * subversion/libsvn_wc/adm_ops.c
> (svn_wc_get_pristine_copy_path): Don't auto-upgrade working copies.
>
> * subversion/libsvn_wc/cleanup.c
> (svn_wc_cleanup3): Don't auto-upgrade working copies.
>
> * subversion/libsvn_wc/context.c
> (svn_wc_context_create): Don't auto-upgrade working copies.
>
> * subversion/libsvn_wc/lock.c
> (pool_cleanup_locked, svn_wc_adm_open3, svn_wc_adm_probe_open3,
> open_anchor): Don't auto-upgrade working copies.
>
> * subversion/libsvn_wc/upgrade.c
> (svn_wc__upgrade_sdb): Initialise *result_format before use if the
> working copy is already at format SVN_WC__VERSION to prevent an
> assertion
> during no-op upgrades of wc-ng working copies.
> (is_old_wcroot): Remove Subversion version numbers from error
> messages.
> It is now misleading to say that an upgrade is not possible because a
> pre-1.7 working copy was found since 'svn upgrade' now runs on any
> format.
> (svn_wc_upgrade): Handle upgrades from wc-ng-style working copies.
> Enable auto-upgrade of the db during upgrades from pre-1.7 WCs to avoid
> 'svn upgrade' advising users to run 'svn upgrade'.
>
> * subversion/libsvn_wc/wc_db.c
> (svn_wc__db_bump_format): New helper function for wc-ng -> wc-ng
> upgrades.
> Ensures that the upgrade target is a working copy root, and performs a
> format bump of wcroot->sdb, which isn't exposed outside of the wc_db
> layer.
>
> * subversion/libsvn_wc/wc_db.h
> (svn_wc__db_bump_format): Declare.
>
> * subversion/libsvn_wc/wc_db_wcroot.c
> (svn_wc__db_pdh_create_wcroot): Properly handle the case where we do
> not
> auto-upgrade and run into a working copy using an older format.
> Previously, we ended up asserting in VERIFY_USABLE_WCROOT()
> somewhere
> because no upgrade happened but no error was returned either. Instead,
> return an error advising the user to upgrade the working copy.
>
> * subversion/tests/cmdline/upgrade_tests.py
> (wc_is_too_old_regex): The error message matched by this regex has
> changed
> in some cases, so adjust the regex accordingly.
> (basic_upgrade): Expect slightly different error messages resulting from
> above changes. The error code returned is unchanged, however.
> (upgrade_tree_conflict_data, upgrade_from_format_28): These tests were
> running 'svn status' and 'svn info' to trigger auto-upgrades. Make them
> run 'svn upgrade' instead to keep them passing.
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/adm_files.c
> subversion/trunk/subversion/libsvn_wc/adm_ops.c
> subversion/trunk/subversion/libsvn_wc/cleanup.c
> subversion/trunk/subversion/libsvn_wc/context.c
> subversion/trunk/subversion/libsvn_wc/lock.c
> subversion/trunk/subversion/libsvn_wc/upgrade.c
> subversion/trunk/subversion/libsvn_wc/wc_db.c
> subversion/trunk/subversion/libsvn_wc/wc_db.h
> subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c
> subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_files.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm
> _files.c?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/adm_files.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_files.c Fri Nov 2 01:53:23
> 2012
> @@ -592,7 +592,7 @@ svn_wc_create_tmp_file2(apr_file_t **fp,
>
> SVN_ERR(svn_wc__db_open(&db,
> NULL /* config */,
> - TRUE /* auto_upgrade */,
> + FALSE /* auto_upgrade */,
> TRUE /* enforce_empty_wq */,
> pool, pool));
>
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm
> _ops.c?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Fri Nov 2 01:53:23
> 2012
> @@ -2211,7 +2211,7 @@ svn_wc_get_pristine_copy_path(const char
>
> SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
>
> - SVN_ERR(svn_wc__db_open(&db, NULL, TRUE, TRUE, pool, pool));
> + SVN_ERR(svn_wc__db_open(&db, NULL, FALSE, TRUE, pool, pool));
> /* DB is now open. This is seemingly a "light" function that a caller
> may use repeatedly despite error return values. The rest of this
> function should aggressively close DB, even in the error case. */
>
> Modified: subversion/trunk/subversion/libsvn_wc/cleanup.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/clea
> nup.c?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/cleanup.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/cleanup.c Fri Nov 2 01:53:23
> 2012
> @@ -206,7 +206,7 @@ svn_wc_cleanup3(svn_wc_context_t *wc_ctx
> /* We need a DB that allows a non-empty work queue (though it *will*
> auto-upgrade). We'll handle everything manually. */
> SVN_ERR(svn_wc__db_open(&db,
> - NULL /* ### config */, TRUE, FALSE,
> + NULL /* ### config */, FALSE, FALSE,
> scratch_pool, scratch_pool));
>
> SVN_ERR(cleanup_internal(db, local_abspath, cancel_func, cancel_baton,
>
> Modified: subversion/trunk/subversion/libsvn_wc/context.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/cont
> ext.c?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/context.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/context.c Fri Nov 2 01:53:23
> 2012
> @@ -70,7 +70,7 @@ svn_wc_context_create(svn_wc_context_t *
> * we need to make it writable */
> ctx->state_pool = result_pool;
> SVN_ERR(svn_wc__db_open(&ctx->db, (svn_config_t *)config,
> - TRUE, TRUE, ctx->state_pool, scratch_pool));
> + FALSE, TRUE, ctx->state_pool, scratch_pool));
> ctx->close_db_on_destroy = TRUE;
>
> apr_pool_cleanup_register(result_pool, ctx, close_ctx_apr,
>
> Modified: subversion/trunk/subversion/libsvn_wc/lock.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/lock.
> c?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/lock.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/lock.c Fri Nov 2 01:53:23 2012
> @@ -331,7 +331,7 @@ pool_cleanup_locked(void *p)
> run, but the subpools will NOT be destroyed) */
> scratch_pool = svn_pool_create(lock->pool);
>
> - err = svn_wc__db_open(&db, NULL /* ### config. need! */, TRUE, TRUE,
> + err = svn_wc__db_open(&db, NULL /* ### config. need! */, FALSE,
> TRUE,
> scratch_pool, scratch_pool);
> if (!err)
> {
> @@ -781,7 +781,7 @@ svn_wc_adm_open3(svn_wc_adm_access_t **a
> do it here. */
> /* ### we could optimize around levels_to_lock==0, but much of this
> ### is going to be simplified soon anyways. */
> - SVN_ERR(svn_wc__db_open(&db, NULL /* ### config. need! */, TRUE,
> TRUE,
> + SVN_ERR(svn_wc__db_open(&db, NULL /* ### config. need! */, FALSE,
> TRUE,
> pool, pool));
> db_provided = FALSE;
> }
> @@ -811,7 +811,7 @@ svn_wc_adm_probe_open3(svn_wc_adm_access
>
> /* Ugh. Too bad about having to open a DB. */
> SVN_ERR(svn_wc__db_open(&db,
> - NULL /* ### config */, TRUE, TRUE, pool, pool));
> + NULL /* ### config */, FALSE, TRUE, pool, pool));
> err = probe(db, &dir, path, pool);
> svn_error_clear(svn_wc__db_close(db));
> SVN_ERR(err);
> @@ -1157,7 +1157,7 @@ open_anchor(svn_wc_adm_access_t **anchor
> ### given that we need DB for format detection, may as well keep this.
> ### in any case, much of this is going to be simplified soon anyways. */
> if (!db_provided)
> - SVN_ERR(svn_wc__db_open(&db, NULL, /* ### config. need! */ TRUE,
> TRUE,
> + SVN_ERR(svn_wc__db_open(&db, NULL, /* ### config. need! */ FALSE,
> TRUE,
> pool, pool));
>
> if (svn_path_is_empty(path)
>
> Modified: subversion/trunk/subversion/libsvn_wc/upgrade.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/upgr
> ade.c?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/upgrade.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/upgrade.c Fri Nov 2 01:53:23
> 2012
> @@ -1892,6 +1892,9 @@ svn_wc__upgrade_sdb(int *result_format,
> *result_format = XXX;
> /* FALLTHROUGH */
> #endif
> + case SVN_WC__VERSION:
> + /* already upgraded */
> + *result_format = SVN_WC__VERSION;
> }
>
> #ifdef SVN_DEBUG
> @@ -2019,7 +2022,7 @@ is_old_wcroot(const char *local_abspath,
> {
> return svn_error_createf(
> SVN_ERR_WC_INVALID_OP_ON_CWD, err,
> - _("Can't upgrade '%s' as it is not a pre-1.7 working copy directory"),
> + _("Can't upgrade '%s' as it is not a working copy"),
> svn_dirent_local_style(local_abspath, scratch_pool));
> }
> else if (svn_dirent_is_root(local_abspath, strlen(local_abspath)))
> @@ -2068,7 +2071,7 @@ is_old_wcroot(const char *local_abspath,
>
> return svn_error_createf(
> SVN_ERR_WC_INVALID_OP_ON_CWD, NULL,
> - _("Can't upgrade '%s' as it is not a pre-1.7 working copy root,"
> + _("Can't upgrade '%s' as it is not a working copy root,"
> " the root is '%s'"),
> svn_dirent_local_style(local_abspath, scratch_pool),
> svn_dirent_local_style(parent_abspath, scratch_pool));
> @@ -2128,6 +2131,34 @@ svn_wc_upgrade(svn_wc_context_t *wc_ctx,
> apr_hash_t *entries;
> const char *root_adm_abspath;
> upgrade_working_copy_baton_t cb_baton;
> + svn_error_t *err;
> + int result_format;
> +
> + /* Try upgrading a wc-ng-style working copy. */
> + SVN_ERR(svn_wc__db_open(&db, NULL /* ### config */, TRUE, FALSE,
> + scratch_pool, scratch_pool));
> +
> + err = svn_wc__db_bump_format(&result_format, local_abspath, db,
> + scratch_pool);
> + if (err)
> + {
> + if (err->apr_err == SVN_ERR_WC_UPGRADE_REQUIRED) /* pre-1.7 WC
> */
> + {
> + svn_error_clear(err);
> + SVN_ERR(svn_wc__db_close(db));
> + }
> + else
> + return svn_error_trace(err);
> + }
> + else
> + {
> + /* Auto-upgrade worked! */
> + SVN_ERR(svn_wc__db_close(db));
> +
> + SVN_ERR_ASSERT(result_format == SVN_WC__VERSION);
> +
> + return SVN_NO_ERROR;
> + }
>
> SVN_ERR(is_old_wcroot(local_abspath, scratch_pool));
>
> @@ -2141,7 +2172,7 @@ svn_wc_upgrade(svn_wc_context_t *wc_ctx,
> upgrade. */
>
> SVN_ERR(svn_wc__db_open(&db,
> - NULL /* ### config */, FALSE, FALSE,
> + NULL /* ### config */, TRUE, FALSE,
> scratch_pool, scratch_pool));
>
> SVN_ERR(svn_wc__read_entries_old(&entries, local_abspath,
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_
> db.c?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Fri Nov 2 01:53:23
> 2012
> @@ -14025,3 +14025,36 @@ svn_wc__db_verify(svn_wc__db_t *db,
> SVN_ERR(verify_wcroot(wcroot, scratch_pool));
> return SVN_NO_ERROR;
> }
> +
> +svn_error_t *
> +svn_wc__db_bump_format(int *result_format,
> + const char *wcroot_abspath,
> + svn_wc__db_t *db,
> + apr_pool_t *scratch_pool)
> +{
> +
> + svn_wc__db_wcroot_t *wcroot;
> + const char *local_relpath;
> +
> + SVN_ERR(svn_wc__db_wcroot_parse_local_abspath(&wcroot,
> &local_relpath,
> + db, wcroot_abspath,
> + scratch_pool, scratch_pool));
> +
> + /* This function is indirectly called from the upgrade code, so we
> + can't verify the wcroot here. Just check that it is not NULL */
> + SVN_ERR_ASSERT(wcroot != NULL);
> +
> + /* Reject attempts to upgrade subdirectories of a working copy. */
> + if (strcmp(wcroot_abspath, wcroot->abspath) != 0)
> + return svn_error_createf(
> + SVN_ERR_WC_INVALID_OP_ON_CWD, NULL,
> + _("Can't upgrade '%s' as it is not a working copy root,"
> + " the root is '%s'"),
> + svn_dirent_local_style(wcroot_abspath, scratch_pool),
> + svn_dirent_local_style(wcroot->abspath, scratch_pool));
> +
> + SVN_ERR(svn_wc__upgrade_sdb(result_format, wcroot->abspath,
> + wcroot->sdb, wcroot->format,
> + scratch_pool));
> + return SVN_NO_ERROR;
> +}
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_
> db.h?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Fri Nov 2 01:53:23
> 2012
> @@ -2792,6 +2792,23 @@ svn_wc__db_upgrade_get_repos_id(apr_int6
> const char *repos_root_url,
> apr_pool_t *scratch_pool);
>
> +/* Upgrade the metadata concerning the WC at WCROOT_ABSPATH, in DB,
> + * to the SVN_WC__VERSION format.
> + *
> + * This function is used for upgrading wc-ng working copies to a newer
> + * wc-ng format. If a pre-1.7 working copy is found, this function
> + * returns SVN_ERR_WC_UPGRADE_REQUIRED.
> + *
> + * Upgrading subdirectories of a working copy is not supported.
> + * If WCROOT_ABSPATH is not a working copy root
> SVN_ERR_WC_INVALID_OP_ON_CWD
> + * is returned.
> + */
> +svn_error_t *
> +svn_wc__db_bump_format(int *result_format,
> + const char *wcroot_abspath,
> + svn_wc__db_t *db,
> + apr_pool_t *scratch_pool);
> +
> /* @} */
>
>
>
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_
> db_wcroot.c?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db_wcroot.c Fri Nov 2
> 01:53:23 2012
> @@ -27,6 +27,7 @@
>
> #include "svn_dirent_uri.h"
> #include "svn_path.h"
> +#include "svn_version.h"
>
> #include "wc.h"
> #include "adm_files.h"
> @@ -294,9 +295,24 @@ svn_wc__db_pdh_create_wcroot(svn_wc__db_
> }
>
> /* Auto-upgrade the SDB if possible. */
> - if (format < SVN_WC__VERSION && auto_upgrade)
> - SVN_ERR(svn_wc__upgrade_sdb(&format, wcroot_abspath, sdb, format,
> - scratch_pool));
> + if (format < SVN_WC__VERSION)
> + {
> + if (auto_upgrade)
> + {
> + if (format >= SVN_WC__WC_NG_VERSION)
> + SVN_ERR(svn_wc__upgrade_sdb(&format, wcroot_abspath, sdb,
> format,
> + scratch_pool));
> + }
> + else
> + return svn_error_createf(SVN_ERR_WC_UPGRADE_REQUIRED, NULL,
> + _("The working copy at '%s'\nis too old "
> + "(format %d) to work with client version "
> + "'%s' (expects format %d). You need to "
> + "upgrade the working copy first.\n"),
> + svn_dirent_local_style(wcroot_abspath,
> + scratch_pool), format, SVN_VERSION,
> + SVN_WC__VERSION);
> + }
>
> *wcroot = apr_palloc(result_pool, sizeof(**wcroot));
>
>
> Modified: subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/
> upgrade_tests.py?rev=1404856&r1=1404855&r2=1404856&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/tests/cmdline/upgrade_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/upgrade_tests.py Fri Nov 2
> 01:53:23 2012
> @@ -50,7 +50,7 @@ Issues = svntest.testcase.Issues_deco
> Issue = svntest.testcase.Issue_deco
> Wimp = svntest.testcase.Wimp_deco
>
> -wc_is_too_old_regex = (".*Working copy '.*' is too old \(format \d+.*\).*")
> +wc_is_too_old_regex = (".*is too old \(format \d+.*\).*")
>
>
> def get_current_format():
> @@ -258,28 +258,29 @@ def basic_upgrade(sbox):
> replace_sbox_with_tarfile(sbox, 'basic_upgrade.tar.bz2')
>
> # Attempt to use the working copy, this should give an error
> - expected_stderr = wc_is_too_old_regex
> - svntest.actions.run_and_verify_svn(None, None, expected_stderr,
> + svntest.actions.run_and_verify_svn(None, None, wc_is_too_old_regex,
> 'info', sbox.wc_dir)
>
> -
> - # Upgrade on something not a versioned dir gives a 'not directory' error.
> - not_dir = ".*E155019.*%s'.*directory"
> + # Upgrade on something anywhere within a versioned subdir gives a
> + # 'not a working copy root' error. Upgrade on something without any
> + # versioned parent gives a 'not a working copy' error.
> + # Both cases use the same error code.
> + not_wc = ".*E155019.*%s'.*not a working copy.*"
> os.mkdir(sbox.ospath('X'))
> - svntest.actions.run_and_verify_svn(None, None, not_dir % 'X',
> + svntest.actions.run_and_verify_svn(None, None, not_wc % 'X',
> 'upgrade', sbox.ospath('X'))

Can you please use a specific error code if an upgrade is required?
(SVN_ERR_WC_UPGRADE_REQUIRED or a new error?)

GUI clients can't just parse the textual output of error messages, and want to parse the error code.

SVN_ERR_WC_NOT_WORKING_COPY has a different handling for those clients than suggesting the user to upgrade.

What menu option should I show on an old working copy?
* Import
* Upgrade

With one error code I will have to show both while the user really expects better behavior.

        Bert
Received on 2012-11-02 12:23:19 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.