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

[RFC] Make mod_dav_svn + mod_authz_svn load-order-independent

From: Peter Samuelson <peter_at_p12n.org>
Date: Sat, 19 Nov 2011 23:45:57 -0600

Currently if you load mod_authz_svn, you have to make sure mod_dav_svn
is loaded before it, not after. I'm sure this is usually not a problem
- but in Debian/Ubuntu, the Apache module machinery enables modules by
use of tiny config files that each load one module. The filenames
determine load order, so a config file named 'authz_svn.load' is read
before one named 'dav_svn.load'.

In the past we got around this by shipping a single 'dav_svn.load' that
loads _both_ modules, but I never liked that solution, as most people
don't need mod_authz_svn at all.

Thus, appended is a patch to mod_authz_svn to pull in the two dav_svn
functions in such a way as to work even if the modules are loaded in
the "wrong" order.

Question: do we care about breaking third-party Apache modules that use
the two functions used by mod_authz_svn? (These third-party modules
would need to do the same thing I'm patching mod_authz_svn to do.) If
so, we can leave the original prototype in the public mod_dav_svn.h
instead of moving it, as I've done below.

-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/
[[[
Use apr_optional.h facilities to import mod_dav_svn functions into
mod_authz_svn.
* subversion/include/mod_dav_svn.h
  (): Convert prototypes for dav_svn_split_uri and
   dav_svn_get_repos_path to APR optional function types.  Move the original prototypes...
* subversion/mod_dav_svn/dav_svn.h
  (): ...here.
* subversion/mod_dav_svn/mod_dav_svn.c
  (register_hooks): Register the two functions for export.
* subversion/mod_authz_svn/mod_authz_svn.c
  (dav_svn_get_repos_path, dav_svn_split_uri): Declare as static
   function pointers.
  (register_hooks): Populate function pointers by calling...
  (import_dav_svn): New.
]]]
Index: subversion/include/mod_dav_svn.h
===================================================================
--- subversion/include/mod_dav_svn.h	(revision 1204126)
+++ subversion/include/mod_dav_svn.h	(working copy)
@@ -30,6 +30,7 @@
 
 #include <httpd.h>
 #include <mod_dav.h>
+#include <apr_optional.h>
 
 
 #ifdef __cplusplus
@@ -75,22 +76,24 @@ extern "C" {
      - @a repos_path:     A/B/alpha
      - @a trailing_slash: FALSE
 */
-AP_MODULE_DECLARE(dav_error *) dav_svn_split_uri(request_rec *r,
-                                                 const char *uri,
-                                                 const char *root_path,
-                                                 const char **cleaned_uri,
-                                                 int *trailing_slash,
-                                                 const char **repos_basename,
-                                                 const char **relative_path,
-                                                 const char **repos_path);
+APR_DECLARE_OPTIONAL_FN(dav_error *, dav_svn_split_uri,
+                        (request_rec *r,
+                         const char *uri,
+                         const char *root_path,
+                         const char **cleaned_uri,
+                         int *trailing_slash,
+                         const char **repos_basename,
+                         const char **relative_path,
+                         const char **repos_path));
 
 
 /**
  * Given an apache request @a r and a @a root_path to the svn location
  * block, set @a *repos_path to the path of the repository on disk.  */
-AP_MODULE_DECLARE(dav_error *) dav_svn_get_repos_path(request_rec *r,
-                                                      const char *root_path,
-                                                      const char **repos_path);
+APR_DECLARE_OPTIONAL_FN(dav_error *, dav_svn_get_repos_path,
+                        (request_rec *r,
+                         const char *root_path,
+                         const char **repos_path));
 
 #ifdef __cplusplus
 }
Index: subversion/mod_authz_svn/mod_authz_svn.c
===================================================================
--- subversion/mod_authz_svn/mod_authz_svn.c	(revision 1204126)
+++ subversion/mod_authz_svn/mod_authz_svn.c	(working copy)
@@ -64,6 +64,9 @@ typedef struct authz_svn_config_rec {
   const char *force_username_case;
 } authz_svn_config_rec;
 
+static APR_OPTIONAL_FN_TYPE(dav_svn_get_repos_path) *dav_svn_get_repos_path;
+static APR_OPTIONAL_FN_TYPE(dav_svn_split_uri) *dav_svn_split_uri;
+
 /*
  * Configuration
  */
@@ -777,6 +780,12 @@ auth_checker(request_rec *r)
   return OK;
 }
 
+static void import_dav_svn(void)
+{
+  dav_svn_get_repos_path = APR_RETRIEVE_OPTIONAL_FN(dav_svn_get_repos_path);
+  dav_svn_split_uri = APR_RETRIEVE_OPTIONAL_FN(dav_svn_split_uri);
+}
+
 /*
  * Module flesh
  */
@@ -798,6 +807,7 @@ register_hooks(apr_pool_t *p)
                        AUTHZ_SVN__SUBREQ_BYPASS_PROV_NAME,
                        AUTHZ_SVN__SUBREQ_BYPASS_PROV_VER,
                        (void*)subreq_bypass);
+  ap_hook_optional_fn_retrieve(import_dav_svn,NULL,NULL,APR_HOOK_MIDDLE);
 }
 
 module AP_MODULE_DECLARE_DATA authz_svn_module =
Index: subversion/mod_dav_svn/mod_dav_svn.c
===================================================================
--- subversion/mod_dav_svn/mod_dav_svn.c	(revision 1204126)
+++ subversion/mod_dav_svn/mod_dav_svn.c	(working copy)
@@ -1040,6 +1040,9 @@ static dav_provider provider =
 static void
 register_hooks(apr_pool_t *pconf)
 {
+  APR_REGISTER_OPTIONAL_FN(dav_svn_get_repos_path);
+  APR_REGISTER_OPTIONAL_FN(dav_svn_split_uri);
+
   ap_hook_pre_config(init_dso, NULL, NULL, APR_HOOK_REALLY_FIRST);
   ap_hook_post_config(init, NULL, NULL, APR_HOOK_MIDDLE);
 
Index: subversion/mod_dav_svn/dav_svn.h
===================================================================
--- subversion/mod_dav_svn/dav_svn.h	(revision 1204126)
+++ subversion/mod_dav_svn/dav_svn.h	(working copy)
@@ -446,6 +446,17 @@ int dav_svn__method_post(request_rec *r);
 
 /*** repos.c ***/
 
+/* See doc in public mod_dav_svn.h */
+dav_error *
+dav_svn_split_uri(request_rec *r,
+                  const char *uri,
+                  const char *root_path,
+                  const char **cleaned_uri,
+                  int *trailing_slash,
+                  const char **repos_basename,
+                  const char **relative_path,
+                  const char **repos_path);
+
 /* generate an ETag for RESOURCE and return it, allocated in POOL. */
 const char *
 dav_svn__getetag(const dav_resource *resource, apr_pool_t *pool);
@@ -492,7 +503,13 @@ extern const dav_hooks_repository dav_svn__hooks_r
 /*** deadprops.c ***/
 extern const dav_hooks_propdb dav_svn__hooks_propdb;
 
+/* See doc in public mod_dav_svn.h */
+dav_error *
+dav_svn_get_repos_path(request_rec *r,
+                       const char *root_path,
+                       const char **repos_path);
 
+
 /*** lock.c ***/
 extern const dav_hooks_locks dav_svn__hooks_locks;
 
Received on 2011-11-20 06:46:49 CET

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.