Comments inline...
Garrett Rooney <rooneg@electricjellyfish.net> writes:
> So I was experimenting with managing a repository's hooks directory in
> the repository, via the following technique:
[trimming recipe]
> $ svnadmin create /tmp/repos
> $ svn import /tmp/repos/hooks svn://localhost/tmp/repos/hooks -m ""
> $ cd /tmp/repos
> $ svn co svn://localhost/tmp/repos/hooks temp
> $ rm -rf hooks
> $ mv temp hooks
> $ cd hooks
> $ svn log pre-commit.tmpl
> subversion/svnserve/serve.c:888: (apr_err=210005)
> svn: Couldn't find a repository
> svn: No repository found in
> 'svn://localhost/tmp/repos/hooks/pre-commit.tmpl'
> subversion/libsvn_repos/repos.c:996: (apr_err=165005)
> svn: Unsupported repository version
> svn: Expected version '2' of repository; found no version at all; is
> `/tmp/repos/hooks/pre-commit.tmpl' a valid repository path?
> subversion/libsvn_subr/io.c:1435: (apr_err=20)
> svn: Not a directory
> svn: svn_io_file_open: can't open `/tmp/repos/hooks/pre-commit.tmpl/format'
> $
Ah! It finally hit me! I too was perplexed by the failure you were
seeing with 'svn log', but I then I remembered something --
svn_ra_local__split_URL().
Hm. I see that this function is now basically a wrapper around
svn_repos_find_root_path(). No matter, the general algorithmic idea
remains. Basically, we do this:
1. Set fs_path = ""; set repos_path = the "path" portion of an input URL.
(for your 'svn log' run, this will be /tmp/repos/hooks/pre-commit.tmpl)
2. Try to open the repository at repos_path. If that succeeds,
return the repos_path and fs_path; if it fails, move the split
point by a single path component (so fs_path becomes
"pre-commit.tmpl" and repos_path becomes "/tmp/repos/hooks").
3. Repeat until you open a repos or run out of path components.
So, I was thinking that it was this algorithm which was failing you.
And I am technically right, though not for the reasons I would have
guessed.
Digging down, I saw that because /tmp/repos/hooks/pre-commit.tmpl is
a real on-disk path which is not a directory, our test for whether or
not it is a repository (by looking for certain subdirs of the test
path) is failing. That test is counting on svn_io_check_path()
returning no errors, but an svn_node_none kind, for non-existent
paths. But because the APR error returned from trying to stat
/tmp/repos/hooks/pre-commit.tmpl/format is ENOTDIR instead of ENOENT,
svn_io_check_path() instead returns an error.
I'm not sure if we want to make svn_io_check_path() also treat ENOTDIR
as a non-error (which yeilds an 'svn_node_none' answer). But if we
don't do that, check_repos_path() will need to explicitly handle that
error.
> Checking out the hooks directory into a location outside the
> repository allows the log to work, and running log against the
> repository directly works as normal. This is reproducable via ra_svn
> and ra_local, haven't tried ra_dav.
Actually, checkout out hooks/ outside the repository didn't work for
me -- I mean, the algorithm is exactly the same. Are you sure this
worked for you, or did you taint that test by, say, mv'ing the hooks
directory out of the repos, or something?
Here's a quick reproduction recipe for those that want to see this bug
in action:
#!/bin/sh
REPO=`pwd`/rooneg-repo
WC=`pwd`/rooneg-wc
rm -rf ${REPO} ${WC}
svnadmin create ${REPO}
svn import rooneg-repo/hooks file://${REPO}/hooks -m ''
cd rooneg-repo
svn co file://${REPO}/hooks temp
svn co file://${REPO}/hooks ${WC}
rm -rf hooks
mv temp hooks
cd hooks
echo "*** Running log on the in-place hooks ***"
svn log pre-commit.tmpl
echo "*** Running log on the out-of-place hooks ***"
svn log ${WC}/pre-commit.tmpl
I'd like to hear what folks think about changed svn_io_check_path() to
ignore ENOTDIR and just return 'svn_node_none'. I think it would be a
fine change -- I can't think of any place in our code that uses this
function and needs to know the difference between "there's no such
path" and "there's no such path because something along the way isn't
a directory".
Here's an untested patch to start the process if we agree to change
svn_io_check_path() (well, I mean, it makes this 'svn log' bug go
away, but I didn't run the whole of 'make check' with it):
* subversion/libsvn_subr/io.c
(io_check_path): Treat APR's ENOTDIR status the same as ENOENT --
both are non-fatal forms of the answer "that path doesn't exist."
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 7133)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -68,12 +68,16 @@
flags = resolve_symlinks ? APR_FINFO_MIN : (APR_FINFO_MIN | APR_FINFO_LINK);
apr_err = apr_stat (&finfo, path_apr, flags, pool);
- if (apr_err && !APR_STATUS_IS_ENOENT(apr_err))
+ if (apr_err
+ && !APR_STATUS_IS_ENOTDIR(apr_err)
+ && !APR_STATUS_IS_ENOENT(apr_err))
return svn_error_createf
(apr_err, NULL,
"check_path: problem checking path \"%s\"", path);
else if (APR_STATUS_IS_ENOENT(apr_err))
*kind = svn_node_none;
+ else if (APR_STATUS_IS_ENOTDIR(apr_err))
+ *kind = svn_node_none;
else if (finfo.filetype == APR_NOFILE)
*kind = svn_node_unknown;
else if (finfo.filetype == APR_REG)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 23 01:22:49 2003