Greg Stein wrote: > 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.
As I noted, the check needs to happen inside the client API. The question becomes "Do we patch svn_client_addX() and svn_client_statusX() and svn_client_updateX() and ..." or is there a more general fix to be made (like perhaps a wrapper around svn_dirent_get_absolute() that gracefully handles the my-caller-passed-a-url case)? -- C. Michael Pilato <cmpil...@collab.net> CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
