You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "钱海远 (Nathan)" <qi...@hikvision.com> on 2020/03/23 05:11:12 UTC

答复: Svnadmin dump with include can not dump the subdir into add when it's parent path was a branch

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) <qi...@hikvision.com>
抄送: users@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>


Re: Svnadmin dump with include can not dump the subdir into add when it's parent path was a branch

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Sergey Raevskiy wrote on Tue, 24 Mar 2020 02:25 +0300:
> 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.

Thanks for the patch.

> @@ -4036,7 +4057,62 @@ PROPS-END
> +def dump_include_parents(sbox):
⋮
> +  # 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'
> +  ])

This uses «\n» as a regex escape, «\ » as a string escape, and «\n» as
a string escape; and then uses re.escape() on constant strings that
it's a no-op on.  There are three problems with this:

1. It's confusing.

2. Backslash-space is deprecated and will become a SyntaxError in the
future (see https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals).

3. We have a way to verify the log output while abstracting away
details of its serialization by the cmdline client: run_and_verify_log_xml().
(That function's docstring says "expected_paths checking is not yet
implemented", but that seems to be factually incorrect.)

Furthermore, the test creates a TTB structure and then uses a branch
root as the target to dump.  Is it significant that the target is
a branch root?

I'm curious why you didn't use the Greek tree.  Using it would have
been idiomatic.

I've not looked closely at the C changes.

Cheers,

Daniel

P.S. Looking into RegexListOutput I noticed something: don't we need the following? —
.
    Index: subversion/tests/cmdline/svntest/verify.py
    ===================================================================
    --- subversion/tests/cmdline/svntest/verify.py	(revision 1875498)
    +++ subversion/tests/cmdline/svntest/verify.py	(working copy)
    @@ -190,7 +190,7 @@ def re_fullmatch(pattern, string, flags=0):
       if pattern.endswith('$'):
         return re.match(pattern, string, flags)
     
    -  return re.match(pattern + '$', string, flags)
    +  return re.match('(?:' + pattern + ')' + '$', string, flags)
     
     def regex_fullmatch(rx, string):
       """If the whole STRING matches the compiled regular expression RX,
.
— to correctly handle re_fullmatch('foo|bar')?

Re: Svnadmin dump with include can not dump the subdir into add when it's parent path was a branch

Posted by Sergey Raevskiy <Se...@visualsvn.com>.
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) <qi...@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) <qi...@hikvision.com>
> 抄送: users@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>
>