You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Pavel Lyalyakin via dev <de...@subversion.apache.org> on 2023/03/07 19:23:28 UTC
"copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Hello,
I seem to have found a new bug (I can't find similar reports) so I thought
I'd report it to this mailing list. Sorry if it's a known issue.
I believe that the command shouldn't work anyway, right? But it shouldn't
assert either.
The SVN client crashes when I run the following command:
[[[
svn move https://svn1.mydomain.com/svn/MyRepo/
https://svn1.mydomain.com/svn/MyRepo/trunk -m "Test Commit"
]]]
[[[
svn: E235000: In file '..\..\..\subversion\libsvn_client\copy.c' line 1187:
assertion failed (! svn_path_is_empty(path))
]]]
SRC URL points to a root of a repository. DST URL points to a non-existent
subdirectory.
svn, version 1.14.2 (r1899510)
compiled Feb 21 2023, 03:04:00 on x86_64-microsoft-windows6.2.9200
Thank you.
--
With best regards,
Pavel Lyalyakin
VisualSVN Team
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den tis 7 mars 2023 kl 20:24 skrev Pavel Lyalyakin via dev <
dev@subversion.apache.org>:
> Hello,
>
> I seem to have found a new bug (I can't find similar reports) so I thought
> I'd report it to this mailing list. Sorry if it's a known issue.
>
> I believe that the command shouldn't work anyway, right? But it shouldn't
> assert either.
>
> The SVN client crashes when I run the following command:
> [[[
> svn move https://svn1.mydomain.com/svn/MyRepo/
> https://svn1.mydomain.com/svn/MyRepo/trunk -m "Test Commit"
> ]]]
> [[[
> svn: E235000: In file '..\..\..\subversion\libsvn_client\copy.c' line
> 1187: assertion failed (! svn_path_is_empty(path))
> ]]]
>
> SRC URL points to a root of a repository. DST URL points to a non-existent
> subdirectory.
>
> svn, version 1.14.2 (r1899510)
> compiled Feb 21 2023, 03:04:00 on x86_64-microsoft-windows6.2.9200
>
Thanks Pavel for reporting!
I get the same on Linux (Ubuntu 22.10 on WSL), with both 1.14.2 and a newly
compiled trunk version. In my case I'm using file:// as the access method:
[[[
dsg@daniel-2022:~/svn_trunk/subversion$ ./svnadmin/svnadmin create
$HOME/repo
dsg@daniel-2022:~/svn_trunk/subversion$ ./svn/svn move file://$HOME/repo
file://$HOME/repo/foo -m 'test move'
subversion/libsvn_client/copy.c:1187: (apr_err=SVN_ERR_ASSERTION_FAIL)
svn: E235000: In file 'subversion/libsvn_client/copy.c' line 1187:
assertion failed (! svn_path_is_empty(path))
Aborted
dsg@daniel-2022:~/svn_trunk/subversion$ ./svn/svn --version
svn, version 1.15.0-dev (under development)
compiled Mar 8 2023, 21:45:52 on x86_64-pc-linux-gnu
[...]
dsg@daniel-2022:~/svn_trunk/subversion$
]]]
I made a quick check and there seems to be code in
subversion/libsvn_client/copy.c to detect if a move is into it's own child,
lines 3080ff, however only
[[[
if (!srcs_are_urls && !dst_is_url)
]]
This condition seems to be added in r846422 (2003-06-25 23:46:20):
[[[
Fix issue 1367, allow parent-into-child copies provided they are not
WC-to-WC.
* subversion/libsvn_client/copy.c
[...]
(setup_copy): Abort parent-into-child copies only if they are WC-to-WC.
]]]
If I remove the condition (!srcs_are_urls && !dst_is_url) the code will
properly detect that there is a move from a parent to its child and abort:
[[[
dsg@daniel-2022:~/svn_trunk/subversion$ ./svn/svn move file://$HOME/repo
file://$HOME/repo/foo -m 'test move'
srcareurls: 1 dstisurl: 1
subversion/svn/move-cmd.c:100,
subversion/svn/util.c:557,
subversion/libsvn_client/copy.c:3495,
subversion/libsvn_client/copy.c:3099: (apr_err=SVN_ERR_UNSUPPORTED_FEATURE)
svn: E200007: Cannot copy path 'file:///home/dsg/repo' into its own child
'file:///home/dsg/repo/foo'
]]]
The code has been refactored a number of times (including a merge from a
feature branch to trunk) so I didn't trace all changes.
I'm running out of time right now but the issue above should probably
explain more about why this condition was added. I also did NOT run the
test suite so it may be that I'm breaking things high and low with that
change. Obviously it isn't committed until I understand the history of the
code.
Kind regards,
Daniel
Re: Invalid escape sequences in Python scripts (was Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `)
Posted by Daniel Sahlberg <ds...@apache.org>.
On 2023/07/20 04:32:43 Yasuhito FUTATSUKI wrote:
> Jun, could you please commit the patch?
I have committed this patch now, r1912632, crediting Jun.
Thanks Jun for the patch and thanks Yasuhito for review.
Kind regards,
Daniel Sahlberg
Invalid escape sequences in Python scripts (was Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `)
Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
Hello,
On 2023/07/19 6:16, Daniel Sahlberg wrote:
> Den tis 18 juli 2023 kl 21:23 skrev Yasuhito FUTATSUKI <
> futatuki@yf.bsdclub.org>:
> I'm not fluent in Python to the level of these changes (although as far as
> I've been able to test they are good). Could one of you commit?
I looked Jun's patch in <54...@gmail.com>
and it looks good as it uses b'...' with valid escape sequence for bytes.
I also checked by using flake8 that after aplying the patch, there is
no invalid sequence warning in the scripts in subversion/ and build/
directories.
[[[
$ find . \( -name .svn -or -name __pycache__ \) -prune -or -name '*.py' -print0 | xargs -0 flake8 --select W605 | sed -e 's/:[0-9][0-9]*: W605 .*/:/' | uniq
./tools/dist/security/parser.py:58: SyntaxWarning: 'tuple' object is not callable; perhaps you missed a comma?
(CULPRIT_SERVER,)
./contrib/client-side/svn-merge-vendor.py:429:
./contrib/client-side/svn_apply_autoprops.py:66:
./contrib/client-side/svn_apply_autoprops.py:67:
./tools/dev/analyze-svnlogs.py:38:
./tools/dev/analyze-svnlogs.py:39:
./tools/dev/analyze-svnlogs.py:40:
./tools/dev/check-license.py:26:
./tools/dev/check-license.py:27:
./tools/dev/check-license.py:28:
./tools/dev/check-license.py:29:
./tools/dev/check-license.py:30:
./tools/dev/check-license.py:31:
./tools/dev/check-license.py:32:
./tools/dev/check-license.py:33:
./tools/dev/check-license.py:34:
./tools/dev/check-license.py:35:
./tools/dev/check-license.py:36:
./tools/dev/check-license.py:37:
./tools/dev/check-license.py:38:
./tools/dev/check-license.py:39:
./tools/dev/check-license.py:40:
./tools/dev/check-license.py:41:
./tools/dev/check-license.py:42:
./tools/dev/check-license.py:43:
./tools/dev/contribulyze.py:520:
./tools/dev/contribulyze.py:523:
./tools/dev/contribulyze.py:528:
./tools/dev/contribulyze.py:579:
./tools/dev/contribulyze.py:706:
./tools/dev/mklog.py:32:
./tools/dev/po-merge.py:25:
./tools/dev/trails.py:35:
./tools/dev/trails.py:36:
./tools/po/l10n-report.py:182:
./tools/server-side/svn-backup-dumps.py:455:
]]]
I apologize if I didn't post an extra comment about it, it woluld be
commited soon.
Jun, could you please commit the patch?
Thanks,
--
Yasuhito FUTATSUKI <fu...@poem.co.jp>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den tis 18 juli 2023 kl 21:23 skrev Yasuhito FUTATSUKI <
futatuki@yf.bsdclub.org>:
> Hi,
>
> On 2023/06/25 19:22, Daniel Sahlberg wrote:
> > Den ons 31 maj 2023 kl 05:29 skrev Yasuhito FUTATSUKI <
> > futatuki@yf.bsdclub.org>:
> >
> >> Hello,
> >>
> >> On 2023/05/30 17:04, Daniel Sahlberg wrote:
> >> > Den tis 30 maj 2023 kl 04:46 skrev Jun Omae <ju...@gmail.com>:
> >>
> >
> > [...]
> >
> >
> >>>> However, in attached patch, correct the escape sequences because "make
> >> >> check" requires at least Python 3.0.
> >>
> >> In the discussion in this list at 2019, there is no objection that
> >> we don't support Python 3.x where x < 5, both in trunk and 1.14.x.
> >> And since r1899944, we drop Python 2.7 support for make check, in trunk.
> >>
> >> So I think it can be accectable, at least trunk.
> >>
> >> However we've not drop Python 2.7 support agressively, using 'br' prefix
> >> instead of 'rb' is prefered if we want back porting this to 1.14.x.
> >> Because 'br' prefix can be also used in Python 2.7.
> >>
> >>
> > I don't think this change was ever committed. Can we come to a conclusion
> > on how to do it and get it committed?
>
> I'm very sorry for replying too much late, but I don't think my post has
> been blocking to commit any of them. Any of three looks good to me.
> Let's go ahead.
>
> > In my opinion this change doesn't need to be backported to 1.14 so we can
> > go with a Python 3-only solution. My reasoning is that it "only" fixes a
> > SyntaxWarning so it isn't a very significant issue. By the time Python
> > changes to a SyntaxError, I hope 1.14 is EOL anyway.
>
> Even if it won't be backported to 1.14.x, there is no reason to avoid using
> br'...', but also there is no strong reason to use it.
>
> A small reasons are that we've already used br'...' in
> subversion/tests/cmdline/svntest/tree.py, and I believe that using raw
> prefix
> in regular expression string rather than using backslash escape is good for
> readability.
I'm not fluent in Python to the level of these changes (although as far as
I've been able to test they are good). Could one of you commit?
Kind regards,
Daniel
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
Hi,
On 2023/06/25 19:22, Daniel Sahlberg wrote:
> Den ons 31 maj 2023 kl 05:29 skrev Yasuhito FUTATSUKI <
> futatuki@yf.bsdclub.org>:
>
>> Hello,
>>
>> On 2023/05/30 17:04, Daniel Sahlberg wrote:
>> > Den tis 30 maj 2023 kl 04:46 skrev Jun Omae <ju...@gmail.com>:
>>
>
> [...]
>
>
>>>> However, in attached patch, correct the escape sequences because "make
>> >> check" requires at least Python 3.0.
>>
>> In the discussion in this list at 2019, there is no objection that
>> we don't support Python 3.x where x < 5, both in trunk and 1.14.x.
>> And since r1899944, we drop Python 2.7 support for make check, in trunk.
>>
>> So I think it can be accectable, at least trunk.
>>
>> However we've not drop Python 2.7 support agressively, using 'br' prefix
>> instead of 'rb' is prefered if we want back porting this to 1.14.x.
>> Because 'br' prefix can be also used in Python 2.7.
>>
>>
> I don't think this change was ever committed. Can we come to a conclusion
> on how to do it and get it committed?
I'm very sorry for replying too much late, but I don't think my post has
been blocking to commit any of them. Any of three looks good to me.
Let's go ahead.
> In my opinion this change doesn't need to be backported to 1.14 so we can
> go with a Python 3-only solution. My reasoning is that it "only" fixes a
> SyntaxWarning so it isn't a very significant issue. By the time Python
> changes to a SyntaxError, I hope 1.14 is EOL anyway.
Even if it won't be backported to 1.14.x, there is no reason to avoid using
br'...', but also there is no strong reason to use it.
A small reasons are that we've already used br'...' in
subversion/tests/cmdline/svntest/tree.py, and I believe that using raw prefix
in regular expression string rather than using backslash escape is good for
readability.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den ons 31 maj 2023 kl 05:29 skrev Yasuhito FUTATSUKI <
futatuki@yf.bsdclub.org>:
> Hello,
>
> On 2023/05/30 17:04, Daniel Sahlberg wrote:
> > Den tis 30 maj 2023 kl 04:46 skrev Jun Omae <ju...@gmail.com>:
>
[...]
> >> However, in attached patch, correct the escape sequences because "make
> >> check" requires at least Python 3.0.
>
> In the discussion in this list at 2019, there is no objection that
> we don't support Python 3.x where x < 5, both in trunk and 1.14.x.
> And since r1899944, we drop Python 2.7 support for make check, in trunk.
>
> So I think it can be accectable, at least trunk.
>
> However we've not drop Python 2.7 support agressively, using 'br' prefix
> instead of 'rb' is prefered if we want back porting this to 1.14.x.
> Because 'br' prefix can be also used in Python 2.7.
>
>
I don't think this change was ever committed. Can we come to a conclusion
on how to do it and get it committed?
In my opinion this change doesn't need to be backported to 1.14 so we can
go with a Python 3-only solution. My reasoning is that it "only" fixes a
SyntaxWarning so it isn't a very significant issue. By the time Python
changes to a SyntaxError, I hope 1.14 is EOL anyway.
Kind regards,
Daniel Sahlberg
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
Hello,
On 2023/05/30 17:04, Daniel Sahlberg wrote:
> Den tis 30 maj 2023 kl 04:46 skrev Jun Omae <ju...@gmail.com>:
>>> If I understand this correctly, the string should be declared as raw
>> instead of binary:
>>> [[[
>>> Index: subversion/tests/cmdline/copy_tests.py
>>> ===================================================================
>>> --- subversion/tests/cmdline/copy_tests.py (revision 1910111)
>>> +++ subversion/tests/cmdline/copy_tests.py (working copy)
>>> @@ -1421,7 +1421,7 @@
>>> if re.match(b'[^\\r]\\n', raw_contents):
>>> raise svntest.Failure
>>>
>>> - if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$',
>> line_contents[3]):
>>> + if not re.match(r'.*\$LastChangedRevision:\s*\d+\s*\$',
>> line_contents[3]):
>>> raise svntest.Failure
>>>
>>> #-------------------------------------------------------------
>>> ]]]
>>>
>>> Maybe the previous re.match (first context line in the diff) should be
>> changed as well?
>>
>> The literal for the regular expression should be a bytes instance because
>> line_contents[3] is a bytes instance.
>> If Python is 3.3 or later, we could use a raw bytes literal.
>>
>> [[[
>> - if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$',
>> line_contents[3]):
>> + if not re.match(rb'.*\$LastChangedRevision:\s*\d+\s*\$',
>> line_contents[3]):
>> ]]]
>>
>> However, in attached patch, correct the escape sequences because "make
>> check" requires at least Python 3.0.
In the discussion in this list at 2019, there is no objection that
we don't support Python 3.x where x < 5, both in trunk and 1.14.x.
And since r1899944, we drop Python 2.7 support for make check, in trunk.
So I think it can be accectable, at least trunk.
However we've not drop Python 2.7 support agressively, using 'br' prefix
instead of 'rb' is prefered if we want back porting this to 1.14.x.
Because 'br' prefix can be also used in Python 2.7.
>> [[[
>> - if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$',
>> line_contents[3]):
>> + if not re.match(b'.*\\$LastChangedRevision:\\s*\\d+\\s*\\$',
>> line_contents[3]):
>> ]]]
>>
>> The patch fixes many "invalid escape sequences" which exist in also other
>> Python scripts (verified unit tests with Python 3.5 and 3.12 on Ubuntu, and
>> Python 3.12 on Windows).
>
> I checked the changes and it looks good as far as I understand. I've also
> run the testsuite (make check, make davautocheck, make svnserveautocheck)
> with Python 3.11.2 on Ubuntu (WSL), all seems fine.
Of course, this form of literals is less incompatible between Python versions.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den tis 30 maj 2023 kl 04:46 skrev Jun Omae <ju...@gmail.com>:
> I just pushed the changes in r1910129.
>
Excellent, thanks!
> > > (Sidenote: There is an error message in the test log about line
> 1424. This was added way back in r846892 and has essentially been unchanged
> ever since. I believe this is unrelated and I will send this in a separate
> thread when I've had some more time to look at it).
> >
> > The `SyntaxWarning` from line 1424 is displayed since Python 3.12,
> even without -Wdefault option.
> >
> > $ python3.12 -c '"\$"'
> > <string>:1: SyntaxWarning: invalid escape sequence '\$'
> > $ python3.11 -c '"\$"' # no warnings
> > $ python3.11 -Wdefault -c '"\$"'
> > <string>:1: DeprecationWarning: invalid escape sequence '\$'
> >
> > See also:
> https://docs.python.org/3.12/whatsnew/3.12.html?highlight=backslash-character#other-language-changes
> <
> https://docs.python.org/3.12/whatsnew/3.12.html?highlight=backslash-character#other-language-changes
> >
> >
> >
> > If I understand this correctly, the string should be declared as raw
> instead of binary:
> > [[[
> > Index: subversion/tests/cmdline/copy_tests.py
> > ===================================================================
> > --- subversion/tests/cmdline/copy_tests.py (revision 1910111)
> > +++ subversion/tests/cmdline/copy_tests.py (working copy)
> > @@ -1421,7 +1421,7 @@
> > if re.match(b'[^\\r]\\n', raw_contents):
> > raise svntest.Failure
> >
> > - if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$',
> line_contents[3]):
> > + if not re.match(r'.*\$LastChangedRevision:\s*\d+\s*\$',
> line_contents[3]):
> > raise svntest.Failure
> >
> > #-------------------------------------------------------------
> > ]]]
> >
> > Maybe the previous re.match (first context line in the diff) should be
> changed as well?
>
> The literal for the regular expression should be a bytes instance because
> line_contents[3] is a bytes instance.
> If Python is 3.3 or later, we could use a raw bytes literal.
>
> [[[
> - if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$',
> line_contents[3]):
> + if not re.match(rb'.*\$LastChangedRevision:\s*\d+\s*\$',
> line_contents[3]):
> ]]]
>
> However, in attached patch, correct the escape sequences because "make
> check" requires at least Python 3.0.
>
> [[[
> - if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$',
> line_contents[3]):
> + if not re.match(b'.*\\$LastChangedRevision:\\s*\\d+\\s*\\$',
> line_contents[3]):
> ]]]
>
> The patch fixes many "invalid escape sequences" which exist in also other
> Python scripts (verified unit tests with Python 3.5 and 3.12 on Ubuntu, and
> Python 3.12 on Windows).
>
I checked the changes and it looks good as far as I understand. I've also
run the testsuite (make check, make davautocheck, make svnserveautocheck)
with Python 3.11.2 on Ubuntu (WSL), all seems fine.
Kind regards,
Daniel Sahlberg
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den mån 9 okt. 2023 kl 07:05 skrev Nathan Hartman <hartman.nathan@gmail.com
>:
> On Sun, Oct 8, 2023 at 3:14 PM Daniel Sahlberg
> <da...@gmail.com> wrote:
> > I was able to reproduce the issue and thanks to Yasuhito's hints I was
> also able to fix it by using absolute paths when running the svn command.
> I've committed r1912826 with a simple fix which seems to work for me.
> >
> > Nathan, can you verify if it also works for you?
>
> Thanks Daniel and Yasuhito for your help!
>
> With r1912826, the test passes now.
>
Great, thanks for confirmation!
>
> I had trouble determining what's different about this test as compared
> to others with expected errors containing wc paths. As I described
> previously, I experimented with different ways to construct the
> expected_error, but I hadn't considered to call the svn command itself
> with absolute paths!!
>
> Just to tie up all the loose ends, replying to some earlier questions:
>
> On Fri, Oct 6, 2023 at 3:33 PM Daniel Sahlberg
> <da...@gmail.com> wrote:
> > From the test:
> >
> > [[[
> > was_cwd = os.getcwd()
> > from_path = os.path.abspath(sbox.ospath(''))
> > to_path = os.path.abspath(sbox.ospath('F/B'))
> > os.chdir(wc_dir)
> > ]]]
> >
> > Would be interesting to know the values of was_cwd, return value of
> sbox.ospath("") and how that translates with os.path.abspath() and the same
> for "F/B".
>
> Even though this is a moot point now, before you wrote your reply I
> instrumented the test and printed these out, so for completeness:
>
> was_cwd = /home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline
>
> from_path:
> sbox.ospath('') = svn-test-work/working_copies/copy_tests-17
> os.path.abspath(sbox.ospath('')) =
>
> /home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests-17
>
> to_path:
> sbox.ospath('F/B') = svn-test-work/working_copies/copy_tests-17/F/B
> os.path.abspath(sbox.ospath('F/B')) =
>
> /home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests-17/F/B
>
> Expected:
> svn: E200007: Cannot move path
>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
> into its own child
>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
>
> Actual:
> svn: E200007: Cannot move path
>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
> into its own child
>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
>
> >> I am assuming that in the expected path, 'svn-test-work' is probably a
> >> symlink to ~/ramdrive/svndev/ramdisk. I can't check this at the moment
> >> but I will try rerunning later and I'll inspect these locations.
>
> Yes, svn-test-work (subversion/tests/cmdline/svn-test-work) is indeed
> a symlink in my case.
>
I was able to get the test cases to fail with the exact same setup. It
seems the \- in the "expected" was a red herring, it obviously doesn't
affect the Expected/Actual check so I guess it is added in the printout
somehow.
Starting the test cases directly and adding the --development option was
very helpful in debugging:
[[[
cd subversion/tests/cmdline/
./copy_tests.py 17 --development
]]]
>
> > The symlink being an artefact from your custom build system? Sounds like
> you might be on to something.
>
> The tools/dev/unix-build/Makefile.svn (svn-check-prepare-ramdisk
> target) constructs this if you specify RAMDISK in the call to 'make',
> e.g.,:
>
> $ make svn-check RAMDISK=$SVN_DEV/ramdisk
>
Are the custom targets in Makefile.svn something that should be added to
the repository? I was blown away by the performance running the tests on a
ramdisk, would be really useful to have, but I had to manipulate each test
to replace the work folders with symlinks before running make check.
Kind regards,
Daniel
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Nathan Hartman <ha...@gmail.com>.
On Sun, Oct 8, 2023 at 3:14 PM Daniel Sahlberg
<da...@gmail.com> wrote:
> I was able to reproduce the issue and thanks to Yasuhito's hints I was also able to fix it by using absolute paths when running the svn command. I've committed r1912826 with a simple fix which seems to work for me.
>
> Nathan, can you verify if it also works for you?
Thanks Daniel and Yasuhito for your help!
With r1912826, the test passes now.
I had trouble determining what's different about this test as compared
to others with expected errors containing wc paths. As I described
previously, I experimented with different ways to construct the
expected_error, but I hadn't considered to call the svn command itself
with absolute paths!!
Just to tie up all the loose ends, replying to some earlier questions:
On Fri, Oct 6, 2023 at 3:33 PM Daniel Sahlberg
<da...@gmail.com> wrote:
> From the test:
>
> [[[
> was_cwd = os.getcwd()
> from_path = os.path.abspath(sbox.ospath(''))
> to_path = os.path.abspath(sbox.ospath('F/B'))
> os.chdir(wc_dir)
> ]]]
>
> Would be interesting to know the values of was_cwd, return value of sbox.ospath("") and how that translates with os.path.abspath() and the same for "F/B".
Even though this is a moot point now, before you wrote your reply I
instrumented the test and printed these out, so for completeness:
was_cwd = /home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline
from_path:
sbox.ospath('') = svn-test-work/working_copies/copy_tests-17
os.path.abspath(sbox.ospath('')) =
/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests-17
to_path:
sbox.ospath('F/B') = svn-test-work/working_copies/copy_tests-17/F/B
os.path.abspath(sbox.ospath('F/B')) =
/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests-17/F/B
Expected:
svn: E200007: Cannot move path
'/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
into its own child
'/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
Actual:
svn: E200007: Cannot move path
'/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
into its own child
'/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
>> I am assuming that in the expected path, 'svn-test-work' is probably a
>> symlink to ~/ramdrive/svndev/ramdisk. I can't check this at the moment
>> but I will try rerunning later and I'll inspect these locations.
Yes, svn-test-work (subversion/tests/cmdline/svn-test-work) is indeed
a symlink in my case.
> The symlink being an artefact from your custom build system? Sounds like you might be on to something.
The tools/dev/unix-build/Makefile.svn (svn-check-prepare-ramdisk
target) constructs this if you specify RAMDISK in the call to 'make',
e.g.,:
$ make svn-check RAMDISK=$SVN_DEV/ramdisk
Cheers,
Nathan
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
On 2023/10/09 21:14, Yasuhito FUTATSUKI wrote:
> And even if the
> test pattern is wrong for the purpose, I think this pit fall should
> be tested here, or elsewhere (even if it is a bug in test case itself).
This was somewhat pointless, because it can be pit fall only when
we make expected output pattern and it is not the context how command
line tools are used. Sorry.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
On 2023/10/09 18:05, Daniel Sahlberg wrote:
> Den mån 9 okt. 2023 kl 10:30 skrev Yasuhito FUTATSUKI <
> futatuki@yf.bsdclub.org>:
>> On 2023/10/09 4:14, Daniel Sahlberg wrote:
>>> Den lör 7 okt. 2023 kl 06:52 skrev Yasuhito FUTATSUKI <
>>> futatuki@yf.bsdclub.org>:
>>
>>>> If svn is called with relative paths, it can only see
>>>> cwd as a realpath, and absolute paths of argment paths
>>>> are composed from it. So the paths in error message we
>>>> can expect here is absolute real paths or relative paths
>>>> only (It assumes that relative paths passed from the
>>>> command line don't contain symlinks).
>>
>>> I was able to reproduce the issue and thanks to Yasuhito's hints I was
>> also
>>> able to fix it by using absolute paths when running the svn command. I've
>>> committed r1912826 with a simple fix which seems to work for me.
>>
>> I don't think r1912826 is a correct fix. For the main purpose of
>> the test, checking SVN-4913 is surely fixed, it is correct,
>> but then how the error message should be when the arguments are
>> passed in the form of relative paths ?
>>
>
> Just so I understand your question, are you asking what the ACTUAL error
> message should be when passed a relative path (which may contain a symlink)?
Yes. As far as I read "Error message conventions"[1] and description
for canonical form[2], I cannot see explicitly what it should be.
[1] Error message conventions, Coding andCommit Conventions
https://subversion.apache.org/docs/community-guide/conventions.html#error-messages
[2] Detailed Description, svn_dirent_uri.h File Reference
https://subversion.apache.org/docs/api/latest/svn__dirent__uri_8h.html#details
> If this is the question, then it goes deep within the library where the
> relative path is resolved to an absolute path. The error message has always
> returned the absolute path (libsvn_client/copy.c, approx 3100). Maybe it
> would have been better to return a path relative to the wc root?
Generally it depends on the system design, not the implementation how
do we treat the paths passed in argments, in for each error messages.
But I think that using the absolute path is correct in this case,
at least within libsvn_client/copy.c: try_copy().
Anyways, I don't think it is right that changing how it checked if
the test pattern is not wrong for the purpose. And even if the
test pattern is wrong for the purpose, I think this pit fall should
be tested here, or elsewhere (even if it is a bug in test case itself).
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Nathan Hartman <ha...@gmail.com>.
On Mon, Oct 9, 2023 at 5:05 AM Daniel Sahlberg <da...@gmail.com>
wrote:
> Den mån 9 okt. 2023 kl 10:30 skrev Yasuhito FUTATSUKI <
> futatuki@yf.bsdclub.org>:
>
>>
>>
>> On 2023/10/09 4:14, Daniel Sahlberg wrote:
>> > Den lör 7 okt. 2023 kl 06:52 skrev Yasuhito FUTATSUKI <
>> > futatuki@yf.bsdclub.org>:
>>
>> >> If svn is called with relative paths, it can only see
>> >> cwd as a realpath, and absolute paths of argment paths
>> >> are composed from it. So the paths in error message we
>> >> can expect here is absolute real paths or relative paths
>> >> only (It assumes that relative paths passed from the
>> >> command line don't contain symlinks).
>>
>> > I was able to reproduce the issue and thanks to Yasuhito's hints I was
>> also
>> > able to fix it by using absolute paths when running the svn command.
>> I've
>> > committed r1912826 with a simple fix which seems to work for me.
>>
>> I don't think r1912826 is a correct fix. For the main purpose of
>> the test, checking SVN-4913 is surely fixed, it is correct,
>> but then how the error message should be when the arguments are
>> passed in the form of relative paths ?
>>
>
> Just so I understand your question, are you asking what the ACTUAL error
> message should be when passed a relative path (which may contain a symlink)?
>
> If this is the question, then it goes deep within the library where the
> relative path is resolved to an absolute path. The error message has always
> returned the absolute path (libsvn_client/copy.c, approx 3100). Maybe it
> would have been better to return a path relative to the wc root?
>
> Kind regards,
> Daniel
>
When I was studying other tests, some of them used a regex to match
anything in the first portion of the string. So if the path is $WC/A/B, the
expected_error contains something like '.*/A/B'. I think this is one of the
things I experimented with in my earlier attempts, but I may have
overlooked something because the test was still failing for me.
I'm away from my computer at the moment but I can follow-up with a few
examples elsewhere in the test suite. (You can also find them pretty easily
by grepping for expected_error and looking for messages that contain wc
paths.)
Cheers,
Nathan
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den mån 9 okt. 2023 kl 10:30 skrev Yasuhito FUTATSUKI <
futatuki@yf.bsdclub.org>:
>
>
> On 2023/10/09 4:14, Daniel Sahlberg wrote:
> > Den lör 7 okt. 2023 kl 06:52 skrev Yasuhito FUTATSUKI <
> > futatuki@yf.bsdclub.org>:
>
> >> If svn is called with relative paths, it can only see
> >> cwd as a realpath, and absolute paths of argment paths
> >> are composed from it. So the paths in error message we
> >> can expect here is absolute real paths or relative paths
> >> only (It assumes that relative paths passed from the
> >> command line don't contain symlinks).
>
> > I was able to reproduce the issue and thanks to Yasuhito's hints I was
> also
> > able to fix it by using absolute paths when running the svn command. I've
> > committed r1912826 with a simple fix which seems to work for me.
>
> I don't think r1912826 is a correct fix. For the main purpose of
> the test, checking SVN-4913 is surely fixed, it is correct,
> but then how the error message should be when the arguments are
> passed in the form of relative paths ?
>
Just so I understand your question, are you asking what the ACTUAL error
message should be when passed a relative path (which may contain a symlink)?
If this is the question, then it goes deep within the library where the
relative path is resolved to an absolute path. The error message has always
returned the absolute path (libsvn_client/copy.c, approx 3100). Maybe it
would have been better to return a path relative to the wc root?
Kind regards,
Daniel
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
On 2023/10/09 4:14, Daniel Sahlberg wrote:
> Den lör 7 okt. 2023 kl 06:52 skrev Yasuhito FUTATSUKI <
> futatuki@yf.bsdclub.org>:
>> If svn is called with relative paths, it can only see
>> cwd as a realpath, and absolute paths of argment paths
>> are composed from it. So the paths in error message we
>> can expect here is absolute real paths or relative paths
>> only (It assumes that relative paths passed from the
>> command line don't contain symlinks).
> I was able to reproduce the issue and thanks to Yasuhito's hints I was also
> able to fix it by using absolute paths when running the svn command. I've
> committed r1912826 with a simple fix which seems to work for me.
I don't think r1912826 is a correct fix. For the main purpose of
the test, checking SVN-4913 is surely fixed, it is correct,
but then how the error message should be when the arguments are
passed in the form of relative paths ?
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den lör 7 okt. 2023 kl 06:52 skrev Yasuhito FUTATSUKI <
futatuki@yf.bsdclub.org>:
> Hi,
>
> On 2023/10/07 4:33, Daniel Sahlberg wrote:
> > Den fre 6 okt. 2023 kl 19:34 skrev Nathan Hartman <
> hartman.nathan@gmail.com
>
> >> The interesting thing, though, is that many tests in the test suite
> >> compare expected error strings to actual error strings, involving
> >> paths etc., and those are passing.
> >>
> >> What I also find interesting, besides the added escaping, is that the
> >> expected path is very different than the actual path. Showing these
> >> here without escaping for illustration:
> >>
> >> Expected:
> >>
> '/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests\-17'
> >> Actual:
> >>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
> >>
> >> The actual path is correct.
>
> In the copy_tests.wc_move_parent_into_child, it executes
> 'svn mv . F/B', which specified paths as relative paths,
> but its error message uses absolute real paths.
>
> On the other hand, expected paths are composed from
> sbox.wc_dir which may contain symlinks, i.e. it may not
> be real paths.
>
> If svn is called with relative paths, it can only see
> cwd as a realpath, and absolute paths of argment paths
> are composed from it. So the paths in error message we
> can expect here is absolute real paths or relative paths
> only (It assumes that relative paths passed from the
> command line don't contain symlinks).
>
>
I was able to reproduce the issue and thanks to Yasuhito's hints I was also
able to fix it by using absolute paths when running the svn command. I've
committed r1912826 with a simple fix which seems to work for me.
Nathan, can you verify if it also works for you?
Kind regards,
Daniel Sahlberg
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
Hi,
On 2023/10/07 4:33, Daniel Sahlberg wrote:
> Den fre 6 okt. 2023 kl 19:34 skrev Nathan Hartman <hartman.nathan@gmail.com
>> The interesting thing, though, is that many tests in the test suite
>> compare expected error strings to actual error strings, involving
>> paths etc., and those are passing.
>>
>> What I also find interesting, besides the added escaping, is that the
>> expected path is very different than the actual path. Showing these
>> here without escaping for illustration:
>>
>> Expected:
>> '/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests\-17'
>> Actual:
>> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
>>
>> The actual path is correct.
In the copy_tests.wc_move_parent_into_child, it executes
'svn mv . F/B', which specified paths as relative paths,
but its error message uses absolute real paths.
On the other hand, expected paths are composed from
sbox.wc_dir which may contain symlinks, i.e. it may not
be real paths.
If svn is called with relative paths, it can only see
cwd as a realpath, and absolute paths of argment paths
are composed from it. So the paths in error message we
can expect here is absolute real paths or relative paths
only (It assumes that relative paths passed from the
command line don't contain symlinks).
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den fre 6 okt. 2023 kl 19:34 skrev Nathan Hartman <hartman.nathan@gmail.com
>:
> On Fri, Oct 6, 2023 at 10:50 AM Daniel Sahlberg
> <da...@gmail.com> wrote:
> >
> > Den fre 6 okt. 2023 kl 06:26 skrev Nathan Hartman <
> hartman.nathan@gmail.com>:
> (snip)
> >> This is on trunk, as of r1912743 (the latest right now), Linux Debian,
> >> Python 3.7.5, building SVN with --enable-maintainer-mode (this might
> >> account for some extra text in the actual error message?).
> >
> >
> > I also build with --enable-maintainer-mode and it has not caused
> problems for me previously.
>
> Thanks. In retrospect I don't think this is an issue.
>
> Replying inline below...
>
> >> [[[
> >> W: Unexpected output
> >> W: EXPECTED STDERR (regexp, match_all=False):
> >> W: | svn: E200007: Cannot move path
> >>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
> >> into its own child
> >>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
> >
> >
> > Note the "\-", ie escaping of the - character. Can you check if removing
> the re.escape (reverting r1910129) helps? Since you are on *nix, the
> re.escape should not be needed (it was only there to account for the \ path
> separator in Windows).
>
> Ok I will check this.
>
> >> W: ACTUAL STDERR:
> >> W: | subversion/svn/move-cmd.c:102,
> >> W: | subversion/svn/util.c:557,
> >> W: | subversion/libsvn_client/copy.c:3495,
> >> W: | subversion/libsvn_client/copy.c:3093:
> (apr_err=SVN_ERR_UNSUPPORTED_FEATURE)
> >> W: | svn: E200007: Cannot move path
> >>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
> >> into its own child
> >>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
> >> W: CWD:
> /home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17
> >
> >
> > Note the extra "/ramdisk" here, between svndev and svn-trunk, missing
> from the expected error previously.
> >
> > What is the actual layout of your system? Do you have any specific
> arguments when starting the test suite affecting the location of the
> test-repo/wc:s?
>
> Yes, this ramdrive/.../ramdisk oddity does exist. I am using a
> (modified to fit my system) version of the Makefile.svn in
> tools/dev/unix-build, which builds all the dependencies and makes
> running tests easy with [local|svn|serf] x [bdb|fsfs|fsx]. This
> makefile can take a RAMDISK variable, e.g.,:
>
> $ make svn-check RAMDISK=$SVN_DEV/ramdisk JAVA=no BRANCH=1.14.x
>
> and then it puts all the test suite's temporary working copies in the
> specified directory.
>
> In the past, the whole tree was on Flash storage and I was using the
> RAMDISK variable as above for testing. More recently, to avoid Flash
> wear from lots of development activities, I started mounting a ramdisk
> on a mountpoint in my home directory (/home/nathan/ramdrive). $SVN_DEV
> points to /home/nathan/ramdrive/svndev. And inside there, for
> historical reasons, is a directory called ramdisk, which is still
> being passed in the RAMDISK variable. It is just a directory, not a
> mountpoint for a second ramdisk. So, yes, it's stupid, but there is in
> fact ~/ramdrive/svndev/ramdisk/...!! (And inconsistent naming:
> ramdrive, ramdisk...)
>
> The interesting thing, though, is that many tests in the test suite
> compare expected error strings to actual error strings, involving
> paths etc., and those are passing.
>
> What I also find interesting, besides the added escaping, is that the
> expected path is very different than the actual path. Showing these
> here without escaping for illustration:
>
> Expected:
> '/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests\-17'
> Actual:
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
>
> The actual path is correct.
>
From the test:
[[[
was_cwd = os.getcwd()
from_path = os.path.abspath(sbox.ospath(''))
to_path = os.path.abspath(sbox.ospath('F/B'))
os.chdir(wc_dir)
]]]
Would be interesting to know the values of was_cwd, return value of
sbox.ospath("") and how that translates with os.path.abspath() and the same
for "F/B".
I am assuming that in the expected path, 'svn-test-work' is probably a
> symlink to ~/ramdrive/svndev/ramdisk. I can't check this at the moment
> but I will try rerunning later and I'll inspect these locations.
>
The symlink being an artefact from your custom build system? Sounds like
you might be on to something.
>
> Is there a way to single-step through Python code that I could use to
> follow the test suite logic?
>
I've said it before; I'm no Python expert but maybe
https://docs.python.org/3/library/pdb.html? No idea if that will work with
the build system.
Kind regards,
Daniel
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Oct 6, 2023 at 10:50 AM Daniel Sahlberg
<da...@gmail.com> wrote:
>
> Den fre 6 okt. 2023 kl 06:26 skrev Nathan Hartman <ha...@gmail.com>:
(snip)
>> This is on trunk, as of r1912743 (the latest right now), Linux Debian,
>> Python 3.7.5, building SVN with --enable-maintainer-mode (this might
>> account for some extra text in the actual error message?).
>
>
> I also build with --enable-maintainer-mode and it has not caused problems for me previously.
Thanks. In retrospect I don't think this is an issue.
Replying inline below...
>> [[[
>> W: Unexpected output
>> W: EXPECTED STDERR (regexp, match_all=False):
>> W: | svn: E200007: Cannot move path
>> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
>> into its own child
>> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
>
>
> Note the "\-", ie escaping of the - character. Can you check if removing the re.escape (reverting r1910129) helps? Since you are on *nix, the re.escape should not be needed (it was only there to account for the \ path separator in Windows).
Ok I will check this.
>> W: ACTUAL STDERR:
>> W: | subversion/svn/move-cmd.c:102,
>> W: | subversion/svn/util.c:557,
>> W: | subversion/libsvn_client/copy.c:3495,
>> W: | subversion/libsvn_client/copy.c:3093: (apr_err=SVN_ERR_UNSUPPORTED_FEATURE)
>> W: | svn: E200007: Cannot move path
>> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
>> into its own child
>> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
>> W: CWD: /home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17
>
>
> Note the extra "/ramdisk" here, between svndev and svn-trunk, missing from the expected error previously.
>
> What is the actual layout of your system? Do you have any specific arguments when starting the test suite affecting the location of the test-repo/wc:s?
Yes, this ramdrive/.../ramdisk oddity does exist. I am using a
(modified to fit my system) version of the Makefile.svn in
tools/dev/unix-build, which builds all the dependencies and makes
running tests easy with [local|svn|serf] x [bdb|fsfs|fsx]. This
makefile can take a RAMDISK variable, e.g.,:
$ make svn-check RAMDISK=$SVN_DEV/ramdisk JAVA=no BRANCH=1.14.x
and then it puts all the test suite's temporary working copies in the
specified directory.
In the past, the whole tree was on Flash storage and I was using the
RAMDISK variable as above for testing. More recently, to avoid Flash
wear from lots of development activities, I started mounting a ramdisk
on a mountpoint in my home directory (/home/nathan/ramdrive). $SVN_DEV
points to /home/nathan/ramdrive/svndev. And inside there, for
historical reasons, is a directory called ramdisk, which is still
being passed in the RAMDISK variable. It is just a directory, not a
mountpoint for a second ramdisk. So, yes, it's stupid, but there is in
fact ~/ramdrive/svndev/ramdisk/...!! (And inconsistent naming:
ramdrive, ramdisk...)
The interesting thing, though, is that many tests in the test suite
compare expected error strings to actual error strings, involving
paths etc., and those are passing.
What I also find interesting, besides the added escaping, is that the
expected path is very different than the actual path. Showing these
here without escaping for illustration:
Expected: '/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svn-test-work/working_copies/copy_tests\-17'
Actual: '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
The actual path is correct.
I am assuming that in the expected path, 'svn-test-work' is probably a
symlink to ~/ramdrive/svndev/ramdisk. I can't check this at the moment
but I will try rerunning later and I'll inspect these locations.
Is there a way to single-step through Python code that I could use to
follow the test suite logic?
Cheers,
Nathan
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den fre 6 okt. 2023 kl 06:26 skrev Nathan Hartman <hartman.nathan@gmail.com
>:
> On Mon, May 29, 2023 at 10:47 PM Jun Omae <ju...@gmail.com> wrote:
> >
> > On 2023/05/29 20:09, Daniel Sahlberg wrote:
>
[...]
> > >
> > > The following patch could fix it and verified (applying
> `re.escape` to the paths).
>
[...]
>
> I'm running into a strange failure with the above test (copy_tests.py
> 17: move WC WC/subdir). The paths we construct for expected_error are
> not matching the actual error in my system. I tried various things but
> nothing seems to help.
>
> Looking around the testsuite, I tried changing '%s' to '.*%s'; also I
> tried removing os.path.abspath when constructing from_path and
> to_path; I made various other attempts, all aimed at minimally
> matching the known portion of the path while ignoring the extra stuff.
> Unfortunately I was not successful.
>
> This is on trunk, as of r1912743 (the latest right now), Linux Debian,
> Python 3.7.5, building SVN with --enable-maintainer-mode (this might
> account for some extra text in the actual error message?).
>
I also build with --enable-maintainer-mode and it has not caused problems
for me previously.
>
> All other tests are passing for me, for all [local|svn|serf] x [fsfs].
>
> Fail log:
>
> [[[
> W: Unexpected output
> W: EXPECTED STDERR (regexp, match_all=False):
> W: | svn: E200007: Cannot move path
>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
> into its own child
>
> '/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
>
Note the "\-", ie escaping of the - character. Can you check if removing
the re.escape (reverting r1910129) helps? Since you are on *nix, the
re.escape should not be needed (it was only there to account for the \ path
separator in Windows).
W: ACTUAL STDERR:
> W: | subversion/svn/move-cmd.c:102,
> W: | subversion/svn/util.c:557,
> W: | subversion/libsvn_client/copy.c:3495,
> W: | subversion/libsvn_client/copy.c:3093:
> (apr_err=SVN_ERR_UNSUPPORTED_FEATURE)
> W: | svn: E200007: Cannot move path
>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
> into its own child
>
> '/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
> W: CWD:
> /home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17
>
Note the extra "/ramdisk" here, between svndev and svn-trunk, missing from
the expected error previously.
What is the actual layout of your system? Do you have any specific
arguments when starting the test suite affecting the location of the
test-repo/wc:s?
> W: EXCEPTION: SVNUnmatchedError
> Traceback (most recent call last):
> File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/main.py",
> line 1996, in run
>
No "ramdisk" here...
> rc = self.pred.run(sandbox)
> File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/testcase.py",
> line 178, in run
> result = self.func(sandbox)
> File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/copy_tests.py",
> line 1299, in wc_move_parent_into_child
> '.', 'F/B')
> File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/actions.py",
> line 340, in run_and_verify_svn
> expected_exit, *varargs)
> File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/actions.py",
> line 380, in run_and_verify_svn2
> expected_stdout, expected_stderr)
> File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/verify.py",
> line 532, in verify_outputs
> compare_and_display_lines(message, label, expected, actual, raisable)
> File
> "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/verify.py",
> line 505, in compare_and_display_lines
> raise raisable
> svntest.main.SVNUnmatchedError
> FAIL: copy_tests.py 17: move WC WC/subdir
> ]]]
>
> Any ideas?
>
> Cheers,
> Nathan
>
Kind regards,
Daniel
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Nathan Hartman <ha...@gmail.com>.
On Mon, May 29, 2023 at 10:47 PM Jun Omae <ju...@gmail.com> wrote:
>
> On 2023/05/29 20:09, Daniel Sahlberg wrote:
> > > For test 17:
> > > [[[
> > > File "C:\usr\apps\python312\Lib\re\_parser.py", line 383, in _escape
> > > raise source.error("incomplete escape %s" % escape, len(escape))
> > > re.error: incomplete escape \u at position 34
> > > ]]]
> > >
> > > I suppose this is because the expected_error is constructed like this:
> > > [[[
> > > expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> > > "child '%s'" % (from_path, to_path)
> > > ]]]
> > >
> > > With a from_path C:\usr\subversion\[...], I suppose Python interpret \ as an escape character. I'm not proficient enough in Python to have an idea of how to escape from_path and to_path in an appropriate way. @Jun, do you have a suggestion?
> >
> > The following patch could fix it and verified (applying `re.escape` to the paths).
> >
> > [[[
> > Index: subversion/tests/cmdline/copy_tests.py
> > ===================================================================
> > --- subversion/tests/cmdline/copy_tests.py (revision 1910112)
> > +++ subversion/tests/cmdline/copy_tests.py (working copy)
> > @@ -1295,7 +1295,7 @@ def wc_move_parent_into_child(sbox):
> > os.chdir(wc_dir)
> >
> > expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> > - "child '%s'" % (from_path, to_path)
> > + "child '%s'" % (re.escape(from_path), re.escape(to_path))
> > svntest.actions.run_and_verify_svn(None, expected_error,
> > 'mv',
> > '.', 'F/B')
> > ]]]
> >
> >
> > Sounds good, thanks for your suggestion. Would you like to commit?
>
> I just pushed the changes in r1910129.
I'm running into a strange failure with the above test (copy_tests.py
17: move WC WC/subdir). The paths we construct for expected_error are
not matching the actual error in my system. I tried various things but
nothing seems to help.
Looking around the testsuite, I tried changing '%s' to '.*%s'; also I
tried removing os.path.abspath when constructing from_path and
to_path; I made various other attempts, all aimed at minimally
matching the known portion of the path while ignoring the extra stuff.
Unfortunately I was not successful.
This is on trunk, as of r1912743 (the latest right now), Linux Debian,
Python 3.7.5, building SVN with --enable-maintainer-mode (this might
account for some extra text in the actual error message?).
All other tests are passing for me, for all [local|svn|serf] x [fsfs].
Fail log:
[[[
W: Unexpected output
W: EXPECTED STDERR (regexp, match_all=False):
W: | svn: E200007: Cannot move path
'/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17'
into its own child
'/home/nathan/ramdrive/svndev/svn\-trunk/subversion/tests/cmdline/svn\-test\-work/working_copies/copy_tests\-17/F/B'
W: ACTUAL STDERR:
W: | subversion/svn/move-cmd.c:102,
W: | subversion/svn/util.c:557,
W: | subversion/libsvn_client/copy.c:3495,
W: | subversion/libsvn_client/copy.c:3093: (apr_err=SVN_ERR_UNSUPPORTED_FEATURE)
W: | svn: E200007: Cannot move path
'/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17'
into its own child
'/home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17/F/B'
W: CWD: /home/nathan/ramdrive/svndev/ramdisk/svn-trunk/working_copies/copy_tests-17
W: EXCEPTION: SVNUnmatchedError
Traceback (most recent call last):
File "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/main.py",
line 1996, in run
rc = self.pred.run(sandbox)
File "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/testcase.py",
line 178, in run
result = self.func(sandbox)
File "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/copy_tests.py",
line 1299, in wc_move_parent_into_child
'.', 'F/B')
File "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/actions.py",
line 340, in run_and_verify_svn
expected_exit, *varargs)
File "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/actions.py",
line 380, in run_and_verify_svn2
expected_stdout, expected_stderr)
File "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/verify.py",
line 532, in verify_outputs
compare_and_display_lines(message, label, expected, actual, raisable)
File "/home/nathan/ramdrive/svndev/svn-trunk/subversion/tests/cmdline/svntest/verify.py",
line 505, in compare_and_display_lines
raise raisable
svntest.main.SVNUnmatchedError
FAIL: copy_tests.py 17: move WC WC/subdir
]]]
Any ideas?
Cheers,
Nathan
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Jun Omae <ju...@gmail.com>.
On 2023/05/29 20:09, Daniel Sahlberg wrote:
> > For test 17:
> > [[[
> > File "C:\usr\apps\python312\Lib\re\_parser.py", line 383, in _escape
> > raise source.error("incomplete escape %s" % escape, len(escape))
> > re.error: incomplete escape \u at position 34
> > ]]]
> >
> > I suppose this is because the expected_error is constructed like this:
> > [[[
> > expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> > "child '%s'" % (from_path, to_path)
> > ]]]
> >
> > With a from_path C:\usr\subversion\[...], I suppose Python interpret \ as an escape character. I'm not proficient enough in Python to have an idea of how to escape from_path and to_path in an appropriate way. @Jun, do you have a suggestion?
>
> The following patch could fix it and verified (applying `re.escape` to the paths).
>
> [[[
> Index: subversion/tests/cmdline/copy_tests.py
> ===================================================================
> --- subversion/tests/cmdline/copy_tests.py (revision 1910112)
> +++ subversion/tests/cmdline/copy_tests.py (working copy)
> @@ -1295,7 +1295,7 @@ def wc_move_parent_into_child(sbox):
> os.chdir(wc_dir)
>
> expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> - "child '%s'" % (from_path, to_path)
> + "child '%s'" % (re.escape(from_path), re.escape(to_path))
> svntest.actions.run_and_verify_svn(None, expected_error,
> 'mv',
> '.', 'F/B')
> ]]]
>
>
> Sounds good, thanks for your suggestion. Would you like to commit?
I just pushed the changes in r1910129.
> > (Sidenote: There is an error message in the test log about line 1424. This was added way back in r846892 and has essentially been unchanged ever since. I believe this is unrelated and I will send this in a separate thread when I've had some more time to look at it).
>
> The `SyntaxWarning` from line 1424 is displayed since Python 3.12, even without -Wdefault option.
>
> $ python3.12 -c '"\$"'
> <string>:1: SyntaxWarning: invalid escape sequence '\$'
> $ python3.11 -c '"\$"' # no warnings
> $ python3.11 -Wdefault -c '"\$"'
> <string>:1: DeprecationWarning: invalid escape sequence '\$'
>
> See also: https://docs.python.org/3.12/whatsnew/3.12.html?highlight=backslash-character#other-language-changes <https://docs.python.org/3.12/whatsnew/3.12.html?highlight=backslash-character#other-language-changes>
>
>
> If I understand this correctly, the string should be declared as raw instead of binary:
> [[[
> Index: subversion/tests/cmdline/copy_tests.py
> ===================================================================
> --- subversion/tests/cmdline/copy_tests.py (revision 1910111)
> +++ subversion/tests/cmdline/copy_tests.py (working copy)
> @@ -1421,7 +1421,7 @@
> if re.match(b'[^\\r]\\n', raw_contents):
> raise svntest.Failure
>
> - if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$', line_contents[3]):
> + if not re.match(r'.*\$LastChangedRevision:\s*\d+\s*\$', line_contents[3]):
> raise svntest.Failure
>
> #-------------------------------------------------------------
> ]]]
>
> Maybe the previous re.match (first context line in the diff) should be changed as well?
The literal for the regular expression should be a bytes instance because line_contents[3] is a bytes instance.
If Python is 3.3 or later, we could use a raw bytes literal.
[[[
- if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$', line_contents[3]):
+ if not re.match(rb'.*\$LastChangedRevision:\s*\d+\s*\$', line_contents[3]):
]]]
However, in attached patch, correct the escape sequences because "make check" requires at least Python 3.0.
[[[
- if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$', line_contents[3]):
+ if not re.match(b'.*\\$LastChangedRevision:\\s*\\d+\\s*\\$', line_contents[3]):
]]]
The patch fixes many "invalid escape sequences" which exist in also other Python scripts (verified unit tests with Python 3.5 and 3.12 on Ubuntu, and Python 3.12 on Windows).
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den mån 29 maj 2023 kl 12:38 skrev Jun Omae <ju...@gmail.com>:
> On 2023/05/29 18:58, Daniel Sahlberg wrote:
> > For test 16:
> > [[[
> > W: Unexpected output
> > W: EXPECTED STDERR (regexp, match_all=False):
> > W: | svn: E200007: Cannot move path '.*
> http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B <
> http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B>'
> into its own child '.*
> http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B/F <
> http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B/F>'
> > W: ACTUAL STDERR:
> > W: | svn: E200007: Cannot move path
> 'http:\\localhost:17838\svn-test-work\repositories\copy_tests-16\A\B' into
> its own child
> 'http:\\localhost:17838\svn-test-work\repositories\copy_tests-16\A\B\F'
> > ]]]
> >
> > There was a call to svn_dirent_local_style on the error message, which
> obviously turned \ to / on Windows. I've added a check in r1910112, please
> check if this works for you.
>
> Thanks. Verified that copy_tests.py 16 goes away with r1910112.
>
Thanks for checking!
> > For test 17:
> > [[[
> > File "C:\usr\apps\python312\Lib\re\_parser.py", line 383, in _escape
> > raise source.error("incomplete escape %s" % escape, len(escape))
> > re.error: incomplete escape \u at position 34
> > ]]]
> >
> > I suppose this is because the expected_error is constructed like this:
> > [[[
> > expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> > "child '%s'" % (from_path, to_path)
> > ]]]
> >
> > With a from_path C:\usr\subversion\[...], I suppose Python interpret \
> as an escape character. I'm not proficient enough in Python to have an idea
> of how to escape from_path and to_path in an appropriate way. @Jun, do you
> have a suggestion?
>
> The following patch could fix it and verified (applying `re.escape` to the
> paths).
>
> [[[
> Index: subversion/tests/cmdline/copy_tests.py
> ===================================================================
> --- subversion/tests/cmdline/copy_tests.py (revision 1910112)
> +++ subversion/tests/cmdline/copy_tests.py (working copy)
> @@ -1295,7 +1295,7 @@ def wc_move_parent_into_child(sbox):
> os.chdir(wc_dir)
>
> expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> - "child '%s'" % (from_path, to_path)
> + "child '%s'" % (re.escape(from_path),
> re.escape(to_path))
> svntest.actions.run_and_verify_svn(None, expected_error,
> 'mv',
> '.', 'F/B')
> ]]]
>
Sounds good, thanks for your suggestion. Would you like to commit?
> > (Sidenote: There is an error message in the test log about line 1424.
> This was added way back in r846892 and has essentially been unchanged ever
> since. I believe this is unrelated and I will send this in a separate
> thread when I've had some more time to look at it).
>
> The `SyntaxWarning` from line 1424 is displayed since Python 3.12, even
> without -Wdefault option.
>
> $ python3.12 -c '"\$"'
> <string>:1: SyntaxWarning: invalid escape sequence '\$'
> $ python3.11 -c '"\$"' # no warnings
> $ python3.11 -Wdefault -c '"\$"'
> <string>:1: DeprecationWarning: invalid escape sequence '\$'
>
> See also:
> https://docs.python.org/3.12/whatsnew/3.12.html?highlight=backslash-character#other-language-changes
If I understand this correctly, the string should be declared as raw
instead of binary:
[[[
Index: subversion/tests/cmdline/copy_tests.py
===================================================================
--- subversion/tests/cmdline/copy_tests.py (revision 1910111)
+++ subversion/tests/cmdline/copy_tests.py (working copy)
@@ -1421,7 +1421,7 @@
if re.match(b'[^\\r]\\n', raw_contents):
raise svntest.Failure
- if not re.match(b'.*\$LastChangedRevision:\s*\d+\s*\$',
line_contents[3]):
+ if not re.match(r'.*\$LastChangedRevision:\s*\d+\s*\$',
line_contents[3]):
raise svntest.Failure
#-------------------------------------------------------------
]]]
Maybe the previous re.match (first context line in the diff) should be
changed as well?
Kind regards,
Daniel
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Jun Omae <ju...@gmail.com>.
On 2023/05/29 18:58, Daniel Sahlberg wrote:
> For test 16:
> [[[
> W: Unexpected output
> W: EXPECTED STDERR (regexp, match_all=False):
> W: | svn: E200007: Cannot move path '.*http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B <http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B>' into its own child '.*http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B/F <http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B/F>'
> W: ACTUAL STDERR:
> W: | svn: E200007: Cannot move path 'http:\\localhost:17838\svn-test-work\repositories\copy_tests-16\A\B' into its own child 'http:\\localhost:17838\svn-test-work\repositories\copy_tests-16\A\B\F'
> ]]]
>
> There was a call to svn_dirent_local_style on the error message, which obviously turned \ to / on Windows. I've added a check in r1910112, please check if this works for you.
Thanks. Verified that copy_tests.py 16 goes away with r1910112.
> For test 17:
> [[[
> File "C:\usr\apps\python312\Lib\re\_parser.py", line 383, in _escape
> raise source.error("incomplete escape %s" % escape, len(escape))
> re.error: incomplete escape \u at position 34
> ]]]
>
> I suppose this is because the expected_error is constructed like this:
> [[[
> expected_error = "svn: E200007: Cannot move path '%s' into its own " \
> "child '%s'" % (from_path, to_path)
> ]]]
>
> With a from_path C:\usr\subversion\[...], I suppose Python interpret \ as an escape character. I'm not proficient enough in Python to have an idea of how to escape from_path and to_path in an appropriate way. @Jun, do you have a suggestion?
The following patch could fix it and verified (applying `re.escape` to the paths).
[[[
Index: subversion/tests/cmdline/copy_tests.py
===================================================================
--- subversion/tests/cmdline/copy_tests.py (revision 1910112)
+++ subversion/tests/cmdline/copy_tests.py (working copy)
@@ -1295,7 +1295,7 @@ def wc_move_parent_into_child(sbox):
os.chdir(wc_dir)
expected_error = "svn: E200007: Cannot move path '%s' into its own " \
- "child '%s'" % (from_path, to_path)
+ "child '%s'" % (re.escape(from_path), re.escape(to_path))
svntest.actions.run_and_verify_svn(None, expected_error,
'mv',
'.', 'F/B')
]]]
> (Sidenote: There is an error message in the test log about line 1424. This was added way back in r846892 and has essentially been unchanged ever since. I believe this is unrelated and I will send this in a separate thread when I've had some more time to look at it).
The `SyntaxWarning` from line 1424 is displayed since Python 3.12, even without -Wdefault option.
$ python3.12 -c '"\$"'
<string>:1: SyntaxWarning: invalid escape sequence '\$'
$ python3.11 -c '"\$"' # no warnings
$ python3.11 -Wdefault -c '"\$"'
<string>:1: DeprecationWarning: invalid escape sequence '\$'
See also: https://docs.python.org/3.12/whatsnew/3.12.html?highlight=backslash-character#other-language-changes
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den mån 29 maj 2023 kl 02:13 skrev Jun Omae <ju...@gmail.com>:
> Hi,
>
> On Fri, Apr 14, 2023 at 6:09 AM Daniel Sahlberg
> <da...@gmail.com> wrote:
> > I have committed a fix as r1909127.
> >
> > Kind regards,
> > Daniel
>
> Added 2 tests to copy_tests.py in r1909127 failed on Windows:
>
> [[[
> FAIL: copy_tests.py 16: move URL URL/subdir
> FAIL: copy_tests.py 17: move WC WC/subdir
> ]]]
>
> See attached dav-fails.log.
>
For test 16:
[[[
W: Unexpected output
W: EXPECTED STDERR (regexp, match_all=False):
W: | svn: E200007: Cannot move path '.*
http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B'
into its own child '.*
http://localhost:17838/svn\-test\-work/repositories/copy_tests\-16/A/B/F'
W: ACTUAL STDERR:
W: | svn: E200007: Cannot move path
'http:\\localhost:17838\svn-test-work\repositories\copy_tests-16\A\B' into
its own child
'http:\\localhost:17838\svn-test-work\repositories\copy_tests-16\A\B\F'
]]]
There was a call to svn_dirent_local_style on the error message, which
obviously turned \ to / on Windows. I've added a check in r1910112, please
check if this works for you.
For test 17:
[[[
File "C:\usr\apps\python312\Lib\re\_parser.py", line 383, in _escape
raise source.error("incomplete escape %s" % escape, len(escape))
re.error: incomplete escape \u at position 34
]]]
I suppose this is because the expected_error is constructed like this:
[[[
expected_error = "svn: E200007: Cannot move path '%s' into its own " \
"child '%s'" % (from_path, to_path)
]]]
With a from_path C:\usr\subversion\[...], I suppose Python interpret \ as
an escape character. I'm not proficient enough in Python to have an idea of
how to escape from_path and to_path in an appropriate way. @Jun, do you
have a suggestion?
(Sidenote: There is an error message in the test log about line 1424. This
was added way back in r846892 and has essentially been unchanged ever
since. I believe this is unrelated and I will send this in a separate
thread when I've had some more time to look at it).
Kind regards,
Daniel
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Jun Omae <ju...@gmail.com>.
Hi,
On Fri, Apr 14, 2023 at 6:09 AM Daniel Sahlberg
<da...@gmail.com> wrote:
> I have committed a fix as r1909127.
>
> Kind regards,
> Daniel
Added 2 tests to copy_tests.py in r1909127 failed on Windows:
[[[
FAIL: copy_tests.py 16: move URL URL/subdir
FAIL: copy_tests.py 17: move WC WC/subdir
]]]
See attached dav-fails.log.
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den fre 7 apr. 2023 kl 10:50 skrev Pavel Lyalyakin <
pavel.lyalyakin@visualsvn.com>:
> Daniel,
>
> Thank you for looking into this!
>
> On a side note, this seems to be a rare problem. I don't see any other
> reports of this particular assertion (error message).
>
I have committed a fix as r1909127.
Kind regards,
Daniel
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
I've created SVN-4913 to cover this error.
Current behaviour:
Moving the repo root causes an assert (as in Pavel's initial report):
[[[
$ svn move $REPOURL $REPOURL/SUBDIR -m 'mv root'
svn: E235000: In file '../subversion/libsvn_client/copy.c' line 1187:
assertion failed (! svn_path_is_empty(path))
Aborted
]]]
Moving a directory below the repo root gives a less then clear error
message (A certainly exists in the repository, but not after the move):
[[[
$ svn move $REPOURL/A $REPOURL/A/SUBDIR -m 'mv dir'
svn: E160016: Path 'A' not present
]]]
Thus, moving a directory to its child does not work in either case.
I'm proposing the following fix:
[[[
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 1909002)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -3077,7 +3077,7 @@
APR_ARRAY_PUSH(copy_pairs, svn_client__copy_pair_t *) = pair;
}
- if (!srcs_are_urls && !dst_is_url)
+ if (is_move || (!srcs_are_urls && !dst_is_url))
{
apr_pool_t *iterpool = svn_pool_create(pool);
]]]
(This basically restores the check for a move to a child directory).
Both test cases now return the same error
[[[
$ svn move $REPOURL $REPOURL/SUBDIR -m 'mv root'
svn: E200007: Cannot copy path '$REPOURL' into its own child
'$REPOURL/SUBDIR'
$ svn move $REPOURL/A2 $REPOURL/A/SUBDIR -m 'mv dir'
svn: E200007: Cannot copy path '$REPOURL/A' into its own child
'$REPOURL/A/SUBDIR'
]]]
I believe it is reasonable to change error code as above. Although I
realise it says "copy", should be "move" in the final commit.
Kind regards,
Daniel Sahlberg
>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
fre 7 apr. 2023 kl. 10:50 skrev Pavel Lyalyakin <
pavel.lyalyakin@visualsvn.com>:
> On a side note, this seems to be a rare problem. I don't see any other
> reports of this particular assertion (error message).
>
I would say this assert is rare because the action (svn move something
something/subdirectory) is not logical to do. Still, there should be an
error rather than an assert. Even doing svn copy something
something/subdirectory makes little sense to me but it would probably work
since something will still be there after the copy.
Thus it is worth fixing. I’ll try to start by writing some test cases.
Kind regards
Daniel
>
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Pavel Lyalyakin via dev <de...@subversion.apache.org>.
On Fri, Apr 7, 2023 at 12:53 AM Daniel Sahlberg <da...@gmail.com>
wrote:
> Den tis 7 mars 2023 kl 20:24 skrev Pavel Lyalyakin via dev <
> dev@subversion.apache.org>:
>
>> Hello,
>>
>> I seem to have found a new bug (I can't find similar reports) so I
>> thought I'd report it to this mailing list. Sorry if it's a known issue.
>>
>> I believe that the command shouldn't work anyway, right? But it shouldn't
>> assert either.
>>
>> The SVN client crashes when I run the following command:
>> [[[
>> svn move https://svn1.mydomain.com/svn/MyRepo/
>> https://svn1.mydomain.com/svn/MyRepo/trunk -m "Test Commit"
>> ]]]
>> [[[
>> svn: E235000: In file '..\..\..\subversion\libsvn_client\copy.c' line
>> 1187: assertion failed (! svn_path_is_empty(path))
>> ]]]
>>
>> SRC URL points to a root of a repository. DST URL points to a
>> non-existent subdirectory.
>>
>
> Hi
>
> I think I have found the root cause and the relevant commit. I will
> document my findings so far in case someone else would like to look at it,
> I need to do some more analysis before I'm ready to suggest a solution.
>
> In subversion/libsvn_client/copy.c, setup_copy, there is a check:
>
> [[[
> if (svn_dirent_is_child(pair->src_abspath_or_url,
> pair->dst_abspath_or_url, iterpool))
> return svn_error_createf
> (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> _("Cannot copy path '%s' into its own child '%s'"),
> svn_dirent_local_style(pair->src_abspath_or_url, pool),
> svn_dirent_local_style(pair->dst_abspath_or_url, pool));
> ]]]
>
> This should prevent the assertion and instead give a reasonable error
> message.
>
> However, a few lines above, this code is only executed
>
> [[[
> if (!srcs_are_urls && !dst_is_url)
> ]]]
>
> This condition seems to come from r846422, with a log message referencing
> issue SVN-1367 (from 2003!). There is a reasonable argument in the issue
> description on why this condition was added (it was supposed to manage svn
> copy). There is an argument "is_move" to the function, so it might be
> reasonable to also look at that variable in the above condition.
>
> Kind regards,
> Daniel
>
Daniel,
Thank you for looking into this!
On a side note, this seems to be a rare problem. I don't see any other
reports of this particular assertion (error message).
--
With best regards,
Pavel Lyalyakin
VisualSVN Team
Re: "copy.c' line 1187: assertion failed (! svn_path_is_empty(path))" when running `svn move `
Posted by Daniel Sahlberg <da...@gmail.com>.
Den tis 7 mars 2023 kl 20:24 skrev Pavel Lyalyakin via dev <
dev@subversion.apache.org>:
> Hello,
>
> I seem to have found a new bug (I can't find similar reports) so I thought
> I'd report it to this mailing list. Sorry if it's a known issue.
>
> I believe that the command shouldn't work anyway, right? But it shouldn't
> assert either.
>
> The SVN client crashes when I run the following command:
> [[[
> svn move https://svn1.mydomain.com/svn/MyRepo/
> https://svn1.mydomain.com/svn/MyRepo/trunk -m "Test Commit"
> ]]]
> [[[
> svn: E235000: In file '..\..\..\subversion\libsvn_client\copy.c' line
> 1187: assertion failed (! svn_path_is_empty(path))
> ]]]
>
> SRC URL points to a root of a repository. DST URL points to a non-existent
> subdirectory.
>
Hi
I think I have found the root cause and the relevant commit. I will
document my findings so far in case someone else would like to look at it,
I need to do some more analysis before I'm ready to suggest a solution.
In subversion/libsvn_client/copy.c, setup_copy, there is a check:
[[[
if (svn_dirent_is_child(pair->src_abspath_or_url,
pair->dst_abspath_or_url, iterpool))
return svn_error_createf
(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
_("Cannot copy path '%s' into its own child '%s'"),
svn_dirent_local_style(pair->src_abspath_or_url, pool),
svn_dirent_local_style(pair->dst_abspath_or_url, pool));
]]]
This should prevent the assertion and instead give a reasonable error
message.
However, a few lines above, this code is only executed
[[[
if (!srcs_are_urls && !dst_is_url)
]]]
This condition seems to come from r846422, with a log message referencing
issue SVN-1367 (from 2003!). There is a reasonable argument in the issue
description on why this condition was added (it was supposed to manage svn
copy). There is an argument "is_move" to the function, so it might be
reasonable to also look at that variable in the above condition.
Kind regards,
Daniel