Eric Gillespie <epg@pretzelnet.org> writes:
> This may seem out-of-scope given the focus on the 1.5 release,
> but since we've already changed the logging for 1.5, I think we
> need to consider these things before we ship. The hard part was
> digging out all this information; I spent my evening playing code
> archaeologist. I don't think fixing all this will be a large
> change, if my suggestions seem like the right way to go.
I've finally returned to this.
> - The biggest problem I've found is we don't log the repository!
> DOH! Instead of adding this the SVN-ACTION environment
> variable, I suggest adding two more variables, SVN-REPOS
> (containing the full path) and SVN-REPOS-NAME (containing only
> the basename). We use the basename for other things already,
> such as authz with ParentPath repositories. Admins using
> ParentPath (and some who aren't, e.g. one repository per vhost)
> probably just want to log the basename, not clutter up the log
> with the full path.
I'll commit this if davautocheck passes; see patch and log
message below, if interested. dav_svn__operational_log isn't the
greatest name in the world; speak up if you have a better idea.
> - Do the '' quotes aid or hinder readability? For me, they're a
> big hindrance.
Removed in r28543.
[[[
Consolidate operational logging and log repository path and name.
* subversion/mod_dav_svn/util.c
(dav_svn__operational_log): Add helper function to log to SVN-ACTION
environment variable as before, but also log repository in
SVN-REPOS and SVN-REPOS-NAME.
* subversion/mod_dav_svn/dav_svn.h
(dav_svn__operational_log): Declare.
* subversion/mod_dav_svn/deadprops.c
(save_value): Use dav_svn__operational_log instead of calling
setting SVN-ACTION directly here.
* subversion/mod_dav_svn/lock.c
(append_locks, remove_lock): same
* subversion/mod_dav_svn/repos.c
(do_walk): same
* subversion/mod_dav_svn/version.c
(merge): same
* subversion/mod_dav_svn/reports/file-revs.c
(dav_svn__file_revs_report): same
* subversion/mod_dav_svn/reports/log.c
(dav_svn__log_report): same
* subversion/mod_dav_svn/reports/mergeinfo.c
(dav_svn__get_mergeinfo_report): same
* subversion/mod_dav_svn/reports/replay.c
(dav_svn__replay_report): same
* subversion/mod_dav_svn/reports/update.c
(dav_svn__update_report): same
* subversion/tests/cmdline/davautocheck.sh
Use SVN-REPOS-NAME in ops log.
]]]
Index: subversion/mod_dav_svn/deadprops.c
===================================================================
--- subversion/mod_dav_svn/deadprops.c (revision 28543)
+++ subversion/mod_dav_svn/deadprops.c (working copy)
@@ -170,12 +170,13 @@
db->resource->pool);
/* Tell the logging subsystem about the revprop change. */
- apr_table_set(db->resource->info->r->subprocess_env, "SVN-ACTION",
- apr_psprintf(db->resource->pool,
- "revprop-change r%ld %s",
+ dav_svn__operational_log(db->resource->info,
+ apr_psprintf(db->resource->pool,
+ "revprop-change r%ld %s %s",
db->resource->info->root.rev,
svn_path_uri_encode(propname,
- db->resource->pool)));
+ db->resource->pool),
+ db->resource->info->repos->repo_name));
}
else
serr = svn_repos_fs_change_node_prop(db->resource->info->root.root,
Index: subversion/mod_dav_svn/dav_svn.h
===================================================================
--- subversion/mod_dav_svn/dav_svn.h (revision 28538)
+++ subversion/mod_dav_svn/dav_svn.h (working copy)
@@ -743,6 +743,11 @@
ap_filter_t *output,
apr_pool_t *pool);
+/* In INFO->r->subprocess_env set "SVN-ACTION" to LINE, "SVN-REPOS" to
+ * INFO->repos->fs_path, and "SVN-REPOS-NAME" to INFO->repos->repo_name. */
+void
+dav_svn__operational_log(struct dav_resource_private *info, const char *line);
+
/*** mirror.c ***/
/* Perform the fixup hook for the R request. */
Index: subversion/mod_dav_svn/version.c
===================================================================
--- subversion/mod_dav_svn/version.c (revision 28543)
+++ subversion/mod_dav_svn/version.c (working copy)
@@ -1325,11 +1325,11 @@
source->info->r->connection->pool);
/* We've detected a 'high level' svn action to log. */
- apr_table_set(target->info->r->subprocess_env, "SVN-ACTION",
- apr_psprintf(target->info->r->pool,
- "commit %s r%ld",
- target->info->repos_path,
- new_rev));
+ dav_svn__operational_log(target->info,
+ apr_psprintf(target->info->r->pool,
+ "commit %s r%ld",
+ target->info->repos_path,
+ new_rev));
/* Since the commit was successful, the txn ID is no longer valid.
Store an empty txn ID in the activity database so that when the
Index: subversion/mod_dav_svn/lock.c
===================================================================
--- subversion/mod_dav_svn/lock.c (revision 28543)
+++ subversion/mod_dav_svn/lock.c (working copy)
@@ -758,10 +758,10 @@
slock->owner);
/* Log the locking as a 'high-level' action. */
- apr_table_set(resource->info->r->subprocess_env, "SVN-ACTION",
- apr_psprintf(resource->info->r->pool,
- "lock %s",
- svn_path_uri_encode(slock->path,
+ dav_svn__operational_log(resource->info,
+ apr_psprintf(resource->info->r->pool,
+ "lock %s",
+ svn_path_uri_encode(slock->path,
resource->info->r->pool)));
return 0;
@@ -845,8 +845,8 @@
resource->pool);
/* Log the unlocking as a 'high-level' action. */
- apr_table_set(resource->info->r->subprocess_env, "SVN-ACTION",
- apr_psprintf(resource->info->r->pool,
+ dav_svn__operational_log(resource->info,
+ apr_psprintf(resource->info->r->pool,
"unlock %s",
svn_path_uri_encode(resource->info->repos_path,
resource->info->r->pool)));
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 28543)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -3461,8 +3461,8 @@
Note: if we cared, we could look at the 'User-Agent:' request
header and distinguish an svn client ('svn ls') from a generic
DAV client. */
- apr_table_set(ctx->info.r->subprocess_env, "SVN-ACTION",
- apr_psprintf(params->pool,
+ dav_svn__operational_log(&ctx->info,
+ apr_psprintf(params->pool,
"list-dir %s r%ld",
svn_path_uri_encode(ctx->info.repos_path,
params->pool),
Index: subversion/mod_dav_svn/reports/log.c
===================================================================
--- subversion/mod_dav_svn/reports/log.c (revision 28543)
+++ subversion/mod_dav_svn/reports/log.c (working copy)
@@ -425,9 +425,8 @@
include_merged_revisions ? "-merge-sensitive" : "",
comma_separated_paths->data, start, end,
comma_separated_revprops->data);
- apr_table_set(resource->info->r->subprocess_env, "SVN-ACTION", action);
+ dav_svn__operational_log(resource->info, action);
-
/* Flush the contents of the brigade (returning an error only if we
don't already have one). */
if (!lrb.needs_header)
Index: subversion/mod_dav_svn/reports/update.c
===================================================================
--- subversion/mod_dav_svn/reports/update.c (revision 28543)
+++ subversion/mod_dav_svn/reports/update.c (working copy)
@@ -1362,7 +1362,7 @@
}
}
- apr_table_set(resource->info->r->subprocess_env, "SVN-ACTION", action);
+ dav_svn__operational_log(resource->info, action);
}
/* this will complete the report, and then drive our editor to generate
Index: subversion/mod_dav_svn/reports/mergeinfo.c
===================================================================
--- subversion/mod_dav_svn/reports/mergeinfo.c (revision 28543)
+++ subversion/mod_dav_svn/reports/mergeinfo.c (working copy)
@@ -187,9 +187,8 @@
(paths, 0, const char *),
resource->pool));
- apr_table_set(resource->info->r->subprocess_env, "SVN-ACTION", action);
+ dav_svn__operational_log(resource->info, action);
-
/* Flush the contents of the brigade (returning an error only if we
don't already have one). */
if ((apr_err = ap_fflush(output, bb)) && !derr)
Index: subversion/mod_dav_svn/reports/file-revs.c
===================================================================
--- subversion/mod_dav_svn/reports/file-revs.c (revision 28543)
+++ subversion/mod_dav_svn/reports/file-revs.c (working copy)
@@ -322,8 +322,8 @@
cleanup:
/* We've detected a 'high level' svn action to log. */
- apr_table_set(resource->info->r->subprocess_env, "SVN-ACTION",
- apr_psprintf(resource->pool, "blame%s %s r%ld:%ld",
+ dav_svn__operational_log(resource->info,
+ apr_psprintf(resource->pool, "blame%s %s r%ld:%ld",
include_merged_revisions ?
"-merge-sensitive" : "",
svn_path_uri_encode(path, resource->pool),
Index: subversion/mod_dav_svn/reports/replay.c
===================================================================
--- subversion/mod_dav_svn/reports/replay.c (revision 28543)
+++ subversion/mod_dav_svn/reports/replay.c (working copy)
@@ -495,7 +495,7 @@
else
action = apr_psprintf(resource->info->r->pool, "replay r%ld", rev);
- apr_table_set(resource->info->r->subprocess_env, "SVN-ACTION", action);
+ dav_svn__operational_log(resource->info, action);
}
ap_fflush(output, bb);
Index: subversion/mod_dav_svn/util.c
===================================================================
--- subversion/mod_dav_svn/util.c (revision 28538)
+++ subversion/mod_dav_svn/util.c (working copy)
@@ -479,3 +479,12 @@
return svn_base64_encode(stream, pool);
}
+
+void
+dav_svn__operational_log(struct dav_resource_private *info, const char *line)
+{
+ apr_table_set(info->r->subprocess_env, "SVN-ACTION", line);
+ apr_table_set(info->r->subprocess_env, "SVN-REPOS", info->repos->fs_path);
+ apr_table_set(info->r->subprocess_env, "SVN-REPOS-NAME",
+ info->repos->repo_basename);
+}
Index: subversion/tests/cmdline/davautocheck.sh
===================================================================
--- subversion/tests/cmdline/davautocheck.sh (revision 28538)
+++ subversion/tests/cmdline/davautocheck.sh (working copy)
@@ -267,7 +267,7 @@
HostNameLookups Off
LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-Agent}i\"" format
CustomLog "$HTTPD_ROOT/req" format
-CustomLog "$HTTPD_ROOT/ops" "%t %u %{SVN-ACTION}e" env=SVN-ACTION
+CustomLog "$HTTPD_ROOT/ops" "%t %u %{SVN-REPOS-NAME}e %{SVN-ACTION}e" env=SVN-ACTION
<Directory />
AllowOverride none
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Dec 19 02:37:48 2007