[PATCH] Add '--include' and '--exclude' options to 'svnadmin dump'

Discussion:

Sergey Raevskiy

2016-10-12 10:43:40 UTC

Hi!

I've attached a patch that adds '--include/--exclude' options to 'svnadmin
dump'. These options work similarly to 'svndumpfilter include/exclude'
but provide proper handling of 'copy from' paths.

Consider the following example with svndumpfilter:
[[
$ svnadmin create /repo
$ svn mkdir -m "" file:///repo/A
$ svn copy -m "" file:///repo/A file:///repo/B
$ svnadmin dump /repo | svndumpfilter include /B > dump
...
Revision 0 committed as 0.
Revision 1 committed as 1.
svndumpfilter: E200003: Invalid copy source path '/A'
]]

The 'svnadmin dump' with proposed include/exclude options can handle this use
case properly:
[[
$ svnadmin dump /repo --include /B > dump
* Dumped revision 0.
* Dumped revision 1.
* Dumped revision 2.
]]

The proposed options are implemented using 'svn_repos_authz_func_t' and the
not-included copy sources are hidden by the 'svn_repos' layer.

Some usage examples for the proposed options:
[[
$ svnadmin dump repos --include /calc > calc-dumpfile
...
$ svnadmin dump repos --include /calc --include /calendar > dumpfile
...
$ svnadmin dump repos --exclude /secret > dumpfile
...
$ svnadmin dump repos --include /calc --exclude /calendar > dumpfile
svnadmin: E205000: Try 'svnadmin help' for more info
svnadmin: E205000: '--exclude' and '--include' options cannot be used
simultaneously

$ svnadmin dump repos --include /cal* --pattern > dumpfile
...
]]

Log message:

[[
Add '--include' and '--exclude' options to 'svnadmin dump'.

* include/svn_repos.h
(svn_repos_dump_filter_func_t): New.
(svn_repos_dump_fs4): Update function signature and comment.
(svn_repos_dump_fs3): Update comment.

* libsvn_repos/deprecated.c
(svn_repos_dump_fs3): Update caller.

* libsvn_repos/dump.c
(write_revision_record): Call to svn_repos_fs_revision_proplist() (with
AUTHZ_FUNC) instead of svn_fs_revision_proplist2() to filter revision
properties as well as revisions. Update comment.
(dump_filter_baton_t,
dump_filter_authz_func): New.
(svn_repos_dump_fs4): Initialize and pass AUTHZ_FUNC and AUTHZ_BATON to
the repos layer API if FILTER_FUNC is specified by caller.

* subversion/svnadmin/svnadmin.c
(svnadmin__cmdline_options_t): Add enum values for new options.
(options_table): Add new options.
(cmd_table): Add new options to 'dump' subcommand.
(svnadmin_opt_state): Add new fields to represent new options.
(ary_prefix_match): New. Copied from svndumpfilter.
(dump_filter_baton_t,
dump_filter_func): New.
(subcommand_dump): Initialize FILTER_BATON. Pass DUMP_FILTER_FUNC and a
pointer to FILTER_BATON to svn_repos_dump_fs() if any filtering prefixes
specified.
(sub_main): Handle new options.

* subversion/tests/cmdline/svnadmin_tests.py
(dump_exclude,
dump_exclude_copysource,
dump_include,
dump_not_include_copysource,
dump_exclude_by_pattern,
dump_include_by_pattern,
dump_exclude_all_rev_changes,
dump_invalid_filtering_option): New.
(test_list): Add new tests to table.

* subversion/tests/libsvn_repos/dump-load-test.c
(test_dump_bad_props): Update caller.

Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]

Bert Huijben

2016-10-12 11:10:05 UTC

Permalink

-----Original Message-----
Sent: woensdag 12 oktober 2016 12:44
Subject: [PATCH] Add '--include' and '--exclude' options to 'svnadmin dump'
Hi!
I've attached a patch that adds '--include/--exclude' options to 'svnadmin
dump'. These options work similarly to 'svndumpfilter include/exclude'
but provide proper handling of 'copy from' paths.
[[
$ svnadmin create /repo
$ svn mkdir -m "" file:///repo/A
$ svn copy -m "" file:///repo/A file:///repo/B
$ svnadmin dump /repo | svndumpfilter include /B > dump

I don't see a --renumber-revs on your invocation of svndumpfilter. Did you try these options to see if these resolve your problem (they should)

$ svndumpfilter help include
include: Filter out nodes without given prefixes from dumpstream.
usage: svndumpfilter include PATH_PREFIX...

Valid options:
--drop-empty-revs : Remove revisions emptied by filtering.
--drop-all-empty-revs : Remove all empty revisions found in dumpstream
except revision 0.
--renumber-revs : Renumber revisions left after filtering.
--skip-missing-merge-sources : Skip missing merge sources.
--targets ARG : Read additional prefixes, one per line, from
file ARG.
--preserve-revprops : Don't filter revision properties.
--quiet : Do not display filtering statistics.
--pattern : Treat the path prefixes as file glob patterns.

Bert

...
Revision 0 committed as 0.
Revision 1 committed as 1.
svndumpfilter: E200003: Invalid copy source path '/A'
]]
The 'svnadmin dump' with proposed include/exclude options can handle this use
[[
$ svnadmin dump /repo --include /B > dump
* Dumped revision 0.
* Dumped revision 1.
* Dumped revision 2.
]]
The proposed options are implemented using 'svn_repos_authz_func_t' and the
not-included copy sources are hidden by the 'svn_repos' layer.
[[
$ svnadmin dump repos --include /calc > calc-dumpfile
...
$ svnadmin dump repos --include /calc --include /calendar > dumpfile
...
$ svnadmin dump repos --exclude /secret > dumpfile
...
$ svnadmin dump repos --include /calc --exclude /calendar > dumpfile
svnadmin: E205000: Try 'svnadmin help' for more info
svnadmin: E205000: '--exclude' and '--include' options cannot be used
simultaneously
$ svnadmin dump repos --include /cal* --pattern > dumpfile
...
]]
[[
Add '--include' and '--exclude' options to 'svnadmin dump'.
* include/svn_repos.h
(svn_repos_dump_filter_func_t): New.
(svn_repos_dump_fs4): Update function signature and comment.
(svn_repos_dump_fs3): Update comment.
* libsvn_repos/deprecated.c
(svn_repos_dump_fs3): Update caller.
* libsvn_repos/dump.c
(write_revision_record): Call to svn_repos_fs_revision_proplist() (with
AUTHZ_FUNC) instead of svn_fs_revision_proplist2() to filter revision
properties as well as revisions. Update comment.
(dump_filter_baton_t,
dump_filter_authz_func): New.
(svn_repos_dump_fs4): Initialize and pass AUTHZ_FUNC and AUTHZ_BATON to
the repos layer API if FILTER_FUNC is specified by caller.
* subversion/svnadmin/svnadmin.c
(svnadmin__cmdline_options_t): Add enum values for new options.
(options_table): Add new options.
(cmd_table): Add new options to 'dump' subcommand.
(svnadmin_opt_state): Add new fields to represent new options.
(ary_prefix_match): New. Copied from svndumpfilter.
(dump_filter_baton_t,
dump_filter_func): New.
(subcommand_dump): Initialize FILTER_BATON. Pass DUMP_FILTER_FUNC and a
pointer to FILTER_BATON to svn_repos_dump_fs() if any filtering prefixes
specified.
(sub_main): Handle new options.
* subversion/tests/cmdline/svnadmin_tests.py
(dump_exclude,
dump_exclude_copysource,
dump_include,
dump_not_include_copysource,
dump_exclude_by_pattern,
dump_include_by_pattern,
dump_exclude_all_rev_changes,
dump_invalid_filtering_option): New.
(test_list): Add new tests to table.
* subversion/tests/libsvn_repos/dump-load-test.c
(test_dump_bad_props): Update caller.
Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]

Sergey Raevskiy

2016-10-12 12:58:37 UTC

Permalink

Hi Bert, thanks for your reply.

The problem that I've described is not about revision numbers, it's about copied
nodes.

Suppose we have an repository 'repo' containing nodes '/A' and '/B',
where '/B' (or some path under '/B') was create by copying of '/A' (or some
path under '/A').

Now, if we run 'svndumpfilter include /B' against dump of 'repo', we will get
an error:

[[
...
svndumpfilter: E200003: Invalid copy source path '/A'
]]

This happens because 'svndumpfilter' tries to add copy of '/A', but node '/A'
is not included in destination dump.

Post by Bert Huijben

I don't see a --renumber-revs on your invocation of svndumpfilter. Did you try these options to see if these resolve your problem (they should)
$ svndumpfilter help include
include: Filter out nodes without given prefixes from dumpstream.
usage: svndumpfilter include PATH_PREFIX...
--drop-empty-revs : Remove revisions emptied by filtering.
--drop-all-empty-revs : Remove all empty revisions found in dumpstream
except revision 0.
--renumber-revs : Renumber revisions left after filtering.
--skip-missing-merge-sources : Skip missing merge sources.
--targets ARG : Read additional prefixes, one per line, from
file ARG.
--preserve-revprops : Don't filter revision properties.
--quiet : Do not display filtering statistics.
--pattern : Treat the path prefixes as file glob patterns.
Bert

Julian Foad

2016-10-28 15:15:06 UTC

Permalink

Post by Sergey Raevskiy
I've attached a patch that adds '--include/--exclude' options to 'svnadmin
dump'. These options work similarly to 'svndumpfilter include/exclude'
but provide proper handling of 'copy from' paths.

Thank you! As I'm sure you're aware, this feature has been wanted almost
forever. For example, over ten years ago

Ben Collins-Sussman wrote "History simply cannot be filtered by
filtering a stream; the filtering process sometimes needs random-access
to all of the history. [...] 'svndumpfilter' needs to die, and the
functionality needs to be pushed into 'svnadmin dump' directly." [1]

However, I don't have the capacity to review it. I hope someone can.

- Julian

[1] On users@, 2005-08-17, "Re: svndumpfilter: Invalid copy source
path", https://svn.haxx.se/users/archive-2005-08/0744.shtml

[...]

Post by Sergey Raevskiy
[[
Add '--include' and '--exclude' options to 'svnadmin dump'.
* include/svn_repos.h
(svn_repos_dump_filter_func_t): New.
(svn_repos_dump_fs4): Update function signature and comment.
(svn_repos_dump_fs3): Update comment.
* libsvn_repos/deprecated.c
(svn_repos_dump_fs3): Update caller.
* libsvn_repos/dump.c
(write_revision_record): Call to svn_repos_fs_revision_proplist() (with
AUTHZ_FUNC) instead of svn_fs_revision_proplist2() to filter revision
properties as well as revisions. Update comment.
(dump_filter_baton_t,
dump_filter_authz_func): New.
(svn_repos_dump_fs4): Initialize and pass AUTHZ_FUNC and AUTHZ_BATON to
the repos layer API if FILTER_FUNC is specified by caller.
* subversion/svnadmin/svnadmin.c
(svnadmin__cmdline_options_t): Add enum values for new options.
(options_table): Add new options.
(cmd_table): Add new options to 'dump' subcommand.
(svnadmin_opt_state): Add new fields to represent new options.
(ary_prefix_match): New. Copied from svndumpfilter.
(dump_filter_baton_t,
dump_filter_func): New.
(subcommand_dump): Initialize FILTER_BATON. Pass DUMP_FILTER_FUNC and a
pointer to FILTER_BATON to svn_repos_dump_fs() if any filtering prefixes
specified.
(sub_main): Handle new options.
* subversion/tests/cmdline/svnadmin_tests.py
(dump_exclude,
dump_exclude_copysource,
dump_include,
dump_not_include_copysource,
dump_exclude_by_pattern,
dump_include_by_pattern,
dump_exclude_all_rev_changes,
dump_invalid_filtering_option): New.
(test_list): Add new tests to table.
* subversion/tests/libsvn_repos/dump-load-test.c
(test_dump_bad_props): Update caller.
Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]

maxin

2017-10-10 22:14:56 UTC

Permalink

I am wondering if there has been any progress on this. I just ran into the
same problem.

--
Sent from: http://subversion.1072662.n5.nabble.com/Subversion-Dev-f4725.html

Julian Foad

2017-10-12 15:32:47 UTC

Permalink

This is really important. I'm going to make some time to review it.

- Julian

Post by Sergey Raevskiy
[[
$ svnadmin create /repo
$ svn mkdir -m "" file:///repo/A
$ svn copy -m "" file:///repo/A file:///repo/B
$ svnadmin dump /repo | svndumpfilter include /B > dump
...
Revision 0 committed as 0.
Revision 1 committed as 1.
svndumpfilter: E200003: Invalid copy source path '/A'
]]
The 'svnadmin dump' with proposed include/exclude options can handle this use
[[
$ svnadmin dump /repo --include /B > dump
* Dumped revision 0.
* Dumped revision 1.
* Dumped revision 2.
]]
The proposed options are implemented using 'svn_repos_authz_func_t' and the
not-included copy sources are hidden by the 'svn_repos' layer.
[[
$ svnadmin dump repos --include /calc > calc-dumpfile
...
$ svnadmin dump repos --include /calc --include /calendar > dumpfile
...
$ svnadmin dump repos --exclude /secret > dumpfile
...
$ svnadmin dump repos --include /calc --exclude /calendar > dumpfile
svnadmin: E205000: Try 'svnadmin help' for more info
svnadmin: E205000: '--exclude' and '--include' options cannot be used
simultaneously
$ svnadmin dump repos --include /cal* --pattern > dumpfile
...
]]
[[
Add '--include' and '--exclude' options to 'svnadmin dump'.
* include/svn_repos.h
(svn_repos_dump_filter_func_t): New.
(svn_repos_dump_fs4): Update function signature and comment.
(svn_repos_dump_fs3): Update comment.
* libsvn_repos/deprecated.c
(svn_repos_dump_fs3): Update caller.
* libsvn_repos/dump.c
(write_revision_record): Call to svn_repos_fs_revision_proplist() (with
AUTHZ_FUNC) instead of svn_fs_revision_proplist2() to filter revision
properties as well as revisions. Update comment.
(dump_filter_baton_t,
dump_filter_authz_func): New.
(svn_repos_dump_fs4): Initialize and pass AUTHZ_FUNC and AUTHZ_BATON to
the repos layer API if FILTER_FUNC is specified by caller.
* subversion/svnadmin/svnadmin.c
(svnadmin__cmdline_options_t): Add enum values for new options.
(options_table): Add new options.
(cmd_table): Add new options to 'dump' subcommand.
(svnadmin_opt_state): Add new fields to represent new options.
(ary_prefix_match): New. Copied from svndumpfilter.
(dump_filter_baton_t,
dump_filter_func): New.
(subcommand_dump): Initialize FILTER_BATON. Pass DUMP_FILTER_FUNC and a
pointer to FILTER_BATON to svn_repos_dump_fs() if any filtering prefixes
specified.
(sub_main): Handle new options.
* subversion/tests/cmdline/svnadmin_tests.py
(dump_exclude,
dump_exclude_copysource,
dump_include,
dump_not_include_copysource,
dump_exclude_by_pattern,
dump_include_by_pattern,
dump_exclude_all_rev_changes,
dump_invalid_filtering_option): New.
(test_list): Add new tests to table.
* subversion/tests/libsvn_repos/dump-load-test.c
(test_dump_bad_props): Update caller.
Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]

Julian Foad

2017-10-12 16:46:09 UTC

Permalink

Post by Julian Foad

This is really important. I'm going to make some time to review it.

The functionality, and even the coding style, all looks perfect.

Committed as revision 1811992.

Thank you, Sergey!

I only noticed one little error: in svnadmin_tests.py test_list one of
the pre-existing tests was duplicated as well as adding the new tests.

Good follow-ups would be:
* update the help text to say: this functionality is equivalent to
using authz exclusions, and unlike svndumpfilter, using these options
does not break copies
* update the help text for "--pattern" to match svndumpfilter (I
added an explanation of the glob syntax there in r1783741, which was
after you wrote your patch)
* update tools/client-side/bash_completion
* update the 1.10 release notes and changelog to mention this

I will do the first three of these soon.

- Julian

Post by Julian Foad

Post by Sergey Raevskiy
[[
$ svnadmin create /repo
$ svn mkdir -m "" file:///repo/A
$ svn copy -m "" file:///repo/A file:///repo/B
$ svnadmin dump /repo | svndumpfilter include /B > dump
...
Revision 0 committed as 0.
Revision 1 committed as 1.
svndumpfilter: E200003: Invalid copy source path '/A'
]]
The 'svnadmin dump' with proposed include/exclude options can handle this use
[[
$ svnadmin dump /repo --include /B > dump
* Dumped revision 0.
* Dumped revision 1.
* Dumped revision 2.
]]
The proposed options are implemented using 'svn_repos_authz_func_t' and the
not-included copy sources are hidden by the 'svn_repos' layer.
[[
$ svnadmin dump repos --include /calc > calc-dumpfile
...
$ svnadmin dump repos --include /calc --include /calendar > dumpfile
...
$ svnadmin dump repos --exclude /secret > dumpfile
...
$ svnadmin dump repos --include /calc --exclude /calendar > dumpfile
svnadmin: E205000: Try 'svnadmin help' for more info
svnadmin: E205000: '--exclude' and '--include' options cannot be used
simultaneously
$ svnadmin dump repos --include /cal* --pattern > dumpfile
...
]]
[[
Add '--include' and '--exclude' options to 'svnadmin dump'.
* include/svn_repos.h
   (svn_repos_dump_filter_func_t): New.
   (svn_repos_dump_fs4): Update function signature and comment.
   (svn_repos_dump_fs3): Update comment.
* libsvn_repos/deprecated.c
   (svn_repos_dump_fs3): Update caller.
* libsvn_repos/dump.c
   (write_revision_record): Call to svn_repos_fs_revision_proplist()
(with
    AUTHZ_FUNC) instead of svn_fs_revision_proplist2() to filter revision
    properties as well as revisions. Update comment.
   (dump_filter_baton_t,
    dump_filter_authz_func): New.
   (svn_repos_dump_fs4): Initialize and pass AUTHZ_FUNC and
AUTHZ_BATON to
    the repos layer API if FILTER_FUNC is specified by caller.
* subversion/svnadmin/svnadmin.c
   (svnadmin__cmdline_options_t): Add enum values for new options.
   (options_table): Add new options.
   (cmd_table): Add new options to 'dump' subcommand.
   (svnadmin_opt_state): Add new fields to represent new options.
   (ary_prefix_match): New. Copied from svndumpfilter.
   (dump_filter_baton_t,
    dump_filter_func): New.
   (subcommand_dump): Initialize FILTER_BATON. Pass DUMP_FILTER_FUNC
and a
    pointer to FILTER_BATON to svn_repos_dump_fs() if any filtering
prefixes
    specified.
   (sub_main): Handle new options.
* subversion/tests/cmdline/svnadmin_tests.py
   (dump_exclude,
    dump_exclude_copysource,
    dump_include,
    dump_not_include_copysource,
    dump_exclude_by_pattern,
    dump_include_by_pattern,
    dump_exclude_all_rev_changes,
    dump_invalid_filtering_option): New.
   (test_list): Add new tests to table.
* subversion/tests/libsvn_repos/dump-load-test.c
   (test_dump_bad_props): Update caller.
Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]

Julian Foad

2017-10-12 21:04:43 UTC

Permalink

* update the help text to say: this functionality is equivalent to
using authz exclusions, and unlike svndumpfilter, using these options
does not break copies
* update the help text for "--pattern" to match svndumpfilter (I
added an explanation of the glob syntax there in r1783741, which was
after you wrote your patch)
* update tools/client-side/bash_completion
* update the 1.10 release notes and changelog to mention this
I will do the first three of these soon.

r1812050 does the first three and the CHANGES file.

- Julian

maxin

2017-10-13 00:37:18 UTC

Permalink

Hi Julian,

Thanks for the prompt response! I just compiled r1812050 and tested with my
own repo, however, somehow the svndumpfilter command cannot remove the
'empty' revisions from the filtered dump file....

Maxin

--
Sent from: http://subversion.1072662.n5.nabble.com/Subversion-Dev-f4725.html

Julian Foad

2017-10-13 09:15:28 UTC

Permalink

Post by maxin
Thanks for the prompt response! I just compiled r1812050 and tested with my
own repo, however, somehow the svndumpfilter command cannot remove the
'empty' revisions from the filtered dump file....

Please report a small test case that demonstrates the problem, with the
exact commands. A script that starts with "svnadmin create repo" and
continues with "svn mkdir ..." and "svnadmin dump ..." and
"svndumpfilter ..." would be ideal.

Without knowing what commands you used, my first guess is you used
something like "svndumpfilter include / --drop-empty-revs".

Post by maxin
$ svndumpfilter include --help
[...]
--drop-empty-revs : Remove revisions emptied by filtering.
--drop-all-empty-revs : Remove all empty revisions found in dumpstream
except revision 0.

--drop-empty-revs only removes revisions that *svndumpfilter* makes
empty, not revisions that were already empty before svndumpfilter
received them. In that case maybe you want --drop-all-empty-revs
instead. Is that it?

- Julian

Julian Foad

2017-10-13 09:31:20 UTC

Permalink

Should we now add all the other options from svndumpfilter?

Post by Bert Huijben
--drop-empty-revs : Remove revisions emptied by filtering.
--drop-all-empty-revs : Remove all empty revisions found in dumpstream
except revision 0.
--renumber-revs : Renumber revisions left after filtering.
--skip-missing-merge-sources : Skip missing merge sources.
--targets ARG : Read additional prefixes, one per line, from
file ARG.
--preserve-revprops : Don't filter revision properties.
--quiet : Do not display filtering statistics.

These were all designed to be useful with filtering. Why *wouldn't* we
want to include them directly in 'svnadmin dump --include/exclude'?

- Julian

Julian Foad

2017-10-13 09:36:41 UTC

Permalink

We should update the JavaHL bindings for 'dump' accordingly.

Here is a patch that begins that process. It is not finished: as noted
in the log message, at least I haven't implemented CreateJ:FSPath and
the callback should (probably) be moved into its own file, and I have
presumably made some mistake already because even when I stub out the
unimplemented part all the tests are failing.

Would someone care to take a look at this and point out what I need to
fix or maybe even finish it, as I am running out of steam? (Brane?)

- Julian

Branko Čibej

2017-10-13 09:58:08 UTC

Permalink

Post by Julian Foad
We should update the JavaHL bindings for 'dump' accordingly.
Here is a patch that begins that process. It is not finished: as noted
in the log message, at least I haven't implemented CreateJ:FSPath and
the callback should (probably) be moved into its own file, and I have
presumably made some mistake already because even when I stub out the
unimplemented part all the tests are failing.
Would someone care to take a look at this and point out what I need to
fix or maybe even finish it, as I am running out of steam? (Brane?)

This is not going to happen any time soon. I have far too many
$dayjob-related things to do right now. Sorry ... JavaHL is an
interesting pile of spaghetti, and I can commiserate with anyone taking
the first steps towards understanding its magic.

-- Brane

Julian Foad

2018-11-30 08:44:29 UTC

Permalink

Hello Sergey!

Your patch was filed as issue #4729
https://issues.apache.org/jira/browse/SVN-4729
and released in Subversion 1.10
https://subversion.apache.org/docs/release-notes/1.10.html#dump-include-exclude

A user has reported a problem with this feature.
https://issues.apache.org/jira/browse/SVN-4760

It looks like when the source of a copied directory is excluded, it is only adding the directory itself and not adding the directory's contents. That sounds to me like a bug -- it is not the behaviour I would expect.

I see that the regression tests only test with an empty source directory.
subversion/tests/cmdline/svnadmin_tests.py : dump_exclude_copysource() etc.

Are you able to investigate this?

--
- Julian