Hello!
I've attached a patch which adds a new option (--include-parents) to 'svnadmin
dump'. This option works with existing --include option and includes parent
nodes for nodes specified in --include option.
This should fix the mentioned case.
The option doesn't work with globbing for now, since I don't see a simple
way to implement this functionality for globbing. Thus the option is
explicitly forbidden if --pattern option is specified. The option is also
forbidden to use with --exclude option because it doesn't make sense.
Log message:
[[[
Add new option (--include-parents) to 'svnadmin dump': include parent
nodes for nodes specified in --include option.
* subversion/svnadmin/svnadmin.c
(svnadmin__cmdline_options_t): Add new member SVNADMIN__INCLUDE_PARENTS.
(options_table): Add an entry for new option (--include-parents).
(cmd_table): Add new options (--include-parents) for 'dump' subcommand.
(struct svnadmin_opt_state): Add member for new option (--include-parents).
(ary_prefix_match): Add new parameter INCLUDE_PARENTS. Return TRUE if
PATH is parent of any of specified prefixes and INCLUDE_PARENTS set
to true. Update comment.
(struct dump_filter_baton_t): Add new member INCLUDE_PARENTS.
(dump_filter_func): Pass B->INCLUDE_PARENTS to ary_prefix_match().
(subcommand_dump): Initialize INCLUDE_PARENTS member of FILTER_BATON.
Rework of filtering options validation code.
(sub_main): Handle new option (--include-parents).
* subversion/tests/cmdline/svnadmin_tests.py
(dump_invalid_filtering_option): Extend test with new cases. Update docstring.
(dump_include_parents): New test.
(test_list): Add new test to table.
Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]]
On Mon, Mar 23, 2020 at 8:11 AM 钱海远(Nathan) <qianhaiy...@hikvision.com> wrote:
>
> Yes, I think so. It's just for include. It's not a very good patch, I think ,
> sorry for this .
>
> And it has a known issue , it will include the parent node path , this is a
> problem. I made another program to delete parent node.
>
>
> Best Regards!
> Haiyuan Qian
> R & D Management Group
> Hangzhou Hikvision Digital Technology Co.,Ltd
> No.555 Qianmo Road, Binjiang District, Hangzhou 310052, China
> M (86)18969199712
>
> 本邮件及其附件含有海康威视公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
> This e-mail and its attachments contain confidential information from
> HIKVISION, which is intended only for the person or entity whose address is
> listed above. Any use of the information contained herein in any way
> (including, but not limited to, total or partial disclosure, reproduction, or
> dissemination) by persons other than the intended recipient(s) is
> prohibited. If you receive this e-mail in error, please notify the sender by
> phone or email immediately and delete it!
>
>
> -----邮件原件-----
> 发件人: Daniel Shahaf <d...@daniel.shahaf.name>
> 发送时间: 2020年3月22日 1:35
> 收件人: 钱海远(Nathan) <qianhaiy...@hikvision.com>
> 抄送: us...@subversion.apache.org; dev@subversion.apache.org
> 主题: Re: Svnadmin dump with include can not dump the subdir into add when it's
> parent path was a branch
>
> [moving to dev@; please remove users@ from replies]
>
> 钱海远(Nathan) wrote on Sat, 21 Mar 2020 06:12 +0000:
> > I found the there is a BUG in subversion 1.10.6.
> >
> > Svnadmin dump with include can not dump the subdir into add when it's
> > parent path was a branch:
> > 1. 1、/A was copy from/XX , revision was 50, it the first revision of /A; 2.
> > 2、There is a subdir named /A/subdir, and several files and dir under
> > /A/subdir; 3. 3、Try to run “svnadmin dump /data/repos_root --include
> > /A/subdir >a” . The expected dump file will include /A/subdir(revision 50)
> > into add. But in fact there was nothing.
> > 4. 4、I was try to fix this bug in the svnadmin.c , a function named
> > ary_prefix_match. If the change list is the parent dir for include path, it
> > return true. Then the dump file will include /A/subdir and it's subdir or
> > subfile into add , but regrettably, the dump file will also include /A into
> > add.
>
> 钱海远(Nathan) wrote on Sat, 21 Mar 2020 06:12 +0000:
> > ---
> > C:/Users/QIANHA~1/AppData/Local/Temp/svnadmin.c-rev30397.svn001.tmp.c____
> > ____ 25 11:51:32 2020
> > +++
> > C:/Users/QIANHA~1/AppData/Local/Temp/svnadmin.c-rev30398.svn000.tmp.c____
> > ____ 21 12:58:58 2020
> > @@ -1297,3 +1297,3 @@ ary_prefix_match(const apr_array_header_t *pfxlist
> > - if (path_len < pfx_len)
> > - continue;
> > - if (strncmp(path, pfx, pfx_len) == 0
> > + /*if (path_len < pfx_len)
> > + continue;*/
> > + if ((strncmp(path, pfx, pfx_len) == 0
> > @@ -1300,0 +1301,3 @@ ary_prefix_match(const apr_array_header_t
> > *pfxlist
> > + || (strncmp(pfx,path, path_len) == 0
> > + && (path_len == 1 || path[path_len] == '\0' || path[path_len] ==
> > '/'))
> > + )
>
> Thanks for the patch. I don't see any obvious problems with this approach.
> However, the patch as it stands causes a regression:
>
> [[[
> W: /home/daniel/src/svn/t1/./subversion/libsvn_repos/load.c:667,
> W: /home/daniel/src/svn/t1/./subversion/libsvn_repos/load-fs-vtable.c:718,
> W: /home/daniel/src/svn/t1/./subversion/libsvn_repos/load-fs-vtable.c:591,
> W: /home/daniel/src/svn/t1/./subversion/libsvn_fs/fs-loader.c:1482,
> W: /home/daniel/src/svn/t1/./subversion/libsvn_fs_fs/tree.c:2524,
> W: /home/daniel/src/svn/t1/./subversion/libsvn_fs_fs/tree.c:1145:
> (apr_err=SVN_ERR_FS_NOT_FOUND)
> W: svnadmin: E160013: File not found: transaction '0-0', path '/A/B/F'
> W: CWD: /tmp/svn/subversion/tests/cmdline
> W: EXCEPTION: Failure: Command failed: "/tmp/svn/subversion/svnadmin/svnadmin
> load --quiet svn-test-work/repositories/svnadmin_tests-60-1"; exit code 1
> Traceback (most recent call last):
> File "/home/daniel/src/svn/t1/subversion/tests/cmdline/svntest/main.py",
> line 1931, in run
> rc = self.pred.run(sandbox)
> File
> "/home/daniel/src/svn/t1/subversion/tests/cmdline/svntest/testcase.py", line
> 178, in run
> result = self.func(sandbox)
> File "/home/daniel/src/svn/t1/subversion/tests/cmdline/svnadmin_tests.py",
> line 3510, in dump_exclude
> load_and_verify_dumpstream(sbox2, None, [], None, False, dump)
> File "/home/daniel/src/svn/t1/subversion/tests/cmdline/svnadmin_tests.py",
> line 304, in load_and_verify_dumpstream
> 'load', '--quiet', sbox.repo_dir, *varargs)
> File "/home/daniel/src/svn/t1/subversion/tests/cmdline/svntest/main.py",
> line 666, in run_command_stdin
> '"; exit code ' + str(exit_code))
> svntest.Failure: Command failed: "/tmp/svn/subversion/svnadmin/svnadmin load
> --quiet svn-test-work/repositories/svnadmin_tests-60-1"; exit code 1
> FAIL: svnadmin_tests.py 60: svnadmin dump with excluded paths ]]]
>
> So, I guess the new condition should be applied to --include's but not to
> --exclude's?
>
> Cheers,
>
> Daniel
>
> ________________________________
> CONFIDENTIALITY NOTICE: This electronic message is intended to be viewed only
> by the individual or entity to whom it is addressed. It may contain
> information that is privileged, confidential and exempt from disclosure under
> applicable law. Any dissemination, distribution or copying of this
> communication is strictly prohibited without our prior permission. If the
> reader of this message is not the intended recipient, or the employee or
> agent responsible for delivering the message to the intended recipient, or if
> you have received this communication in error, please notify us immediately
> by return e-mail and delete the original message and any copies of it from
> your computer system. For further information about Hikvision company. please
> see our website at www.hikvision.com<http://www.hikvision.com>
>
Index: subversion/svnadmin/svnadmin.c
===================================================================
--- subversion/svnadmin/svnadmin.c (revision 1875560)
+++ subversion/svnadmin/svnadmin.c (working copy)
@@ -158,7 +158,8 @@ enum svnadmin__cmdline_options_t
svnadmin__normalize_props,
svnadmin__exclude,
svnadmin__include,
- svnadmin__glob
+ svnadmin__glob,
+ svnadmin__include_parents
};
/* Option codes and descriptions.
@@ -297,6 +298,9 @@ static const apr_getopt_option_t options_table[] =
" Character '/' is not treated specially,
so\n"
" pattern /*/foo matches paths /a/foo and
/a/b/foo.") },
+ {"include-parents", svnadmin__include_parents, 0,
+ N_("include parent nodes for nodes specified with --include option")},
+
{NULL}
};
@@ -372,7 +376,7 @@ static const svn_opt_subcommand_desc3_t cmd_table[
"excluded, the copy is transformed into an add (unlike in
'svndumpfilter').\n"
)},
{'r', svnadmin__incremental, svnadmin__deltas, 'q', 'M', 'F',
- svnadmin__exclude, svnadmin__include, svnadmin__glob },
+ svnadmin__exclude, svnadmin__include, svnadmin__glob,
svnadmin__include_parents },
{{'F', N_("write to file ARG instead of stdout")}} },
{"dump-revprops", subcommand_dump_revprops, {0}, {N_(
@@ -662,6 +666,7 @@ struct svnadmin_opt_state
apr_array_header_t *exclude; /* --exclude */
apr_array_header_t *include; /* --include */
svn_boolean_t glob; /* --pattern */
+ svn_boolean_t include_parents; /* --include-parents */
const char *config_dir; /* Overriding Configuration Directory */
};
@@ -1361,10 +1366,13 @@ get_dump_range(svn_revnum_t *lower,
/* Compare the node-path PATH with the (const char *) prefixes in PFXLIST.
* Return TRUE if any prefix is a prefix of PATH (matching whole path
* components); FALSE otherwise.
+ * If INCLUDE_PARENTS is set to TRUE, return TRUE if PATH is parent of
+ * any of specified prefixes.
* PATH starts with a '/', as do the (const char *) paths in PREFIXES. */
-/* This function is a duplicate of svndumpfilter.c:ary_prefix_match(). */
+/* This function is an extended version of svndumpfilter.c:ary_prefix_match().
*/
static svn_boolean_t
-ary_prefix_match(const apr_array_header_t *pfxlist, const char *path)
+ary_prefix_match(const apr_array_header_t *pfxlist, const char *path,
+ svn_boolean_t include_parents)
{
int i;
size_t path_len = strlen(path);
@@ -1374,11 +1382,18 @@ static svn_boolean_t
const char *pfx = APR_ARRAY_IDX(pfxlist, i, const char *);
size_t pfx_len = strlen(pfx);
- if (path_len < pfx_len)
- continue;
- if (strncmp(path, pfx, pfx_len) == 0
- && (pfx_len == 1 || path[pfx_len] == '\0' || path[pfx_len] == '/'))
- return TRUE;
+ if (path_len < pfx_len && include_parents)
+ {
+ if (strncmp(pfx, path, path_len) == 0
+ && (path_len == 1 || pfx[path_len] == '/'))
+ return TRUE;
+ }
+ else if (path_len >= pfx_len)
+ {
+ if (strncmp(path, pfx, pfx_len) == 0
+ && (pfx_len == 1 || path[pfx_len] == '\0' || path[pfx_len] ==
'/'))
+ return TRUE;
+ }
}
return FALSE;
@@ -1390,6 +1405,7 @@ struct dump_filter_baton_t
apr_array_header_t *prefixes;
svn_boolean_t glob;
svn_boolean_t do_exclude;
+ svn_boolean_t include_parents;
};
/* Implements svn_repos_dump_filter_func_t. */
@@ -1404,7 +1420,7 @@ dump_filter_func(svn_boolean_t *include,
const svn_boolean_t matches =
(b->glob
? svn_cstring_match_glob_list(path, b->prefixes)
- : ary_prefix_match(b->prefixes, path));
+ : ary_prefix_match(b->prefixes, path, b->include_parents));
*include = b->do_exclude ? !matches : matches;
return SVN_NO_ERROR;
@@ -1445,25 +1461,42 @@ subcommand_dump(apr_getopt_t *os, void *baton, apr
if (! opt_state->quiet)
feedback_stream = recode_stream_create(stderr, pool);
+ /* Validate filtering options. */
+ if (opt_state->exclude && opt_state->include)
+ {
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("'--exclude' and '--include' options "
+ "cannot be used simultaneously"));
+ }
+
+ if (opt_state->glob && opt_state->include_parents)
+ {
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("'--pattern' and '--include-parents' options "
+ "cannot be used simultaneously"));
+ }
+
+ if (opt_state->exclude && opt_state->include_parents)
+ {
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("'--exclude' and '--include-parents' options "
+ "cannot be used simultaneously"));
+ }
+
/* Initialize the filter baton. */
filter_baton.glob = opt_state->glob;
+ filter_baton.include_parents = opt_state->include_parents;
- if (opt_state->exclude && !opt_state->include)
+ if (opt_state->exclude)
{
filter_baton.prefixes = opt_state->exclude;
filter_baton.do_exclude = TRUE;
}
- else if (opt_state->include && !opt_state->exclude)
+ else if (opt_state->include)
{
filter_baton.prefixes = opt_state->include;
filter_baton.do_exclude = FALSE;
}
- else if (opt_state->include && opt_state->exclude)
- {
- return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
- _("'--exclude' and '--include' options "
- "cannot be used simultaneously"));
- }
SVN_ERR(svn_repos_dump_fs4(repos, out_stream, lower, upper,
opt_state->incremental, opt_state->use_deltas,
@@ -3178,6 +3211,9 @@ sub_main(int *exit_code, int argc, const char *arg
case svnadmin__glob:
opt_state.glob = TRUE;
break;
+ case svnadmin__include_parents:
+ opt_state.include_parents = TRUE;
+ break;
default:
{
SVN_ERR(subcommand_help(NULL, NULL, pool));
Index: subversion/tests/cmdline/svnadmin_tests.py
===================================================================
--- subversion/tests/cmdline/svnadmin_tests.py (revision 1875560)
+++ subversion/tests/cmdline/svnadmin_tests.py (working copy)
@@ -3808,7 +3808,7 @@ def dump_exclude_all_rev_changes(sbox):
'log', '-v', sbox2.repo_url)
def dump_invalid_filtering_option(sbox):
- "dump with --include and --exclude simultaneously"
+ "dump with invalid filtering options"
sbox.build(create_wc=False, empty=False)
@@ -3822,6 +3822,27 @@ def dump_invalid_filtering_option(sbox):
'--include', '/A/B/E',
sbox.repo_dir)
+ # Attempt to dump repository with '--include-parents' and '--exclude' options
+ # specified simultaneously.
+ expected_error = ".*: '--exclude' and '--include-parents' options cannot be
used " \
+ "simultaneously"
+ svntest.actions.run_and_verify_svnadmin(None, expected_error,
+ 'dump', '-q',
+ '--exclude', '/A/D/H',
+ '--include-parents',
+ sbox.repo_dir)
+
+ # Attempt to dump repository with '--include-parents' and '--pattern' options
+ # specified simultaneously.
+ expected_error = ".*: '--pattern' and '--include-parents' options cannot be
used " \
+ "simultaneously"
+ svntest.actions.run_and_verify_svnadmin(None, expected_error,
+ 'dump', '-q',
+ '--include', '/A/D/H',
+ '--pattern',
+ '--include-parents',
+ sbox.repo_dir)
+
@Issue(4725)
def load_issue4725(sbox):
"""load that triggers issue 4725"""
@@ -4036,7 +4057,62 @@ PROPS-END
if output != ['\n', '\n']:
raise svntest.Failure("Unexpected property value %s" % output)
+def dump_include_parents(sbox):
+ "svnadmin dump --include with parents"
+ sbox.build(create_wc=False, empty=True)
+
+ # Create a couple of directories.
+ svntest.actions.run_and_verify_svn(svntest.verify.AnyOutput, [], "mkdir",
+ sbox.repo_url + '/project/trunk',
+ sbox.repo_url + '/project/branches',
+ sbox.repo_url + '/project/tags',
+ "-m", "Create repository structure.",
+ "--parents")
+
+ # Make some changes in trunk.
+ svntest.actions.run_and_verify_svn(svntest.verify.AnyOutput, [], "mkdir",
+ sbox.repo_url + '/project/trunk/dir',
+ "-m", "Add directory.")
+
+ # Create branch 'b1'.
+ svntest.actions.run_and_verify_svn(svntest.verify.AnyOutput, [], "copy",
+ sbox.repo_url + '/project/trunk',
+ sbox.repo_url + '/project/branches/b1',
+ "-m", "Create branch.")
+
+ # Dump with only 'b1' included.
+ _, dump, _ = svntest.actions.run_and_verify_svnadmin(None, [],
+ 'dump', '-q',
+ '--include',
'/project/branches/b1',
+ '--include-parents',
+ sbox.repo_dir)
+
+ # Load repository from dump.
+ sbox2 = sbox.clone_dependent()
+ sbox2.build(create_wc=False, empty=True)
+ load_and_verify_dumpstream(sbox2, None, [], None, False, dump)
+
+ # Check log.
+ expected_output = svntest.verify.RegexListOutput([
+ '-+\\n',
+ 'r3\ .*\n',
+ re.escape('Changed paths:'),
+ re.escape(' A /project/branches/b1'),
+ re.escape(' A /project/branches/b1/dir'),
+ '-+\\n',
+ 'r2\ .*\n',
+ '-+\\n',
+ 'r1\ .*\n',
+ re.escape('Changed paths:'),
+ re.escape(' A /project'),
+ re.escape(' A /project/branches'),
+ '-+\\n'
+ ])
+ svntest.actions.run_and_verify_svn(expected_output, [],
+ 'log', '-v', '-q', sbox2.repo_url)
+
+
########################################################################
# Run the tests
@@ -4116,6 +4192,7 @@ test_list = [ None,
recover_prunes_rep_cache_when_disabled,
dump_include_copied_directory,
load_normalize_node_props,
+ dump_include_parents
]
if __name__ == '__main__':
