You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Barry Scott <ba...@barrys-emacs.org> on 2011/11/02 01:11:59 UTC
1.7.0 assert on svn_client_checkout with E235000
This is a extract from the pysvn win32 test-01 output.
(I will test with 1.7.1 once there are kits from David Darj's win32svn).
pysvn is calling svn_client_mkdir and it works
Info: PYSVN CMD mkdir file:///b:/repos/trunk/test -m "test-01 add test"
pysvn is calling svn_client_list and it works
Info: PYSVN CMD ls file:///b:/repos -v -R
2 barry 0 01-Nov-2011 23:57:49 file:///B:/repos/trunk
2 barry 0 01-Nov-2011 23:57:49 file:///B:/repos/trunk/test
Info: Test - checkout
pysvn is calling svn_client_checkout and it fails
Info: PYSVN CMD checkout file:///b:/repos/trunk b:\wc1
svn: E235000: In file 'C:\SVN-1.7.0\src-1.7.0\subversion\libsvn_client\checkout.c' line 94: assertion failed (svn_uri_is_canonical(url, pool))
These same tests pass for SVN 1.6.x.
I'm well aware of the assert for canonical URLs and as you can see
the URL is same shape for mkdir, list and checkout.
Is this a known problem with the svn_client API? Or do I need to dig deeper?
Barry
Re: 1.7.0 assert on svn_client_checkout with E235000
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Neels J Hofmeyr wrote on Fri, Nov 04, 2011 at 12:28:14 +0100:
> A possibility would be to provide additional API functions that are simple
> canonicalization wrappers for the different real API functions -- and that
> would be code bloat.
Or we could switch to C++ and have automatic promotion/coercion from
svn_dirent_t<false> to svn_dirent_t<true>...
Re: 1.7.0 assert on svn_client_checkout with E235000
Posted by Neels J Hofmeyr <ne...@elego.de>.
On 11/04/2011 10:53 AM, Barry Scott wrote:
>
> On 3 Nov 2011, at 11:48, C. Michael Pilato wrote:
>
>> On 11/02/2011 05:42 PM, Barry Scott wrote:
>>> I wish that the canonical stuff was inside the svn_client_XXX API calls and not
>>> a burdon on callers. To my mind the svn.exe API and the svn_client_XXX should
>>> accept the same strings and either operate or return an error. Avoiding the
>>> asserts from using svn_client_XXX is the lions share of the work of getting
>>> svn binding sane.
>>
>> Two problems with such a move:
>>
>> - libsvn_client isn't the only API called from code outside of the
>> Subversion core distribution. What does to auto-canonicalization in
>> libsvn_client do if passing the same uncanonical path to libsvn_wc will then
>> crash?
>
> Then the logic of the suituation would be that lib_svn_wc needed to
> change as well.
>
>>
>> - canonicalizing inside the API means the API must therefore assume that
>> input paths are not canonical. That means performing the same
>> canonicalization over and over again each time the API is called. That's
>> wasteful. Better to ask the highest level of code to make its input conform
>> once (we even offer the functions to do so!) and then take advantage of
>> known-good input from then on.
>
> From my point of view svn_client_XXX is the highest level of code I call.
> I convert to canonical on every call of svn. Its saves nothing.
It probably saves many duplicate *_canonicalize() calls that the API would
cause. You can write a canonicalization wrapper for coding convenience, like
you did, or you can choose to handle canonicalization even earlier.
> I think it makes using the svn API unnecessarily complex.
Actually, the same argument goes for inside the API: assuming
non-canonicalized paths makes API development more complex. IMO we need to
expect valid data from API callers, for paths as for other args.
A possibility would be to provide additional API functions that are simple
canonicalization wrappers for the different real API functions -- and that
would be code bloat.
~Neels
Re: 1.7.0 assert on svn_client_checkout with E235000
Posted by Barry Scott <ba...@barrys-emacs.org>.
On 3 Nov 2011, at 11:48, C. Michael Pilato wrote:
> On 11/02/2011 05:42 PM, Barry Scott wrote:
>> I wish that the canonical stuff was inside the svn_client_XXX API calls and not
>> a burdon on callers. To my mind the svn.exe API and the svn_client_XXX should
>> accept the same strings and either operate or return an error. Avoiding the
>> asserts from using svn_client_XXX is the lions share of the work of getting
>> svn binding sane.
>
> Two problems with such a move:
>
> - libsvn_client isn't the only API called from code outside of the
> Subversion core distribution. What does to auto-canonicalization in
> libsvn_client do if passing the same uncanonical path to libsvn_wc will then
> crash?
Then the logic of the suituation would be that lib_svn_wc needed to
change as well.
>
> - canonicalizing inside the API means the API must therefore assume that
> input paths are not canonical. That means performing the same
> canonicalization over and over again each time the API is called. That's
> wasteful. Better to ask the highest level of code to make its input conform
> once (we even offer the functions to do so!) and then take advantage of
> known-good input from then on.
From my point of view svn_client_XXX is the highest level of code I call.
I convert to canonical on every call of svn. Its saves nothing.
I think it makes using the svn API unnecessarily complex.
I'm under no illusions that this is likely to change, but I wanted to
bring this issue to your attention.
Barry
Re: 1.7.0 assert on svn_client_checkout with E235000
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/02/2011 05:42 PM, Barry Scott wrote:
> I wish that the canonical stuff was inside the svn_client_XXX API calls and not
> a burdon on callers. To my mind the svn.exe API and the svn_client_XXX should
> accept the same strings and either operate or return an error. Avoiding the
> asserts from using svn_client_XXX is the lions share of the work of getting
> svn binding sane.
Two problems with such a move:
- libsvn_client isn't the only API called from code outside of the
Subversion core distribution. What does to auto-canonicalization in
libsvn_client do if passing the same uncanonical path to libsvn_wc will then
crash?
- canonicalizing inside the API means the API must therefore assume that
input paths are not canonical. That means performing the same
canonicalization over and over again each time the API is called. That's
wasteful. Better to ask the highest level of code to make its input conform
once (we even offer the functions to do so!) and then take advantage of
known-good input from then on.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: 1.7.0 assert on svn_client_checkout with E235000
Posted by Barry Scott <ba...@barrys-emacs.org>.
On 2 Nov 2011, at 09:47, Bert Huijben wrote:
>
>
>> -----Original Message-----
>> From: Barry Scott [mailto:barry@barrys-emacs.org]
>> Sent: woensdag 2 november 2011 10:19
>> To: Bert Huijben
>> Cc: 'Subversion Development'
>> Subject: Re: 1.7.0 assert on svn_client_checkout with E235000
>>
>> On 2 Nov 2011, at 06:47, Bert Huijben wrote:
>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Barry Scott [mailto:barry@barrys-emacs.org]
>>>> Sent: woensdag 2 november 2011 1:12
>>>> To: Subversion Development
>>>> Subject: 1.7.0 assert on svn_client_checkout with E235000
>>>>
>>>> This is a extract from the pysvn win32 test-01 output.
>>>> (I will test with 1.7.1 once there are kits from David Darj's
> win32svn).
>>>>
>>>> pysvn is calling svn_client_mkdir and it works
>>>> Info: PYSVN CMD mkdir file:///b:/repos/trunk/test -m "test-01 add
>>>> test"
>>>>
>>>> pysvn is calling svn_client_list and it works
>>>> Info: PYSVN CMD ls file:///b:/repos -v -R
>>>> 2 barry 0 01-Nov-2011 23:57:49
>>> file:///B:/repos/trunk
>>>> 2 barry 0 01-Nov-2011 23:57:49
>>> file:///B:/repos/trunk/test
>>>> Info: Test - checkout
>>>> pysvn is calling svn_client_checkout and it fails
>>>> Info: PYSVN CMD checkout file:///b:/repos/trunk b:\wc1
>>>> svn: E235000: In file 'C:\SVN-1.7.0\src-
>>>> 1.7.0\subversion\libsvn_client\checkout.c' line 94: assertion failed
>>>> (svn_uri_is_canonical(url, pool))
>>>>
>>>> These same tests pass for SVN 1.6.x.
>>>>
>>>> I'm well aware of the assert for canonical URLs and as you can see
>>>> the URL is same shape for mkdir, list and checkout.
>>>>
>>>> Is this a known problem with the svn_client API? Or do I need to dig
>>> deeper?
>>>
>>> The client library expects canonical path arguments and some of these
>> paths
>>> are no longer canonical since we added better validations.
>>>
>>> The canonical form of file:///b:/repos is file:///B:/repos
>>> And the canonical form of b:/wc1 is B:/wc1
>>
>> I'm surprised. I though win32 API calls did not care about the case.
>
> The Win32 APIs don't care but our own APIs, including our storage in and
> around use of wc.db do care about casing.
>
> Subversion always canonicalized paths to its internal format, but because
> Subversion < 1.6 only cared about relative paths we didn't check those and
> just assumed our caller passed the right form. In most cases users just
> passed always lowercase or always uppercase and that worked correctly. But
> when developing 1.7 we found issues when mixing.
>
>>> Either you (as caller of pysvn) or pysvn should canonicalize paths
> before
>>> passing them to the Subversion api.
>>
>> I already do use SVN APIs.
>>
>>>
>>> Personally I would say that pysvn shoulddo that for you, but that is up
> for
>>> debate.
>>
>> I use this code to get canonical paths or URLs. As you can see it uses SVN
>> functions to keep things legal.
>>
>> std::string svnNormalisedIfPath( const std::string &unnormalised, SvnPool
>> &pool )
>> {
>> if( is_svn_url( unnormalised ) )
>> {
>> const char *normalised_path = svn_path_canonicalize(
>> unnormalised.c_str(), pool );
>> return std::string( normalised_path );
>
> For 1.7+ I would recommend using svn_uri_canonicalize.
> (svn_path_canonicalize does a similar check and then calls either the dirent
> or the uri variant)
svn_path_canonicalize is deprecated...
I've avoided the deprecated APIs and now use the 1.7 APIs for canonical.
>> }
>> else
>> {
>> const char *normalised_path = svn_path_internal_style(
>> unnormalised.c_str(), pool );
>
> svn_path_internal_style is deprecated. Local paths now use svn_dirent_*
> functions.
>> return std::string( normalised_path );
>> }
>> }
>
> I'm surprised that this function doesn't work for you though...
> It svn_path_canonicalize should generate canonical uris and dirents. (It may
> fail for some relpaths though)
I found that I missed calling canonical for the URL arg to svn_client_checkin.
Now My tests do not crash.
>>
>> bool is_svn_url( const std::string &path_or_url )
>> {
>> return svn_path_is_url( path_or_url.c_str() ) != 0;
>> }
>>
>> Why is this not good enough for svn_client_commit4 but it is good enough
>> for svn_client_mkdir3 and svn_client_ls2?
>
> Some functions recanonicalize paths internally for other reasons or just
> don't check what they get. All Subversion functions except a few are
> documented to require canonical paths.
>
> Commit is very sensitive about canonicalization as it has to build its own
> commit tree that doesn't have to be similar as the filesystem tree. I'm not
> surprised that it performs more checking.
I wish that the canonical stuff was inside the svn_client_XXX API calls and not
a burdon on callers. To my mind the svn.exe API and the svn_client_XXX should
accept the same strings and either operate or return an error. Avoiding the
asserts from using svn_client_XXX is the lions share of the work of getting
svn binding sane.
The whole b: vs. B: should never be exposed to API callers in my opinion.
Barry
RE: 1.7.0 assert on svn_client_checkout with E235000
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Barry Scott [mailto:barry@barrys-emacs.org]
> Sent: woensdag 2 november 2011 10:19
> To: Bert Huijben
> Cc: 'Subversion Development'
> Subject: Re: 1.7.0 assert on svn_client_checkout with E235000
>
> On 2 Nov 2011, at 06:47, Bert Huijben wrote:
>
> >
> >
> >> -----Original Message-----
> >> From: Barry Scott [mailto:barry@barrys-emacs.org]
> >> Sent: woensdag 2 november 2011 1:12
> >> To: Subversion Development
> >> Subject: 1.7.0 assert on svn_client_checkout with E235000
> >>
> >> This is a extract from the pysvn win32 test-01 output.
> >> (I will test with 1.7.1 once there are kits from David Darj's
win32svn).
> >>
> >> pysvn is calling svn_client_mkdir and it works
> >> Info: PYSVN CMD mkdir file:///b:/repos/trunk/test -m "test-01 add
> >> test"
> >>
> >> pysvn is calling svn_client_list and it works
> >> Info: PYSVN CMD ls file:///b:/repos -v -R
> >> 2 barry 0 01-Nov-2011 23:57:49
> > file:///B:/repos/trunk
> >> 2 barry 0 01-Nov-2011 23:57:49
> > file:///B:/repos/trunk/test
> >> Info: Test - checkout
> >> pysvn is calling svn_client_checkout and it fails
> >> Info: PYSVN CMD checkout file:///b:/repos/trunk b:\wc1
> >> svn: E235000: In file 'C:\SVN-1.7.0\src-
> >> 1.7.0\subversion\libsvn_client\checkout.c' line 94: assertion failed
> >> (svn_uri_is_canonical(url, pool))
> >>
> >> These same tests pass for SVN 1.6.x.
> >>
> >> I'm well aware of the assert for canonical URLs and as you can see
> >> the URL is same shape for mkdir, list and checkout.
> >>
> >> Is this a known problem with the svn_client API? Or do I need to dig
> > deeper?
> >
> > The client library expects canonical path arguments and some of these
> paths
> > are no longer canonical since we added better validations.
> >
> > The canonical form of file:///b:/repos is file:///B:/repos
> > And the canonical form of b:/wc1 is B:/wc1
>
> I'm surprised. I though win32 API calls did not care about the case.
The Win32 APIs don't care but our own APIs, including our storage in and
around use of wc.db do care about casing.
Subversion always canonicalized paths to its internal format, but because
Subversion < 1.6 only cared about relative paths we didn't check those and
just assumed our caller passed the right form. In most cases users just
passed always lowercase or always uppercase and that worked correctly. But
when developing 1.7 we found issues when mixing.
> > Either you (as caller of pysvn) or pysvn should canonicalize paths
before
> > passing them to the Subversion api.
>
> I already do use SVN APIs.
>
> >
> > Personally I would say that pysvn shoulddo that for you, but that is up
for
> > debate.
>
> I use this code to get canonical paths or URLs. As you can see it uses SVN
> functions to keep things legal.
>
> std::string svnNormalisedIfPath( const std::string &unnormalised, SvnPool
> &pool )
> {
> if( is_svn_url( unnormalised ) )
> {
> const char *normalised_path = svn_path_canonicalize(
> unnormalised.c_str(), pool );
> return std::string( normalised_path );
For 1.7+ I would recommend using svn_uri_canonicalize.
(svn_path_canonicalize does a similar check and then calls either the dirent
or the uri variant)
> }
> else
> {
> const char *normalised_path = svn_path_internal_style(
> unnormalised.c_str(), pool );
svn_path_internal_style is deprecated. Local paths now use svn_dirent_*
functions.
> return std::string( normalised_path );
> }
> }
I'm surprised that this function doesn't work for you though...
It svn_path_canonicalize should generate canonical uris and dirents. (It may
fail for some relpaths though)
>
> bool is_svn_url( const std::string &path_or_url )
> {
> return svn_path_is_url( path_or_url.c_str() ) != 0;
> }
>
> Why is this not good enough for svn_client_commit4 but it is good enough
> for svn_client_mkdir3 and svn_client_ls2?
Some functions recanonicalize paths internally for other reasons or just
don't check what they get. All Subversion functions except a few are
documented to require canonical paths.
Commit is very sensitive about canonicalization as it has to build its own
commit tree that doesn't have to be similar as the filesystem tree. I'm not
surprised that it performs more checking.
Bert
>
> Barry
Re: 1.7.0 assert on svn_client_checkout with E235000
Posted by Barry Scott <ba...@barrys-emacs.org>.
On 2 Nov 2011, at 06:47, Bert Huijben wrote:
>
>
>> -----Original Message-----
>> From: Barry Scott [mailto:barry@barrys-emacs.org]
>> Sent: woensdag 2 november 2011 1:12
>> To: Subversion Development
>> Subject: 1.7.0 assert on svn_client_checkout with E235000
>>
>> This is a extract from the pysvn win32 test-01 output.
>> (I will test with 1.7.1 once there are kits from David Darj's win32svn).
>>
>> pysvn is calling svn_client_mkdir and it works
>> Info: PYSVN CMD mkdir file:///b:/repos/trunk/test -m "test-01 add
>> test"
>>
>> pysvn is calling svn_client_list and it works
>> Info: PYSVN CMD ls file:///b:/repos -v -R
>> 2 barry 0 01-Nov-2011 23:57:49
> file:///B:/repos/trunk
>> 2 barry 0 01-Nov-2011 23:57:49
> file:///B:/repos/trunk/test
>> Info: Test - checkout
>> pysvn is calling svn_client_checkout and it fails
>> Info: PYSVN CMD checkout file:///b:/repos/trunk b:\wc1
>> svn: E235000: In file 'C:\SVN-1.7.0\src-
>> 1.7.0\subversion\libsvn_client\checkout.c' line 94: assertion failed
>> (svn_uri_is_canonical(url, pool))
>>
>> These same tests pass for SVN 1.6.x.
>>
>> I'm well aware of the assert for canonical URLs and as you can see
>> the URL is same shape for mkdir, list and checkout.
>>
>> Is this a known problem with the svn_client API? Or do I need to dig
> deeper?
>
> The client library expects canonical path arguments and some of these paths
> are no longer canonical since we added better validations.
>
> The canonical form of file:///b:/repos is file:///B:/repos
> And the canonical form of b:/wc1 is B:/wc1
I'm surprised. I though win32 API calls did not care about the case.
>
> Either you (as caller of pysvn) or pysvn should canonicalize paths before
> passing them to the Subversion api.
I already do use SVN APIs.
>
> Personally I would say that pysvn shoulddo that for you, but that is up for
> debate.
I use this code to get canonical paths or URLs. As you can see it uses SVN
functions to keep things legal.
std::string svnNormalisedIfPath( const std::string &unnormalised, SvnPool &pool )
{
if( is_svn_url( unnormalised ) )
{
const char *normalised_path = svn_path_canonicalize( unnormalised.c_str(), pool );
return std::string( normalised_path );
}
else
{
const char *normalised_path = svn_path_internal_style( unnormalised.c_str(), pool );
return std::string( normalised_path );
}
}
bool is_svn_url( const std::string &path_or_url )
{
return svn_path_is_url( path_or_url.c_str() ) != 0;
}
Why is this not good enough for svn_client_commit4 but it is good enough
for svn_client_mkdir3 and svn_client_ls2?
Barry
RE: 1.7.0 assert on svn_client_checkout with E235000
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Barry Scott [mailto:barry@barrys-emacs.org]
> Sent: woensdag 2 november 2011 1:12
> To: Subversion Development
> Subject: 1.7.0 assert on svn_client_checkout with E235000
>
> This is a extract from the pysvn win32 test-01 output.
> (I will test with 1.7.1 once there are kits from David Darj's win32svn).
>
> pysvn is calling svn_client_mkdir and it works
> Info: PYSVN CMD mkdir file:///b:/repos/trunk/test -m "test-01 add
> test"
>
> pysvn is calling svn_client_list and it works
> Info: PYSVN CMD ls file:///b:/repos -v -R
> 2 barry 0 01-Nov-2011 23:57:49
file:///B:/repos/trunk
> 2 barry 0 01-Nov-2011 23:57:49
file:///B:/repos/trunk/test
> Info: Test - checkout
> pysvn is calling svn_client_checkout and it fails
> Info: PYSVN CMD checkout file:///b:/repos/trunk b:\wc1
> svn: E235000: In file 'C:\SVN-1.7.0\src-
> 1.7.0\subversion\libsvn_client\checkout.c' line 94: assertion failed
> (svn_uri_is_canonical(url, pool))
>
> These same tests pass for SVN 1.6.x.
>
> I'm well aware of the assert for canonical URLs and as you can see
> the URL is same shape for mkdir, list and checkout.
>
> Is this a known problem with the svn_client API? Or do I need to dig
deeper?
The client library expects canonical path arguments and some of these paths
are no longer canonical since we added better validations.
The canonical form of file:///b:/repos is file:///B:/repos
And the canonical form of b:/wc1 is B:/wc1
Either you (as caller of pysvn) or pysvn should canonicalize paths before
passing them to the Subversion api.
Personally I would say that pysvn shoulddo that for you, but that is up for
debate.
Bert
>
> Barry