On Fri, Apr 23, 2010 at 11:58, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Uwe Stuehler wrote:
>> [[[
>> Fix issue #3620: svn add ^/ triggers assertion failure
>>
>> * subversion/svn/add-cmd.c:
>> (svn_cl__add): Raise an error if a target argument isn't a working
>> copy path before calling svn_client_add4(), which would trigger an
>> assertion.
>>
>> Found by: stsp
>> Patch by: Uwe Stuehler (subversion-lists_at_bsdx.de)
>> ]]]
>
> Uwe,
>
> Thanks again for your patch. I took some time to look at it just now. The
> patch, as patches go, is fine. But the approach isn't the best one. Here
> are the notes I just recorded in issue #3620:
>
> ----------------------------------------------------------------------------
> 'svn st ^/' suffers from the same thing.
>
> Subversion 1.6 handles these cases gracefully. Note the use of the
> svn_cl__try() wrapper arounds the client's calls to svn_client_status5() and
> svn_client_add4() -- it clearly is expecting those APIs to return a
> particular error code when asked to operate on paths that aren't working
> copies. The fix for this issue needs to preserve those behaviors (which
> means it must live behind the libsvn_client API).
>
> I'm not confident, but I suspect that the root cause is the use of
> svn_dirent_get_absolute() all over the place for WC-NG-ness. That function
> asserts (ultimately) that its input isn't a URL. We need to either make it
> return a graceful error in those conditions, or do this kind of checking in
> its callers.
> ----------------------------------------------------------------------------
>
> We need to figure out the right approach before moving on.
I don't think this is rocket science. My comment in the issue:
----------------------------------------------------------------------------
I think that svn_client_add4 should check if a URL is passed and
return an error. Simple as that. Then we
can proceed with the svn_dirent_get_absolute call.
I don't really see the point about prior error returns and svn_cl__try
and stuff. It is clear that a bad input
was provided to svn_client_add4, and it should return an error stating such.
----------------------------------------------------------------------------
Basically, put Uwe's patch right into svn_client_add4, rather than in
the cmdline tool.
Cheers,
-g
Received on 2010-04-23 18:40:14 CEST