> -----Original Message-----
> From: s...@apache.org [mailto:s...@apache.org]
> Sent: vrijdag 2 november 2012 02:53
> To: comm...@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 

Reply via email to