You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jens Peters <jp...@gmx.de> on 2009/06/21 10:01:38 UTC
[PATCH] Fix python wc testcase failure
Hello there,
I did some research/debugging to get to the ground of this test failure:
http://svn.haxx.se/dev/archive-2009-05/0087.shtml
It turns out that its just a mix up of wc.get_status_editor2 arguments.
The following patch fixes that, at least for me.
With this patch I can finally finish the python without a failure (yes :)).
Regards,
jens
[[[
Fix a python wc testcase failure.
(SubversionException, "Can't check path '[path][path]': The filename,
directory name, or volume label syntax is incorrect.")
* subversion/bindings/swig/python/tests/wc.py
(test_status_editor):
Put the correct arguments into 'wc.get_status_editor2'. The second
argument should be 'target'.
Patch by: Jens Peters <jpeters7677 { at } gmx.de>
]]]
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2363926
Re: [PATCH] Fix python wc testcase failure
Posted by Gavin Baumanis <ga...@thespidernet.com>.
Thanks Roman,
I'll remove it from m watch-list.
Gavin.
On 24/07/2009, at 01:22 , Roman Donchenko wrote:
> Jens Peters <jp...@gmx.de> писал в своём
> письме Sun, 21 Jun 2009
> 14:01:38 +0400:
>
>> Hello there,
>>
>> I did some research/debugging to get to the ground of this test
>> failure:
>> http://svn.haxx.se/dev/archive-2009-05/0087.shtml
>>
>> It turns out that its just a mix up of wc.get_status_editor2
>> arguments.
>> The following patch fixes that, at least for me.
>>
>> With this patch I can finally finish the python without a failure
>> (yes
>> :)).
>>
>> Regards,
>> jens
>>
>> [[[
>> Fix a python wc testcase failure.
>> (SubversionException, "Can't check path '[path][path]': The filename,
>> directory name, or volume label syntax is incorrect.")
>>
>> * subversion/bindings/swig/python/tests/wc.py
>> (test_status_editor):
>> Put the correct arguments into 'wc.get_status_editor2'. The second
>> argument should be 'target'.
>>
>> Patch by: Jens Peters <jpeters7677 { at } gmx.de>
>> ]]]
>
> Committed in r38475 with my newly-obtained demigodlike powers.
>
> Roman.
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2374858
Gavin Baumanis
P: +61 438 545 586
E: gavinb@thespidernet.com
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2375004
Re: [PATCH] Fix python wc testcase failure
Posted by Roman Donchenko <DX...@yandex.ru>.
Jens Peters <jp...@gmx.de> писал в своём письме Sun, 21 Jun 2009
14:01:38 +0400:
> Hello there,
>
> I did some research/debugging to get to the ground of this test failure:
> http://svn.haxx.se/dev/archive-2009-05/0087.shtml
>
> It turns out that its just a mix up of wc.get_status_editor2 arguments.
> The following patch fixes that, at least for me.
>
> With this patch I can finally finish the python without a failure (yes
> :)).
>
> Regards,
> jens
>
> [[[
> Fix a python wc testcase failure.
> (SubversionException, "Can't check path '[path][path]': The filename,
> directory name, or volume label syntax is incorrect.")
>
> * subversion/bindings/swig/python/tests/wc.py
> (test_status_editor):
> Put the correct arguments into 'wc.get_status_editor2'. The second
> argument should be 'target'.
>
> Patch by: Jens Peters <jpeters7677 { at } gmx.de>
> ]]]
Committed in r38475 with my newly-obtained demigodlike powers.
Roman.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2374858
Re: [PATCH] Fix python wc testcase failure
Posted by "C. Michael Pilato" <cm...@collab.net>.
> In 1.7, *all* working copy APIs will state explicitly they require
> absolute paths. Hopefully, this will fix problems and confusion like
> this.
Awesome! This should remedy many of the Windows users' issues with paths
being too long on that platform. (Absolute paths can be 65k long or so;
relative ones only 255 characters or so.)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2371655
Re: [PATCH] Fix python wc testcase failure
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On Jul 17, 2009, at 12:27 PM, Roman Donchenko wrote:
> Hyrum K. Wright <hy...@mail.utexas.edu> писал в
> своём письме Thu,
> 16 Jul 2009 01:15:00 +0400:
>
>> In 1.7, *all* working copy APIs will state explicitly they require
>> absolute paths. Hopefully, this will fix problems and confusion like
>> this.
>
> But svn_wc_get_status_editor5 is new to 1.7, yet it neither states,
> nor
> requires. Does that mean it's going to be rewritten?
Yes. At some point, the adm_access baton will be replaced with a
working copy context, and the path will be explicitly required to be
an absolute path.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2372145
Re: [PATCH] Fix python wc testcase failure
Posted by Roman Donchenko <DX...@yandex.ru>.
Hyrum K. Wright <hy...@mail.utexas.edu> писал в своём письме Thu,
16 Jul 2009 01:15:00 +0400:
> In 1.7, *all* working copy APIs will state explicitly they require
> absolute paths. Hopefully, this will fix problems and confusion like
> this.
But svn_wc_get_status_editor5 is new to 1.7, yet it neither states, nor
requires. Does that mean it's going to be rewritten?
Roman.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2372130
Re: [PATCH] Fix python wc testcase failure
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On Jul 11, 2009, at 4:42 PM, Роман Донченко wrote:
> Jens Peters <jp...@gmx.de> писал в своём
> письме Sun, 05 Jul 2009
> 23:49:41 +0400:
>
>> Any thoughts?
>
> I analyzed this and I agree with you.
>
> It all really boils down to the following line in the function
> close_edit
> in status.c:
>
> const char *full_path = svn_path_join(eb->anchor, eb->target, pool);
>
> The way the test creates the status editor, both anchor and target are
> absolute paths to some temporary directory.
>
> On Unix-likes, it's something like '/tmp/abcd123'; due to the way
> svn_path_join operates, full_path becomes equal to '/tmp/abcd123',
> which
> is correct. On Windows, it's something like 'C:\Temp\abcd123', and
> full_path becomes 'C:\Temp\abcd123\C:\Temp\abcd123', which is
> obviously
> wrong. As such, the test passed on Unixes when it was written (and
> presumably still does), yet fails on Windows (and presumably did
> earlier).
>
> Short version: it has nothing to do with backwards compatibility, +1
> on
> your patch.
>
> Interestingly enough, on trunk the test succeeds; it happens because
> on
> that line svn_path_join was replaced with svn_dirent_join, which
> does the
> right thing for Windows paths too. However the doc for
> svn_wc_get_status_editor5 weakly implies that target should be a
> relative
> path, so the test still violates the contract.
In 1.7, *all* working copy APIs will state explicitly they require
absolute paths. Hopefully, this will fix problems and confusion like
this.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2371628
Re: [PATCH] Fix python wc testcase failure
Posted by Роман Донченко <DX...@yandex.ru>.
Jens Peters <jp...@gmx.de> писал в своём письме Sun, 05 Jul 2009
23:49:41 +0400:
> Any thoughts?
I analyzed this and I agree with you.
It all really boils down to the following line in the function close_edit
in status.c:
const char *full_path = svn_path_join(eb->anchor, eb->target, pool);
The way the test creates the status editor, both anchor and target are
absolute paths to some temporary directory.
On Unix-likes, it's something like '/tmp/abcd123'; due to the way
svn_path_join operates, full_path becomes equal to '/tmp/abcd123', which
is correct. On Windows, it's something like 'C:\Temp\abcd123', and
full_path becomes 'C:\Temp\abcd123\C:\Temp\abcd123', which is obviously
wrong. As such, the test passed on Unixes when it was written (and
presumably still does), yet fails on Windows (and presumably did earlier).
Short version: it has nothing to do with backwards compatibility, +1 on
your patch.
Interestingly enough, on trunk the test succeeds; it happens because on
that line svn_path_join was replaced with svn_dirent_join, which does the
right thing for Windows paths too. However the doc for
svn_wc_get_status_editor5 weakly implies that target should be a relative
path, so the test still violates the contract.
Roman.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2370706
Re: [PATCH] Fix python wc testcase failure
Posted by Jens Peters <jp...@gmx.de>.
> Even the very first version of get_status_editor uses 'target' as
> argument
> (http://svn.collab.net/viewvc/svn/branches/1.0.x/subversion/include/svn_wc.h?revision=8524&view=markup,
> line 1174). But I do wonder as well how the testcase could ever pass.
>
> Just a shot in the dark, but it could pass on a linux box if self.path
> is './', not sure though...
Any thoughts?
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2368172
Re: [PATCH] Fix python wc testcase failure
Posted by Jens Peters <jp...@gmx.de>.
On 22-6-2009 16:29, Hyrum K. Wright wrote:
> Hmmm, I wonder if this is really a problem with the underlying
> backward compat C code. The test presumably passed when it was
> originally added, but when the underlying APIs got rewritten, there
> may be some bugs in the backward compatibility wrapper. If that's the
> case, we should fix the bugs there, not in the python testsuite.
>
> Caveat: I haven't looked at the history of this test, or the
> underlying code, so I could be completely making stuff up. But it's a
> thought.
Even the very first version of get_status_editor uses 'target' as
argument
(http://svn.collab.net/viewvc/svn/branches/1.0.x/subversion/include/svn_wc.h?revision=8524&view=markup,
line 1174). But I do wonder as well how the testcase could ever pass.
Just a shot in the dark, but it could pass on a linux box if self.path
is './', not sure though...
Regards,
Jens
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2364265
Re: [PATCH] Fix python wc testcase failure
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On Jun 21, 2009, at 5:01 AM, Jens Peters wrote:
> Hello there,
>
> I did some research/debugging to get to the ground of this test
> failure:
> http://svn.haxx.se/dev/archive-2009-05/0087.shtml
>
> It turns out that its just a mix up of wc.get_status_editor2
> arguments.
> The following patch fixes that, at least for me.
>
> With this patch I can finally finish the python without a failure
> (yes :)).
>
> Regards,
> jens
>
> [[[
> Fix a python wc testcase failure.
> (SubversionException, "Can't check path '[path][path]': The filename,
> directory name, or volume label syntax is incorrect.")
>
> * subversion/bindings/swig/python/tests/wc.py
> (test_status_editor):
> Put the correct arguments into 'wc.get_status_editor2'. The second
> argument should be 'target'.
>
> Patch by: Jens Peters <jpeters7677 { at } gmx.de>
> ]]]
Hmmm, I wonder if this is really a problem with the underlying
backward compat C code. The test presumably passed when it was
originally added, but when the underlying APIs got rewritten, there
may be some bugs in the backward compatibility wrapper. If that's the
case, we should fix the bugs there, not in the python testsuite.
Caveat: I haven't looked at the history of this test, or the
underlying code, so I could be completely making stuff up. But it's a
thought.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2364184