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

On Fri, Mar 13, 2009 at 23:02, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_wc/entries.c        Fri Mar 13 15:02:06 2009        (r36537)

for entries.c, note that we could have a temp function to get an sdb
from the db.

>...
> @@ -1851,6 +1810,21 @@ write_entry(svn_sqlite__db_t *wc_db,
>       base_node->changed_author = entry->cmt_author;
>
>       SVN_ERR(insert_base_node(wc_db, base_node, scratch_pool));
> +
> +      /* We have to insert the lock after the base node, because the node
> +         must exist to lookup various bits of repos related information for
> +         the abs path. */
> +      if (entry->lock_token)
> +        {
> +          svn_wc__db_lock_t *lock = apr_pcalloc(scratch_pool, sizeof(*lock));

gah. dude. put it on the stack.

>...
> @@ -1981,9 +1963,11 @@ entries_write_body(svn_sqlite__db_t *wc_
>
>   /* Write out "this dir" */
>   SVN_ERR(fetch_wc_id(&wc_id, wc_db));
> -  SVN_ERR(write_entry(wc_db, wc_id, repos_id, repos_root, this_dir,
> -                      SVN_WC_ENTRY_THIS_DIR, this_dir,
> -                      scratch_pool));
> +  SVN_ERR(write_entry(db, wc_db, wc_id, repos_id, repos_root, this_dir,
> +                      SVN_WC_ENTRY_THIS_DIR,
> +                      svn_dirent_join(local_abspath, SVN_WC_ENTRY_THIS_DIR,
> +                                      scratch_pool),
> +                      this_dir, scratch_pool));

SVN_WC_ENTRY_THIS_DIR is defined as "". no need for the join, and
(really) no need for the dumb symbol. in ancient history, we had a
name like "svn:this_dir" or somesuch when we recorded the item in the
'entries' file. it just doesn't make sense to keep the symbol.

>...
> @@ -2844,14 +2832,17 @@ svn_wc__entries_init(const char *path,
>                  || depth == svn_depth_immediates
>                  || depth == svn_depth_infinity);
>
> -  /* Check that the entries sqlite database does not yet exist. */
> -  SVN_ERR(svn_io_check_path(wc_db_path, &kind, scratch_pool));
> -  if (kind != svn_node_none)
> -    return svn_error_createf(SVN_ERR_WC_DB_ERROR, NULL,
> -                             _("Existing sqlite database found at '%s'"),
> -                             svn_path_local_style(wc_db_path, pool));
> -
> -  /* Create the entries database, and start a transaction. */
> +  /* ### chicken and egg problem: we don't have an adm_access baton to
> +     ### use to open the initial entries database, we we've got to do it
> +     ### manually. */
> +  SVN_ERR(svn_wc__db_open(&db, svn_wc__db_openmode_readwrite, path,
> +                          NULL, scratch_pool, scratch_pool));
> +
> +  /* Open the sqlite database, and insert the REPOS and WCROOT.
> +     ### this is redundant, but we currently need it for the entry
> +     ### inserting API.  DB and WC_DB should be pointing to the *same*
> +     ### sqlite database, and it works fine thanks for sqlite's
> +     ### concurrency handling.  However, this should eventually disappear. */
>   SVN_ERR(svn_sqlite__open(&wc_db, wc_db_path, svn_sqlite__mode_rwcreate,
>                            statements,
>                            SVN_WC__VERSION_EXPERIMENTAL, upgrade_sql,

could get this from the db? we could also have something like:
svn_wc__db_construct() to construct a wc for the given directory. (or
do nothing in the future, if it finds a parent directory is already a
wc)

>...
> +++ trunk/subversion/libsvn_wc/wc_db.c  Fri Mar 13 15:02:06 2009        (r36537)
>...
> @@ -2657,6 +2663,55 @@ svn_wc__db_global_commit(svn_wc__db_t *d
>
>
>  svn_error_t *
> +svn_wc__db_lock_add(svn_wc__db_t *db,
> +                    const char *local_abspath,
> +                    const svn_wc__db_lock_t *lock,
> +                    apr_pool_t *scratch_pool)
> +{
> +  svn_wc__db_pdh_t *pdh;
> +  const char *local_relpath;
> +  svn_sqlite__stmt_t *stmt;
> +  const char *repos_root_url;
> +  const char *repos_relpath;
> +  const char *repos_uuid;
> +  apr_int64_t repos_id;
> +
> +  SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
> +
> +  /* Fetch the repos root and repos uuid from the base node, we we can
> +     then create or get the repos id.
> +     ### is there a better way to do this? */
> +  SVN_ERR(svn_wc__db_base_get_info(NULL, NULL, NULL, &repos_relpath,
> +                                   &repos_root_url, &repos_uuid,
> +                                   NULL, NULL, NULL, NULL, NULL, NULL,
> +                                   NULL, NULL, NULL,
> +                                   db, local_abspath, scratch_pool,
> +                                   scratch_pool));

Yeah, there might be a better way, but this isn't bad. It handles the
case of the row containing the data, or needing to inherit the
information. And it gets the repos_relpath.

> +
> +  SVN_ERR(parse_local_abspath(&pdh, &local_relpath, db, local_abspath,
> +                              svn_sqlite__mode_readwrite,
> +                              scratch_pool, scratch_pool));
> +  SVN_ERR(create_repos_id(&repos_id, repos_root_url, repos_uuid, pdh->sdb,
> +                          scratch_pool));

These two lines are the best reason to find another way. base_get_info
already does a parse_local_abspath, and we want to share that. Also,
it discovered a repos_id, so we don't want to look it up again.

> +  SVN_ERR(svn_sqlite__get_statement(&stmt, pdh->sdb, STMT_INSERT_LOCK));
> +  SVN_ERR(svn_sqlite__bind_int64(stmt, 1, repos_id));
> +  SVN_ERR(svn_sqlite__bind_text(stmt, 2, repos_relpath));
> +  SVN_ERR(svn_sqlite__bind_text(stmt, 3, lock->token));
> +
> +  if (lock->owner != NULL)
> +    SVN_ERR(svn_sqlite__bind_text(stmt, 4, lock->owner));
> +
> +  if (lock->comment != NULL)
> +    SVN_ERR(svn_sqlite__bind_text(stmt, 5, lock->comment));
> +
> +  SVN_ERR(svn_sqlite__bind_int64(stmt, 6, lock->date));

If the date is not specified (0), then should we avoid binding this
column? Leave it null?

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1321370
Received on 2009-03-14 11:55:45 CET