julianf...@apache.org wrote on Tue, Feb 22, 2011 at 15:38:35 -0000:
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c?rev=1073366&r1=1073365&r2=1073366&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c Tue Feb 22 
> 15:38:35 2011
> @@ -163,6 +165,57 @@ svn_wc__db_pristine_get_future_path(cons
> +/* Set (*BATON->contents) to a readable stream from which the pristine text
> + * identified by BATON->sha1_checksum can be read from the pristine store of
> + * SDB.  If that text is not in the pristine store, return an error.
> + *
> + * Allocate the stream in BATON->result_pool.
> + *
> + * This function expects to be executed inside a SQLite txn.
> + *
> + * Implements 'notes/wc-ng/pristine-store' section A-3(d).
> + * Implements svn_sqlite__transaction_callback_t. */
> +static svn_error_t *
> +pristine_read_txn(void *baton,
> +                  svn_sqlite__db_t *sdb,
> +                  apr_pool_t *scratch_pool)
> +{

Should document somewhere that the returned stream is only guaranteed to
remain valid if the caller holds a wc lock, etc.?

(svn_wc__db_pristine_read()'s docs don't mention locks)

Obviously there's still a bit of work here (eg, that "Delayed delete"
flag on Windows) and it can't all be done in one commit.

> +  return SVN_NO_ERROR;
> +}
> +
>  svn_error_t *
>  svn_wc__db_pristine_read(svn_stream_t **contents,
>                           svn_wc__db_t *db,
> @@ -173,7 +226,7 @@ svn_wc__db_pristine_read(svn_stream_t **
>  {
>    svn_wc__db_pdh_t *pdh;
>    const char *local_relpath;
> -  const char *pristine_abspath;
> +  pristine_read_baton_t b;
...
> +pristine_install_txn(void *baton,
> +                     svn_sqlite__db_t *sdb,
> +                     apr_pool_t *scratch_pool)
> +{
...
> +  /* If this pristine text is already present in the store, just keep it:
> +   * delete the new one and return. */
> +  SVN_ERR(svn_sqlite__get_statement(&stmt, sdb, STMT_SELECT_PRISTINE));
> +  SVN_ERR(svn_sqlite__bind_checksum(stmt, 1, b->sha1_checksum, 
> scratch_pool));
> +  SVN_ERR(svn_sqlite__step(&have_row, stmt));
> +  SVN_ERR(svn_sqlite__reset(stmt));
> +  if (have_row)
> +    {
> +      /* Remove the temp file: it's already there */
> +      /* ### TODO: Maybe verify the new file matches the existing one. */
> +      SVN_ERR(svn_io_remove_file2(b->tempfile_abspath, FALSE, scratch_pool));
> +      return SVN_NO_ERROR;

Perhaps check (assert?) that b->PRISTINE_ABSPATH exists?
(which reduces to dropping the "if (have_row)" check)

> +    }
> +
> +  /* Move the file to its target location, or discard it if already there. */
> +  SVN_ERR(svn_io_check_path(b->pristine_abspath, &kind, scratch_pool));
> +  if (kind == svn_node_file)
> +    {
> +      /* Remove the temp file: it's already there */
> +      /* ### TODO: Maybe verify the new file matches the existing one. */
> +      SVN_ERR(svn_io_remove_file2(b->tempfile_abspath, FALSE, scratch_pool));
> +    }
> +  else
> +    {
> +      SVN_ERR(svn_io_file_rename(b->tempfile_abspath, b->pristine_abspath,
> +                                 scratch_pool));
> +    }
...
> +}

> +pristine_remove_if_unreferenced_txn(void *baton,
> +                                    svn_sqlite__db_t *sdb,
> +                                    apr_pool_t *scratch_pool)
>  {
...
> +  /* If we removed the DB row, then remove the file. */
>    if (affected_rows > 0)
>      {
> -      /* Remove the file. */
> -      SVN_ERR(get_pristine_fname(&pristine_abspath, wcroot->abspath,
> -                                 sha1_checksum, TRUE /* create_subdir */,
> -                                 scratch_pool, scratch_pool));
> -      SVN_ERR(svn_io_remove_file2(pristine_abspath, TRUE /* ignore_enoent */,
> +      /* ### TODO: If file not present, log a consistency error but return
> +       * success. */
> +      SVN_ERR(svn_io_remove_file2(b->pristine_abspath, FALSE /* 
> ignore_enoent */,
>                                    scratch_pool));

Shouldn't it be ignore_enoent=TRUE at least in release builds?

>      }
>  
>    return SVN_NO_ERROR;
>  }

Reply via email to