Re: 1.7.0 assert on svn_client_checkout with E235000

On 2 Nov 2011, at 09:47, Bert Huijben wrote:

>
>
>> -----Original Message-----
>> From: Barry Scott [mailto:barry_at_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_at_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
Received on 2011-11-02 22:43:08 CET