Julian Foad wrote:
> [...] There are
> currently two implementations of a "dump" editor and neither is
> immediately usable. The one in "svnadmin dump" is not written as a clean
> editor, and instead calls directly into the FS layer to fetch its data.
> The one in "svnrdump" is clean but does not support all the features of
> the other such as non-delta mode and verify mode.
For connecting to the WC and to shelving, it is not a problem that the
"svnrdump" dump editor lacks support for those features. It's a problem that
the implementation is located in the "svnrdump" executable rather than in a
library, but that is easy to overcome.
> => We need to unify these two implementations, to have a clean re-
> usable dump editor.
So moving the "svnrdump" implementation into the library can be the first step
(done: r1838835), and unifying can be the second step.
The attached patch 'unifying-dump-editors-1.patch' goes some way towards that.
> The delta edit driver in "svnadmin load" also does not seem to be shared
> with the one in "svnrdump load".
>
> => We need to unify these two implementations, to have a re-usable
> load edit-driver.
>
> To be clear, re-using the dump/load functionality is not a direct
> requirement for shelving; rather, it is an existing functionality that
> supports input and output of shelvable changes. By providing an input
> and output mechanism that can be attached to various APIs (repository,
> WC, and shelves) it is useful for testing that the APIs all work
> consistently.
Only by implementing more than one edit driver for each editor (and vice versa)
can we prove that the components are cleanly separated by an API and thus
re-usable.
Then there is the need for a test framework that can generate all different
combinations of changes and test each of the subsystems with all of those
changes.
These improvements are not just to benefit shelving. In the longer term, if we
are to make a major improvement like supporting moves/renames properly, in my
opinion we need to do it starting from a baseline where we consistently and
cleanly use APIs that encode the current semantics of versioned changes, and
then migrate those.
--
- Julian
Towards unifying the two repository dump editor implementations.
('Dump editor' here means a dump-stream writer driven through the
svn_delta_editor_t API.)
The older dump editor in 'dump.c' supports 'svnadmin dump' and 'svnadmin
verify'. The newer dump editor in 'dump_editor.c' supports 'svnrdump dump'.
The latter can only produce delta-mode dumps.
This patch extracts some of the ancillary functionality (some kinds of
verification) out of the original dump editor into a separate wrapper, and
applies this verification wrapper to either the original dump editor or the
newer one. This brings us closer to being able to use the newer dump editor
as a drop-in replacement for the old one.
### So far, only 2 of the 3 checks performed in dump mode are moved to
the new checking wrapper: fspath and copyfrom, but not mergeinfo.
* subversion/libsvn_repos/dump.c
(check_mergeinfo_revisions,
verify_fspath,
check_for_old_copy_reference): New, extracted from dump_node().
(edit_baton,
dump_node,
get_dump_editor): Remove the three checks that are moved to the wrapper,
and their related parameters.
(rc_*,
get_ref_checking_editor): New: a reference-checking wrapper editor
implementing those three checks.
(get_dump_only_editor): New function. Wraps the basic editor with the new
reference-checking wrapper editor.
### For now, calls the newer dump editor in 'deltas' mode, else the
older one; thus in deltas mode the mergeinfo revs check is missing.
(svn_repos_dump_fs4): Call get_dump_only_editor() instead of
get_gump_editor().
(get_verify_editor): New function, wrapping get_dump_editor().
(verify_one_revision): Call get_verify_editor() instead of
get_gump_editor().
--This line, and those below, will be ignored--
Index: subversion/libsvn_repos/dump.c
===
--- subversion/libsvn_repos/dump.c (revision 1838836)
+++ subversion/libsvn_repos/dump.c (working copy)
@@ -651,16 +651,12 @@ struct edit_baton
/* True if checking UCS normalization during a verify. */
svn_boolean_t check_normalization;
/* The first revision dumped in this dumpstream. */
svn_revnum_t oldest_dumped_rev;
- /* If not NULL, set to true if any references to revisions older than
- OLDEST_DUMPED_REV were found in the dumpstream. */
- svn_boolean_t *found_old_reference;
-
/* If not NULL, set to true if any mergeinfo was dumped which contains
revisions older than OLDEST_DUMPED_REV. */
svn_boolean_t *found_old_mergeinfo;
/* Structure allows us to verify the paths currently being dumped.
If NULL, validity checks are being skipped. */
@@ -874,12 +870,43 @@ verify_mergeinfo_revisions(svn_boolean_t
