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'.

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
Received on 2010-08-01 23:09:56 CEST