[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Unifying and fixing the error handling for ra_local and svnserve

From: Tobias Ringström <tobias_at_ringstrom.mine.nu>
Date: 2003-08-23 01:45:06 CEST

The error message you get from svnserve when it fails to open a
repository is not very informative to say the least. For an example see
Guido van Rossum's email to the users list from a couple of days ago:

http://www.contactor.se/~dast/svnusers/archive-2003-08/0528.shtml
> But no matter what I try, I get the following error messages:
>
> svn: Couldn't find a repository
> svn: No repository found in 'svn+ssh://<host>/home/subversion/<repos>'

It later turned out to be a permission/umask problem. With the enclosed
patch, the output looks like this instead:

svn: Couldn't find a repository
svn: No repository found in 'svn://localhost/home/svn/trunk'
svn: Berkeley DB error
svn: Berkeley DB error while opening environment for filesystem
/home/svn/db:
Permission denied

[This particular error message could be improved, but that's another issue]

The culprit was the code in svnserve which tries to find the root of a
repository given an URL. The code in ra_local has the same problem, but
has a better implementation.

This patch creates a new function in svn_repos.h to find the root of a
repository given a path (e.g. it returns /home/svn if it is given
/home/svn/trunk/README). The new function removes components from the
end of the path until a valid repository is found (or there is nothing
left). A directory containing a file "format" and a directory "db" is
considered a subversion repository. This means that the function can
return false positives, but if it does, svn_repos_open will fail shortly
thereafter.

The new error handling does not special-case any errors, but sends all
errors upwards. This means that the user will actually see errors such
as wrong DB format version for both ra_local and ra_svn/svnserve. Those
errors were hidden before.

The new code passes "make check" without failures. I do not know if make
check ran any svnserve tests, but I tried to test it manually as well of
course.

/Tobias

------------- Changelog ---------------
The methods used by ra_local and svnserve to find the repository of an
URL were different. This was causing different behaviour in case of
errors. Fix the error handling to make sure that the error codes are
propagated to the user.

* subversion/include/svn_repos.h
   (svn_repos_find_root_path): New

* libsvn_repos/repos.c
   (check_repos_path): New
   (svn_repos_find_root_path): New

* libsvn_ra_local/split_url.c
   (svn_ra_local__split_URL): Use the new svn_repos_find_root_path
   function instead of looping and calling svn_repos_open. Renamed
   candidate_url to candidate since it is not an URL. Propagate the
   error from svn_repos_open if it fails so the user can get a clue.

* svnserve/serve.c
   (find_repos): Use the new svn_repos_find_root_path function instead
   of looping and calling svn_repos_open. Propagate the error from
   svn_repos_open if it fails so the user can get a clue.

Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 6838)
+++ subversion/include/svn_repos.h (working copy)
@@ -44,6 +44,16 @@
 /* Opening and creating repositories. */
 
 
+/** Find the root path of the repository that contains @a path by
+ * removing a component at a time from the end of @a path until a
+ * likely repository root path is found.
+ *
+ * If a repository was found, the path to the root of the repository
+ * is returned, else @c NULL.
+ */
+char *svn_repos_find_root_path (const char *path,
+ apr_pool_t *pool);
+
 /** Set @a *repos_p to a repository object for the repository at @a path.
  *
  * Allocate @a *repos_p in @a pool.
Index: subversion/libsvn_ra_local/split_url.c
===================================================================
--- subversion/libsvn_ra_local/split_url.c (revision 6838)
+++ subversion/libsvn_ra_local/split_url.c (working copy)
@@ -30,7 +30,7 @@
                          apr_pool_t *pool)
 {
   svn_error_t *err = SVN_NO_ERROR;
- const char *candidate_url;
+ const char *candidate;
   const char *hostname, *path;
 
   /* Decode the URL, as we only use its parts as filesystem paths
@@ -69,7 +69,7 @@
 
   /* Duplicate the URL, starting at the top of the path */
 #ifndef SVN_WIN32
- candidate_url = apr_pstrdup (pool, path);
+ candidate = apr_pstrdup (pool, path);
 #else /* SVN_WIN32 */
   /* On Windows, we'll typically have to skip the leading / if the
      path starts with a drive letter. Like most Web browsers, We
@@ -92,72 +92,34 @@
        char *const dup_path = apr_pstrdup (pool, ++path);
        if (dup_path[1] == '|')
          dup_path[1] = ':';
- candidate_url = dup_path;
+ candidate = dup_path;
      }
    else
- candidate_url = apr_pstrdup (pool, path);
+ candidate = apr_pstrdup (pool, path);
  }
 #endif /* SVN_WIN32 */
 
- /* Loop, trying to open a repository at URL. If this fails, remove
- the last component from the URL, then try again. */
- while (1)
+ /* Search for a repository in the full path. */
+ candidate = svn_repos_find_root_path(candidate, pool);
+ if (!candidate)
     {
- /* Attempt to open a repository at URL. */
- err = svn_repos_open (repos, candidate_url, pool);
+ return svn_error_createf
+ (SVN_ERR_RA_LOCAL_REPOS_OPEN_FAILED, NULL,
+ "Unable to open repository '%s'", URL);
+ }
 
- /* Hey, cool, we were successful. Stop looping. */
- if (err == SVN_NO_ERROR)
- break;
-
- /* If we get an error -other- than the path or 'format' file not
- existing, then throw the error immediately. For example, we
- want permissions errors to be seen right away. */
- if ((! APR_STATUS_IS_ENOENT(err->apr_err))
- && (err->apr_err != SVN_ERR_REPOS_UNSUPPORTED_VERSION))
+ /* Attempt to open a repository at URL. */
+ err = svn_repos_open (repos, candidate, pool);
+ if (err)
+ {
         return svn_error_createf
           (SVN_ERR_RA_LOCAL_REPOS_OPEN_FAILED, err,
            "Unable to open repository '%s'", URL);
-
- /* It would be strange indeed if "/" were a repository, but hey,
- people do strange things sometimes. Anyway, if "/" failed
- the test above, then reduce it to the empty string.
-
- ### I'm not sure whether SVN_EMPTY_PATH and SVN_PATH_IS_EMPTY
- in libsvn_subr/path.c is are supposed to be changeable. If
- they are, this code could conceivably break. If they're not,
- then we should probably make SVN_PATH_EMPTY_PATH a public
- constant and use it here. But either way, note that the test
- for '/' below is kosher, because we know it's the canonical
- APR separator. */
- if ((candidate_url[0] == '/') && (candidate_url[1] == '\0'))
- candidate_url = "";
-
- /* If we're down to an empty path here, and we still haven't
- found the repository, we're just out of luck. Time to bail
- and face the music. */
- if (svn_path_is_empty (candidate_url))
- break;
-
- /* We didn't successfully open the repository, and we haven't
- hacked this path down to a bare nub yet, so we'll chop off
- the last component of this path. */
- candidate_url = svn_path_dirname (candidate_url, pool);
- if (err)
- svn_error_clear (err);
     }
 
- /* If we are still sitting in an error-ful state, we must not have
- found the repository. We give up. */
- if (err)
- return svn_error_createf
- (SVN_ERR_RA_LOCAL_REPOS_NOT_FOUND, NULL,
- "svn_ra_local__split_URL: Unable to find valid repository\n"
- " (%s)", URL);
-
   /* What remains of URL after being hacked at in the previous step is
      REPOS_URL. FS_PATH is what we've hacked off in the process. */
- *fs_path = apr_pstrdup (pool, path + strlen (candidate_url));
+ *fs_path = apr_pstrdup (pool, path + strlen (candidate));
   *repos_url = apr_pstrmemdup (pool, URL, strlen(URL) - strlen(*fs_path));
 
   return SVN_NO_ERROR;
Index: subversion/libsvn_repos/repos.c
===================================================================
--- subversion/libsvn_repos/repos.c (revision 6838)
+++ subversion/libsvn_repos/repos.c (working copy)
@@ -1043,6 +1043,39 @@
 }
 
 
+/* Check if @a path is the root of a repository by checking if the
+ * path contains the expected files and directories. */
+static svn_boolean_t
+check_repos_path (const char *path,
+ apr_pool_t *pool)
+{
+ svn_node_kind_t kind;
+ svn_error_t *err;
+
+ err = svn_io_check_path (svn_path_join (path, SVN_REPOS__FORMAT, pool),
+ &kind, pool);
+ if (err)
+ {
+ svn_error_clear (err);
+ return TRUE;
+ }
+ if (kind != svn_node_file)
+ return FALSE;
+
+ err = svn_io_check_path (svn_path_join (path, SVN_REPOS__DB_DIR, pool),
+ &kind, pool);
+ if (err)
+ {
+ svn_error_clear (err);
+ return TRUE;
+ }
+ if (kind != svn_node_dir)
+ return FALSE;
+
+ return TRUE;
+}
+
+
 /* Verify that the repository's 'format' file is a suitable version. */
 static svn_error_t *
 check_repos_version (const char *path,
@@ -1155,6 +1188,25 @@
 
 
 
+char *
+svn_repos_find_root_path (const char *path,
+ apr_pool_t *pool)
+{
+ char *candidate = apr_pstrdup (pool, path);
+
+ while (1)
+ {
+ if (check_repos_path (candidate, pool))
+ break;
+ if (candidate[0] == '\0' || strcmp(candidate, "/") == 0)
+ return NULL;
+ candidate = svn_path_dirname (candidate, pool);
+ }
+
+ return candidate;
+}
+
+
 svn_error_t *
 svn_repos_open (svn_repos_t **repos_p,
                 const char *path,
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 6838)
+++ subversion/svnserve/serve.c (working copy)
@@ -877,18 +877,19 @@
   full_path = svn_path_canonicalize(full_path, pool);
 
   /* Search for a repository in the full path. */
- candidate = full_path;
- while (1)
+ candidate = svn_repos_find_root_path(full_path, pool);
+ if (!candidate)
     {
- err = svn_repos_open(repos, candidate, pool);
- if (err == SVN_NO_ERROR)
- break;
- svn_error_clear(err);
- if (!*candidate || strcmp(candidate, "/") == 0)
- return svn_error_createf(SVN_ERR_RA_SVN_REPOS_NOT_FOUND, NULL,
- "No repository found in '%s'", url);
- candidate = svn_path_dirname(candidate, pool);
+ return svn_error_createf(SVN_ERR_RA_SVN_REPOS_NOT_FOUND, NULL,
+ "No repository found in '%s'", url);
     }
+
+ err = svn_repos_open(repos, candidate, pool);
+ if (err)
+ {
+ return svn_error_createf(SVN_ERR_RA_SVN_REPOS_NOT_FOUND, err,
+ "No repository found in '%s'", url);
+ }
   *fs_path = apr_pstrdup(pool, full_path + strlen(candidate));
   *repos_url = apr_pstrmemdup(pool, url, strlen(url) - strlen(*fs_path));
   return SVN_NO_ERROR;

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Aug 23 01:47:12 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.