> -----Original Message----- > From: 'Stefan Sperling' [mailto:s...@elego.de] > Sent: zondag 1 augustus 2010 23:09 > To: Bert Huijben > Cc: dev@subversion.apache.org > Subject: Re: svn commit: r980811 - in > /subversion/trunk/subversion/libsvn_fs_fs: fs.h fs_fs.c > > On Sun, Aug 01, 2010 at 10:56:47PM +0200, Bert Huijben wrote: > > Looking a bit further: Do we really need to take the absolute path > for the > > error message? (What do we in similar code paths?) > > I would like Subversion to print helpful error message. > Though the path need not be absolute, but see below. > > > The easiest way to fix this would be to just use the caller provided > paths > > in the error message. That would immediately fix the error leak. > > The caller-provided paths contain a trailing '/db', and running upgrade > with those doesn't work: > > $ svnadmin upgrade repos/db > subversion/svnadmin/main.c:1543: (apr_err=2) > subversion/libsvn_repos/repos.c:1572: (apr_err=2) > subversion/libsvn_repos/repos.c:1502: (apr_err=2) > subversion/libsvn_repos/repos.c:1338: (apr_err=2) > subversion/libsvn_repos/repos.c:1338: (apr_err=2) > svnadmin: Error opening db lockfile > subversion/libsvn_subr/io.c:1625: (apr_err=2) > subversion/libsvn_subr/io.c:2714: (apr_err=2) > svnadmin: Can't open file 'repos/db/locks/db.lock': No such file or > directory > > So we cannot advise users to run the command with 'db' at the end. > We could use the relative paths to avoid further SVN_ERR() calls, sure. > But you've also said that you didn't want the dirname() hack to strip > off the trailing 'db'.
I'm not against doing this.. I just think we should fix the other messages too, if we decide this is the right thing to do. I'm not aware of direct users of our fs layer, that would have an issue with this fix... So please stand up if you care!!! (And in case nobody stands up we should fix those other errors). > So I'm not sure how you'd like to make Subversion provide a useful > error message for this case. > > > In this case you would need to use src_path for the error message > anyway if > > the svn_dirent_get_absolute() fails. > > That's very likely not gonna fail. We've already opened the repository. > > Stefan