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

Re: svn commit: r922884 - in /subversion/trunk/subversion/libsvn_wc: update_editor.c wc-queries.sql wc_db.c wc_db.h

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 18 Mar 2010 18:06:43 -0400

On Sun, Mar 14, 2010 at 12:13, <rhuijben_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Sun Mar 14 16:13:43 2010
> @@ -243,6 +243,14 @@ where wc_id = ?1 and local_relpath = ?2;
>  update working_node set presence = ?3
>  where wc_id = ?1 and local_relpath =?2;
>
> +-- STMT_UPDATE_BASE_PRESENCE_AND_REVNUM
> +update base_node set presence = ?3, revnum = ?4
> +where wc_id = ?1 and local_relpath = ?2;
> +
> +-- STMT_UPDATE_BASE_PRESENCE_REVNUM_AND_REPOS_RELPATH
> +update base_node set presence = ?3, revnum = ?4, repos_relpath = ?5
> +where wc_id = ?1 and local_relpath = ?2;

As mentioned on IRC, you cannot update 'repos_relpath' without
ensuring that 'repos_id' follows along. The two columns must both be
set, or must both be null. You can't update one without the other.

>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Sun Mar 14 16:13:43 2010
> @@ -6350,3 +6350,91 @@ svn_wc__db_temp_op_set_working_last_chan
>
>   return SVN_NO_ERROR;
>  }
> +
> +struct start_directory_update_baton
> +{
> +  svn_wc__db_t *db;
> +  const char *local_abspath;
> +  apr_int64_t wc_id;
> +  const char *local_relpath;
> +  svn_revnum_t new_rev;
> +  const char *new_repos_relpath;
> +};
> +
> +static svn_error_t *
> +start_directory_update_txn(void *baton,
> +                           svn_sqlite__db_t *db,
> +                           apr_pool_t *scratch_pool)
> +{
> +  struct start_directory_update_baton *du = baton;
> +  const char *repos_relpath;
> +  svn_sqlite__stmt_t *stmt;
> +
> +  SVN_ERR(svn_wc__db_scan_base_repos(&repos_relpath, NULL, NULL,
> +                                     du->db, du->local_abspath,
> +                                     scratch_pool, scratch_pool));
> +
> +  if (strcmp(du->new_repos_relpath, repos_relpath) == 0)
> +    {

You're not validating that these came from the same repository. All
code in wc_db should enable/allow for mixed-repository working copies.
Especially where it is a simple strcmp() of the root (or uuid?).

If it is horrendous to support mixed-repos for some reason, then I
expect to see a large ### block noting the lack of capability and an
explanation why.

>...
> +  else
> +    {
> +      /* ### TODO: Maybe check if we can make repos_relpath NULL. */
> +      SVN_ERR(svn_sqlite__get_statement(
> +                        &stmt, db,
> +                        STMT_UPDATE_BASE_PRESENCE_REVNUM_AND_REPOS_RELPATH));

See comment above re: setting just one column.

>...
> +svn_error_t *
> +svn_wc__db_temp_op_start_directory_update(svn_wc__db_t *db,
> +                                          const char *local_abspath,
> +                                          const char* new_repos_relpath,
> +                                          svn_revnum_t new_rev,
> +                                          apr_pool_t *scratch_pool)

Style nit alert: mis-positioned *

>...

Cheers,
-g
Received on 2010-03-18 23:07:14 CET

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