C. Michael Pilato wrote:
> C. Michael Pilato wrote:
>> C. Michael Pilato wrote:
>>> Specifically in this case, svn_fs_base__dag_commit_txn() (which is
>>> wrapped with a BDB transaction) calls
>>> svn_fs_mergeinfo__update_index(), which in turn calls
>>> index_txn_mergeinfo(), which calls (UH-OH!) svn_fs_revision_root().
>>>
>>> I started to fix this weeks ago on the reintegrate branch, but
>>> stopped because that branch took a turn towards removing sqlite from
>>> BDB altogether.
>>
>> I'm testing a patch for this right now. I hope it's temporary and
>> that David Glasser (or someone else) will be able to return to the
>> task of de-sqlite-ing our BDB mergeinfo logic Real Soon Now(tm).
>
> Oh, geez. I think there are other circular call paths to fix, too, as I
> see the mergeinfo util code also calls svn_fs_node_prop().
Attached is a patch that partially fixes the problems. But it's useless
without the fix to the svn_fs_node_prop-calling-codepath, too. I'm still
hoping this code will just go completely away soon, so I'm not going to
attack the rest of the problem right now.
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Fix abort()s triggered by illegal libsvn_fs_base call stacks (nested
BDB transactions are an absolute no-no!).
Sadly, this doesn't fix them all, as there is still a call to
svn_fs_node_prop() from inside the sqlite utility code. :-(
* subversion/include/private/svn_fs_mergeinfo.h
(svn_fs_mergeinfo__update_index): Add 'prev_rev_root' parameter,
* subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
(index_txn_mergeinfo): Add 'prev_rev_root' parameter, and use it
instead of generating a new root object.
(svn_fs_mergeinfo__update_index): Add 'prev_rev_root' parameter,
passed to updated call to index_txn_mergeinfo().
* subversion/libsvn_fs_base/dag.c
(svn_fs_base__dag_commit_txn): Don't do mergeinfo index updates here.
* subversion/libsvn_fs_base/tree.c
(txn_body_commit): Instead, update mergeinfo indexes here.
* subversion/libsvn_fs_fs/fs_fs.c
(commit_body): Update call to svn_fs_mergeinfo__update_index().
Index: subversion/libsvn_fs_base/tree.c
===================================================================
--- subversion/libsvn_fs_base/tree.c (revision 29007)
+++ subversion/libsvn_fs_base/tree.c (working copy)
@@ -41,6 +41,7 @@
#include "svn_mergeinfo.h"
#include "svn_fs.h"
#include "svn_sorts.h"
+#include "svn_hash.h"
#include "fs.h"
#include "err.h"
#include "trail.h"
@@ -2372,10 +2373,12 @@
svn_fs_txn_t *txn = args->txn;
svn_fs_t *fs = txn->fs;
const char *txn_name = txn->id;
-
svn_revnum_t youngest_rev;
const svn_fs_id_t *y_rev_root_id;
dag_node_t *txn_base_root_node;
+ apr_hash_t *txnprops;
+ const svn_string_t *target_mergeinfo;
+ apr_pool_t *pool = trail->pool;
/* Getting the youngest revision locks the revisions table until
this trail is done. */
@@ -2408,10 +2411,33 @@
discovered locks. */
SVN_ERR(verify_locks(txn_name, trail, trail->pool));
- /* Else, commit the txn. */
+ /* Check for mergeinfo before finalizing the commit. */
+ SVN_ERR(svn_fs_base__txn_proplist_in_trail(&txnprops, txn_name, trail));
+ target_mergeinfo = apr_hash_get(txnprops, SVN_FS__PROP_TXN_MERGEINFO,
+ APR_HASH_KEY_STRING);
+
+ /* Now commit the txn. */
SVN_ERR(svn_fs_base__dag_commit_txn(&(args->new_rev), txn, trail,
trail->pool));
+ /* If there was mergeinfo, update the mergeinfo index. */
+ if (target_mergeinfo)
+ {
+ apr_hash_t *mergeinfo = apr_hash_make(pool);
+ svn_fs_root_t *prev_rev_root;
+ struct revision_root_args rr_args;
+ svn_stream_t *stream =
+ svn_stream_from_stringbuf(svn_stringbuf_create_from_string
+ (target_mergeinfo, pool), pool);
+
+ rr_args.root_p = &prev_rev_root;
+ rr_args.rev = args->new_rev;
+ SVN_ERR(txn_body_revision_root(&rr_args, trail));
+ SVN_ERR(svn_hash_read2(mergeinfo, stream, NULL, pool));
+ SVN_ERR(svn_fs_mergeinfo__update_index(txn, args->new_rev,
+ prev_rev_root, mergeinfo, pool));
+ }
+
return SVN_NO_ERROR;
}
Index: subversion/libsvn_fs_base/dag.c
===================================================================
--- subversion/libsvn_fs_base/dag.c (revision 29007)
+++ subversion/libsvn_fs_base/dag.c (working copy)
@@ -1433,7 +1433,6 @@
apr_hash_t *txnprops;
svn_fs_t *fs = txn->fs;
const char *txn_id = txn->id;
- const svn_string_t *target_mergeinfo;
/* Remove any temporary transaction properties initially created by
begin_txn(). */
@@ -1444,19 +1443,9 @@
*new_rev = SVN_INVALID_REVNUM;
SVN_ERR(svn_fs_bdb__put_rev(new_rev, fs, &revision, trail, pool));
- target_mergeinfo = apr_hash_get(txnprops, SVN_FS__PROP_TXN_MERGEINFO,
- APR_HASH_KEY_STRING);
- if (target_mergeinfo)
- {
- svn_stringbuf_t *buf =
- svn_stringbuf_create_from_string(target_mergeinfo, pool);
- svn_stream_t *stream = svn_stream_from_stringbuf(buf, pool);
- apr_hash_t *mergeinfo = apr_hash_make(pool);
- SVN_ERR(svn_hash_read2(mergeinfo, stream, NULL, pool));
- SVN_ERR(svn_fs_mergeinfo__update_index(txn, *new_rev, mergeinfo, pool));
- SVN_ERR(svn_fs_base__set_txn_prop
- (fs, txn_id, SVN_FS__PROP_TXN_MERGEINFO, NULL, trail, pool));
- }
+ if (apr_hash_get(txnprops, SVN_FS__PROP_TXN_MERGEINFO, APR_HASH_KEY_STRING))
+ SVN_ERR(svn_fs_base__set_txn_prop
+ (fs, txn_id, SVN_FS__PROP_TXN_MERGEINFO, NULL, trail, pool));
if (apr_hash_get(txnprops, SVN_FS__PROP_TXN_CHECK_OOD, APR_HASH_KEY_STRING))
SVN_ERR(svn_fs_base__set_txn_prop
Index: subversion/libsvn_fs_util/mergeinfo-sqlite-index.c
===================================================================
--- subversion/libsvn_fs_util/mergeinfo-sqlite-index.c (revision 29007)
+++ subversion/libsvn_fs_util/mergeinfo-sqlite-index.c (working copy)
@@ -200,15 +200,13 @@
static svn_error_t *
index_txn_mergeinfo(sqlite3 *db,
svn_revnum_t new_rev,
+ svn_fs_root_t *prev_rev_root,
apr_hash_t *mergeinfo_for_paths,
svn_fs_t *fs,
apr_pool_t *pool)
{
apr_hash_index_t *hi;
- svn_fs_root_t *old_root;
- SVN_ERR(svn_fs_revision_root(&old_root, fs, new_rev - 1, pool));
-
for (hi = apr_hash_first(pool, mergeinfo_for_paths);
hi != NULL;
hi = apr_hash_next(hi))
@@ -217,8 +215,8 @@
void *mergeinfo;
apr_hash_this(hi, &path, NULL, &mergeinfo);
- SVN_ERR(index_path_mergeinfo(new_rev, db, (const char *) path,
- (svn_string_t *) mergeinfo, old_root, pool));
+ SVN_ERR(index_path_mergeinfo(new_rev, db, path, mergeinfo,
+ prev_rev_root, pool));
}
return SVN_NO_ERROR;
}
@@ -292,7 +290,9 @@
revision number as NEW_REV, and if the current transaction contains
mergeinfo, record it. */
svn_error_t *
-svn_fs_mergeinfo__update_index(svn_fs_txn_t *txn, svn_revnum_t new_rev,
+svn_fs_mergeinfo__update_index(svn_fs_txn_t *txn,
+ svn_revnum_t new_rev,
+ svn_fs_root_t *old_rev_root,
apr_hash_t *mergeinfo_for_paths,
apr_pool_t *pool)
{
@@ -306,17 +306,14 @@
/* Clean up old data. (If we're going to write to the DB anyway,
there's no reason to do extra checks to avoid no-op DELETEs.) */
- err = clean_tables(db,
- new_rev,
- (mergeinfo_for_paths == NULL),
- subpool);
+ err = clean_tables(db, new_rev, (mergeinfo_for_paths == NULL), subpool);
MAYBE_CLEANUP;
/* Record any mergeinfo from the current transaction. */
if (mergeinfo_for_paths)
{
- err = index_txn_mergeinfo(db, new_rev, mergeinfo_for_paths, txn->fs,
- subpool);
+ err = index_txn_mergeinfo(db, new_rev, old_rev_root,
+ mergeinfo_for_paths, txn->fs, subpool);
MAYBE_CLEANUP;
}
Index: subversion/include/private/svn_fs_mergeinfo.h
===================================================================
--- subversion/include/private/svn_fs_mergeinfo.h (revision 29007)
+++ subversion/include/private/svn_fs_mergeinfo.h (working copy)
@@ -33,17 +33,23 @@
/* Update the mergeinfo index according to the changes made in
- transaction TXN for revision NEW_REV. MERGEINFO_FOR_PATHS is the
- mergeinfo for each path changed in the transaction (a mapping of
- const char * -> svn_string_t *), or NULL if there was no mergeinfo
- recorded for that transaction. Use POOL for any temporary allocations.
+ transaction TXN for the revision NEW_REV. MERGEINFO_FOR_PATHS is
+ the mergeinfo for each path changed in the transaction (a mapping
+ of const char * -> svn_string_t *), or NULL if there was no
+ mergeinfo recorded for that transaction.
+
+ PREV_REV_ROOT is a revision root constructed for the revision
+ NEW_REV - 1.
+ Use POOL for any temporary allocations.
+
NOTE: Even if there is no mergeinfo, this function should be
called to make sure there is no stray mergeinfo for this revision
left from a previous failed commit. */
svn_error_t *
svn_fs_mergeinfo__update_index(svn_fs_txn_t *txn,
svn_revnum_t new_rev,
+ svn_fs_root_t *prev_rev_root,
apr_hash_t *mergeinfo_for_paths,
apr_pool_t *pool);
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 29007)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -4997,6 +4997,7 @@
const char *revprop_filename, *final_revprop;
const svn_fs_id_t *root_id, *new_root_id;
const char *start_node_id, *start_copy_id;
+ svn_fs_root_t *prev_rev_root;
svn_revnum_t old_rev, new_rev;
apr_file_t *proto_file;
void *proto_file_lockcookie;
@@ -5130,8 +5131,9 @@
old_rev_filename, pool));
/* Update the merge tracking information index. */
- SVN_ERR(svn_fs_mergeinfo__update_index(cb->txn, new_rev, target_mergeinfo,
- pool));
+ SVN_ERR(svn_fs_revision_root(&prev_rev_root, cb->fs, new_rev - 1, pool));
+ SVN_ERR(svn_fs_mergeinfo__update_index(cb->txn, new_rev, prev_rev_root,
+ target_mergeinfo, pool));
/* Update the 'current' file. */
SVN_ERR(write_final_current(cb->fs, cb->txn->id, new_rev, start_node_id,
Received on 2008-01-24 17:14:50 CET