On Fri, Apr 23, 2010 at 11:58, C. Michael Pilato <cmpil...@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-li...@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
