Julian Foad wrote on Sat, 10 Oct 2009 at 01:55 +0100:
> Julian Foad wrote:
> > Here is the patch I'm planning to commit initially. I'll fill in the log
> > message first.
>
> Thanks for the comments on IRC. In the revised patch attached, printf's
> are gone, fs_fs code updated with today's changes, file licence header
> added. Inserting the "obliterate" entry somewhere in the middle of the
> repos/fs vtables should be fine because those are private vtables so no
> API versioning needed and all code using them is built at once.
*nod*.
> By the way I think it's rotten to post a patch without a proper log
> message but I'm doing it again. My log message will basically say "New
> function" or "New entry in vtable" or "New test" for every change. There
> is not a single modification or deletion.
>
I don't see a problem. Seeing a thousand "New function" lines in the
log message would teach me nothing --- I would skip reading it.
> Add an initial skeleton implementation for "svn obliterate", with the UI
> and public API changes hidden behind #ifdef SVN_WITH_EXPERIMENTAL_OBLITERATE.
>
> ...
>
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c (revision 39914)
> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> @@ -5885,6 +6025,45 @@ svn_fs_fs__commit(svn_revnum_t *new_rev_
> }
>
> svn_error_t *
> +svn_fs_fs__commit_obliteration(svn_revnum_t rev,
> + svn_fs_t *fs,
> + svn_fs_txn_t *txn,
> + apr_pool_t *pool)
> +{
> + struct commit_baton cb;
> + fs_fs_data_t *ffd = fs->fsap_data;
> +
> + /* Analogous to svn_fs_fs__commit(). */
> + cb.new_rev_p = &rev;
> + cb.fs = fs;
> + cb.txn = txn;
> +
> + if (ffd->rep_sharing_allowed)
> + {
> + cb.reps_to_cache = apr_array_make(pool, 5, sizeof(representation_t *));
> + cb.reps_pool = pool;
> + }
> + else
> + {
> + cb.reps_to_cache = NULL;
> + cb.reps_pool = NULL;
> + }
> +
> + SVN_ERR(svn_fs_fs__with_write_lock(fs, commit_obliteration_body, &cb, pool));
> +
> + if (ffd->rep_sharing_allowed)
> + {
> + /* ### TODO: ignore errors opening the DB (issue #3506) * */
> + SVN_ERR(svn_fs_fs__open_rep_cache(fs, pool));
> + SVN_ERR(svn_sqlite__with_transaction(ffd->rep_cache_db,
> + commit_sqlite_txn_callback,
> + &cb, pool));
> + }
> +
[ I think you know this already, but I'll say it anyway. ]
Half of the lines of this function deal with storing new reps in the
rep-cache DB. Some day it should also remove the old DB rows (and
somehow cause references in other revisions into the revision-being-
obliterated to still work).
> + return SVN_NO_ERROR;
> +}
> Index: subversion/libsvn_fs/fs-loader.c
> ===================================================================
> --- subversion/libsvn_fs/fs-loader.c (revision 39914)
> +++ subversion/libsvn_fs/fs-loader.c (working copy)
> @@ -678,6 +688,31 @@ svn_fs_commit_txn(const char **conflict_
> }
>
> svn_error_t *
> +svn_fs__commit_obliteration_txn(svn_revnum_t rev, svn_fs_txn_t *txn,
> + apr_pool_t *pool)
> +{
> +#ifdef PACK_AFTER_EVERY_COMMIT
> + svn_fs_root_t *txn_root;
> + svn_fs_t *fs;
> + const char *fs_path;
> +
> + SVN_ERR(svn_fs_txn_root(&txn_root, txn, pool));
> + fs = svn_fs_root_fs(txn_root);
> + fs_path = svn_fs_path(fs, pool);
> +#endif
> +
> + SVN_ERR(txn->vtable->commit_obliteration(rev, txn, pool));
> +
> +#ifdef PACK_AFTER_EVERY_COMMIT
> + /* ### Can't do the normal packing: this isn't a normal commit.
> + * SVN_ERR(svn_fs_pack(fs_path, NULL, NULL, NULL, NULL, pool)); */
> + return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL, NULL);
> +#endif
> +
There is no reason to run svn_fs_pack() here because obliteration
doesn't create a new youngest revision. You can probably kill
the #ifdef..#endif parts of this function.
(The reason PACK_AFTER_EVERY_COMMIT exists is to make it discover
concurrency issues in the packing code; see, for example,
update_min_unpacked_rev() in <fs_fs.c>. It is not part of the API.)
> + return SVN_NO_ERROR;
> +}
> Index: subversion/include/private/svn_fs_private.h
> ===================================================================
> --- subversion/include/private/svn_fs_private.h (revision 39914)
> +++ subversion/include/private/svn_fs_private.h (working copy)
> @@ -58,6 +58,37 @@ extern "C" {
> apr_hash_t *
> svn_fs__access_get_lock_tokens(svn_fs_access_t *access_ctx);
>
> +
> +/**
> + * Same as svn_fs_begin_txn2(), except it begins an obliteration-txn
> + * that can be used to replace revision @a rev. @a rev must be a valid
> + * revision number at the time of this call. This transaction cannot be
> + * committed with a normal commit but only with an "obliterate commit" - see
> + * svn_fs_commit_obliterate_txn().
s/svn_fs_commit_obliterate_txn/svn_fs__commit_obliterate_txn/
> Index: subversion/libsvn_client/obliterate.c
> ===================================================================
> --- subversion/libsvn_client/obliterate.c (revision 0)
> +++ subversion/libsvn_client/obliterate.c (revision 0)
> + if (ctx->notify_func2)
> + {
> + svn_wc_notify_t *notify
> + = svn_wc_create_notify_url(url, svn_wc_notify_delete, pool);
svn_wc_notify_obliterate?
> Index: subversion/tests/libsvn_fs/fs-test.c
> ===================================================================
> --- subversion/tests/libsvn_fs/fs-test.c (revision 39914)
> +++ subversion/tests/libsvn_fs/fs-test.c (working copy)
> @@ -31,6 +31,7 @@
> #include "svn_time.h"
> #include "svn_string.h"
> #include "svn_fs.h"
> +#include "private/svn_fs_private.h"
Usually all private includes come after all public includes, not mixed with
them.
> #include "svn_checksum.h"
> #include "svn_mergeinfo.h"
> #include "svn_props.h"
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2405848
Received on 2009-10-10 14:32:09 CEST