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

Re: svn commit: r1073366 - in /subversion/trunk: notes/wc-ng/pristine-store subversion/libsvn_wc/wc-queries.sql subversion/libsvn_wc/wc_db_pristine.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 23 Feb 2011 18:31:28 +0200

julianfoad_at_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;
> }
Received on 2011-02-23 17:37:15 CET

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