More issue #745 work. Now, I think I'm finished.
* subversion/include/svn_fs.h
(svn_fs_path_change_kind_t): Document values.
(svn_fs_path_change_t): Document members.
* subversion/libsvn_fs/tree.c
(undelete_change): New helper function for undoing a deletion change.
(merge): Remove the "delete" change whenever this transaction wasn't
actually responsible for it (it was a safe merge from a prior
commit). Also, update docstring outline to reflect what the code is
actually doing in some places.
* subversion/libsvn_fs/bdb/changes-table.c
(svn_fs__open_changes_table): Remove prophetic pseudo-code.
(make_change): Change docstring to reflect the Truth.
git-svn-id: https://svn.apache.org/repos/asf/subversion/branches/issue-745-dev@842560 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/subversion/include/svn_fs.h b/subversion/include/svn_fs.h
index 413cb9f..cea2790 100644
--- a/subversion/include/svn_fs.h
+++ b/subversion/include/svn_fs.h
@@ -554,26 +554,29 @@
/* The kind of change that occured on the path. */
typedef enum
{
- svn_fs_path_change_modify = 0,
- svn_fs_path_change_add,
- svn_fs_path_change_delete,
- svn_fs_path_change_replace,
- svn_fs_path_change_reset
+ svn_fs_path_change_modify = 0, /* default value */
+ svn_fs_path_change_add, /* path added in txn */
+ svn_fs_path_change_delete, /* path removed in txn */
+ svn_fs_path_change_replace, /* path removed and re-added in txn */
+ svn_fs_path_change_reset /* ignore all previous change items for
+ path (internal-use only) */
} svn_fs_path_change_kind_t;
+/* Change descriptor. */
typedef struct svn_fs_path_change_t
{
- const svn_fs_id_t *node_rev_id;
- svn_fs_path_change_kind_t change_kind;
- int text_mod;
- int prop_mod;
+ const svn_fs_id_t *node_rev_id; /* node revision id of changed path */
+ svn_fs_path_change_kind_t change_kind; /* kind of change (see above) */
+ int text_mod; /* were there text mods? */
+ int prop_mod; /* were there property mods? */
} svn_fs_path_change_t;
+
/* Allocate and return a hash *CHANGED_PATHS_P containing descriptions
- of the paths changed under ROOT. The hash is keyed with const char
- * paths, and has svn_fs_path_change_t * values. Use POOL for all
+ of the paths changed under ROOT. The hash is keyed with const char *
+ paths, and has svn_fs_path_change_t * values. Use POOL for all
allocations, including the hash and its values. */
svn_error_t *svn_fs_paths_changed (apr_hash_t **changed_paths_p,
svn_fs_root_t *root,
diff --git a/subversion/libsvn_fs/bdb/changes-table.c b/subversion/libsvn_fs/bdb/changes-table.c
index 7fbff49..9fee95f 100644
--- a/subversion/libsvn_fs/bdb/changes-table.c
+++ b/subversion/libsvn_fs/bdb/changes-table.c
@@ -45,17 +45,6 @@
one-per-row. Note: this must occur before ->open(). */
DB_ERR (changes->set_flags (changes, DB_DUP));
-#if 0
- /* ### Someday we might enable sorting of the items as they are
- added to the table. This will add cost to the addition of new
- things to the table (a place where we might not want any more
- cost), but would greatly enhance the efficiency of reading back
- the changes (in that all the changes related to a given path, or
- node-revision-id, would be grouped together). */
- DB_ERR (changes->set_flags (changes, DB_DUPSORT));
- DB_ERR (changes->set_dup_sort (changes, not_yet_written_sort_function);
-#endif
-
DB_ERR (changes->open (changes, "changes", 0, DB_BTREE,
create ? (DB_CREATE | DB_EXCL) : 0,
0666));
@@ -113,7 +102,8 @@
}
-/* Return a deep copy of CHANGE alloced in POOL. */
+/* Make a public change structure from an internal one, allocating the
+ structure, and copies of necessary members, POOL. */
static svn_fs_path_change_t *
make_change (svn_fs__change_t *change,
apr_pool_t *pool)
diff --git a/subversion/libsvn_fs/tree.c b/subversion/libsvn_fs/tree.c
index 6568408..2bff6fa 100644
--- a/subversion/libsvn_fs/tree.c
+++ b/subversion/libsvn_fs/tree.c
@@ -1294,6 +1294,55 @@
return SVN_NO_ERROR;
}
+
+/* Possibly */
+static svn_error_t *
+undelete_change (svn_fs_t *fs,
+ const char *path,
+ const char *txn_id,
+ trail_t *trail)
+{
+ apr_hash_t *changes;
+ svn_fs_path_change_t *this_change;
+
+ /* Canonicalize PATH. */
+ path = svn_fs__canonicalize_abspath (path, trail->pool);
+
+ /* First, get the changes associated with TXN_ID. */
+ SVN_ERR (svn_fs__changes_fetch (&changes, fs, txn_id, trail));
+
+ /* Now, do any of those changes apply to path and indicate deletion? */
+ this_change = apr_hash_get (changes, path, APR_HASH_KEY_STRING);
+ if (this_change
+ && ((this_change->change_kind == svn_fs_path_change_delete)
+ || (this_change->change_kind == svn_fs_path_change_replace)))
+ {
+ /* If so, reset the changes and re-add everything except the
+ deletion. */
+ SVN_ERR (add_change (fs, txn_id, path, NULL,
+ svn_fs_path_change_reset, 0, 0, trail));
+ if (this_change->change_kind == svn_fs_path_change_replace)
+ {
+ SVN_ERR (add_change (fs, txn_id, path, this_change->node_rev_id,
+ svn_fs_path_change_add, this_change->text_mod,
+ this_change->prop_mod, trail));
+ }
+ }
+ else
+ {
+ /* Else, this function was called in error, OR something is not
+ as we expected it to be in the changes table. */
+ return svn_error_createf
+ (SVN_ERR_FS_CORRUPT, 0, NULL, trail->pool,
+ "undelete_change: no deletion changes for path `%s' "
+ "in transaction `%s' of filesystem `%s'",
+ path, txn_id, fs->path);
+ }
+
+ return SVN_NO_ERROR;
+}
+
+
/* Merge changes between ANCESTOR and SOURCE into TARGET, as part of
* TRAIL. ANCESTOR and TARGET must be distinct node revisions.
* TARGET_PATH should correspond to TARGET's full path in its
@@ -1408,14 +1457,21 @@
* }
* }
* else if (E exists in source but not target)
- * add same entry to target, pointing to source entry's id;
+ * {
+ * if (E changed between ancestor and source)
+ * conflict;
+ * else if (E did not change between ancestor and source)
+ * // do nothing
* else if (E exists in target but not source)
* {
* if (E points the same node rev in target and ancestor)
* delete E from target;
* else // E points to different node revs in target & ancestor
* {
- * conflict;
+ * if (E in target is not related to E in ancestor)
+ * conflict;
+ * else
+ * // do nothing
* }
* }
* else
@@ -1727,18 +1783,30 @@
(SVN_ERR_FS_CONFLICT, 0, NULL, trail->pool,
"conflict at \"%s\"", conflict_p->data);
}
+ else
+ {
+ /* It's a double delete (plus an add), so do nothing
+ except un-record the deletion of E so that this
+ transaction isn't given credit for that portion of
+ this change. */
+ SVN_ERR (undelete_change (fs, svn_path_join (target_path,
+ t_entry->name,
+ trail->pool),
+ txn_id, trail));
+ }
}
/* E exists in neither target nor source */
else
{
- /* It's a double delete, so do nothing except effectively
- erase all the changes associated with E so that this
- transaction isn't given credit for the deletion of E.
- ### kff todo: what about the rename case? */
- SVN_ERR (add_change
- (fs, txn_id,
- svn_path_join (target_path, a_entry->name, trail->pool),
- NULL, svn_fs_path_change_reset, 0, 0, trail));
+ /* It's a double delete, so do nothing except un-record the
+ deletion of E so that this transaction isn't given credit
+ for that change. */
+ SVN_ERR (undelete_change (fs, svn_path_join (target_path,
+ t_entry->name,
+ trail->pool),
+ txn_id, trail));
+
+ /* ### kff todo: what about the rename case? */
}
/* We've taken care of any possible implications E could have.