You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2005/06/05 18:45:32 UTC
[PATCH] operation logging
This is a proposal and patch for operation logging.
Goal:
=====
Have the server log "high level" client operations, such as checkout,
update, switch, diff, commit, log, cat, etc.
Think of it as glorified "CVSROOT/history" functionality :-).
Design:
=======
Instead of having the server deduce operations, have the client
declare them.
Remember that, depending on the RA layer, some operations look the
same over the wire: updates, checkouts, and switches are all very
similar, for example. Diff and merge likewise. Thus, deducing the
operation by looking at the underlying RA requests can be difficult,
and in some cases impossible.
So here's how we do it instead:
1. New 'operation_name' field inside svn_ra_session_t, and a new
public RA function to set its value.
2. All svn_client_XXX() functions set operation_name at critical
points before invoking the relevant svn_ra_foo().
3. If the operation_name is non-NULL in the svn_ra_session_t
object, the RA layer sends the operation name to the server as
request metadata (then it sets the operation name to NULL, so
that one operation doesn't get logged multiple times)
4. The server notices the string and logs it.
In this patch, Karl taught libsvn_client when to announce operations
and when not to. Ben taught ra_dav to send the operation_name in a
custom HTTP header, and have mod_dav_svn parse the string and log it
to Apache's logging system.
We haven't done ra_svn/svnserve yet, but we will if this flies.
Advantages:
* Ensures consistent logs across RA implementations, because the
client is declaring, rather than server deducing.
* The door is still open for a 3rd-party logging library. If we
give libsvn_repos a generalized logging facility, then both
svnserve *and* mod_dav_svn can use it. Of course, mod_dav_svn
should continue to log to Apache's native logs, and that's the
part we implemented in this patch, because it allowed us to punt
on the question of a generalized logging library for now.
Here's the patch. Ben Collins-Sussman and I decided to do this
skunkworks-style, because the design was so simple that the best way
to express it was just to code it :-). Don't be put off by the size
of the patch -- most of it is just the expected fallout from a few
small changes, so it should be pretty easy to follow.
The patch works, but note that for certain operations, sometimes an
empty or absent path gets logged. This is due to some mod_dav_svn
trickery and is completely fixeable. We just haven't bothered to fix
it yet, since the fix won't affect the overall design, and we wanted
to get this in front of the list as soon as it was working.
[[[
With Ben Collins-Sussman, implement operation logging for ra_dav.
Make libsvn_client tell the RA layer the operation name:
* subversion/libsvn_ra/ra_loader.h
(struct svn_ra_session_t): New operation_name field.
* subversion/libsvn_client/client.h,
(svn_client__update_internal, svn_client__checkout_internal): Take
new operation_name parameter.
* subversion/libsvn_ra/ra_loader.c
(svn_ra_set_operation_name): New function.
(svn_ra_open): Initialize the session's operation_name to NULL.
* subversion/libsvn_client/add.c
(mkdir_urls): Set operation name to "mkdir".
* subversion/libsvn_client/blame.c
(svn_client_blame2): Set operation name to "blame".
(old_blame): Set operation name to "blame", but leave a comment
speculating that "blame-old" might be better.
* subversion/libsvn_client/cat.c
(svn_client_cat2): Set operation name to "cat".
* subversion/libsvn_client/checkout.c
(svn_client__checkout_internal): Take new operation_name parameter,
pass it along to svn_client__update_internal.
* subversion/libsvn_client/commit.c
(svn_client_import): Set the operation name to "import".
(svn_client_commit2): Set the operation name to "commit".
* subversion/libsvn_client/copy.c
(repos_to_repos_copy): Set operation name to "copy-repos-to-repos".
(wc_to_repos_copy): Set operation name to "copy-wc-to-repos".
(repos_to_wc_copy): Set operation name to "copy-repos-to-wc-dir" in
the directory case, and "copy-repos-to-wc-file" in the file case.
### For other kinds of copies, the dir-vs-file distinction would ###
### be hard to detect, so we just report that they're copies. ###
### This makes our reporting a bit inconsistent. Is that bad? ###
* subversion/libsvn_client/delete.c
(delete_urls): Set operation name to "delete".
* subversion/libsvn_client/diff.c
(do_merge): Set operation name to "merge".
(single_file_merge_get_file): Set operation name to "merge-single-file".
(diff_repos_repos): Set operation name to "diff-repos-to-repos".
(diff_repos_wc): Set operation name to "diff-repos-to-wc".
* subversion/libsvn_client/export.c
(svn_client_export3): Set operation name to "export-file" or
"export-directory" as appropriate.
* subversion/libsvn_client/info.c
(svn_client_info): Set operation name to "info", in both first
attempt and in fallback case.
* subversion/libsvn_client/locking_commands.c
(svn_client_lock): Set operation name to "lock".
(svn_client_unlock): Set operation name to "unlock".
* subversion/libsvn_client/log.c
(svn_client_log2): Set operation name to "log".
* subversion/libsvn_client/ls.c
(svn_client_ls2): Set operation name to "list-dir" or "list-file" as
appropriate.
* subversion/libsvn_client/prop_commands.c
(svn_client_revprop_set): Set operation name to "revprop-set".
(svn_client_revprop_get): Set operation name to "revprop-get".
(svn_client_revprop_list): Set operation name to "revprop-list".
(svn_client_propget2): Set operation name to "prop-get".
(svn_client_proplist2): Set operation name to "prop-list".
* subversion/libsvn_client/relocate.c
(validator_func): Leave a comment about how to do operation logging.
* subversion/libsvn_client/status.c
(svn_client_status2): Set operation name to "status". Leave a
comment about a possible better way to do it.
* subversion/libsvn_client/update.c
(svn_client__update_internal): Take new operation_name name
parameter, set the ra_session's operation name to it right before
the reporter drive. Leave a comment about possible better way.
* subversion/libsvn_client/switch.c
(svn_client_switch): Set operation name to "switch". Leave a
comment about possible better way.
Make ra_dav transmit the operation name:
* subversion/include/svn_dav.h
(SVN_DAV_OPERATION_NAME_HEADER): New request header.
* subversion/mod_dav_svn/repos.c
(dav_svn_get_resource): Notice new header, log its value as at INFO
level in apache's errorlog.
* subversion/libsvn_ra_dav/ra_dav.h
(svn_ra_dav__merge_activity): Take an svn_ra_session_t argument.
(svn_ra_dav__get_props): Take an extra_headers argument.
(struct lock_request_baton): Add an operation_name field.
* subversion/libsvn_ra_dav/props.c
(svn_ra_dav__get_props): Take incoming extra_headers arg and add to
it, rather than creating the hash.
(svn_ra_dav__do_stat): If session->operation_name is present, send
it in the PROPFIND. This covers 'svn info'.
(svn_ra_dav__get_props_resource): Pass NULL extra_headers to
svn_ra_dav__get_props.
* subversion/libsvn_ra_dav/fetch.c
(report_baton_t): Store svn_ra_session_t as field in report baton.
(make_reporter): Set the new field.
(svn_ra_dav__change_rev_prop): If session->operation_name is
present, send it in the PROPPATCH. This covers 'svn ps --revprop'.
(reporter_finish_report, svn_ra_dav__get_dated_revision,
svn_ra_dav__get_locations, svn_ra_dav__get_locks): If
session->operation_name is present, send it in the REPORT headers.
This covers 'svn co/export/up/switch/diff/merge'.
(svn_ra_dav__get_dir): If operation_name is present, pass it as an
extra header to __get_props(). This covers 'svn ls' and 'svn pl --revprop'.
(custom_get_request): Take 'operation_name' parameter, attach to request.
(simple_fetch_file): Pass NULL to new custom_get_request() param.
(svn_ra_dav__get_file): Pass session->operation_name to
custom_get_request(). This covers 'svn cat'.
(start_element): Pass NULL extra_headers to svn_ra_dav__get_props.
* subversion/libsvn_ra_dav/file_revs.c
(svn_ra_dav__get_file_revs): If operation_name is present, pass it
as an extra REPORT header. This covers 'svn blame'.
* subversion/libsvn_ra_dav/log.c
(svn_ra_dav__get_log): If operation_name is present, send it as
an extra header in the REPORT request. This covers 'svn log'.
* subversion/libsvn_ra_dav/commit.c
(struct commit_ctx_t): Add svn_ra_session_t field.
(svn_ra_dav__get_commit_editor): Set the session field.
(commit_close_edit): Pass the session to __merge_activity().
* subversion/libsvn_ra_dav/merge.c: Include ../libsvn_ra/ra_loader.h.
(svn_ra_dav__merge_activity): If operation_name is present, send it
as an extra header in the MERGE request. This covers any client
command that causes a commit to happen.
* subversion/libsvn_ra_dav/session.c
(svn_ra_dav__lock): Pass session->operation_name to shim_ra_dav__lock().
(shim_svn_ra_dav__lock): Take an operation_name arg, place it in lrb.
(pre_send_hook): If lrb->operation_name is present, attach as header
to LOCK/UNLOCK request. This covers 'svn lock/unlock'.
]]]
Index: subversion/libsvn_ra/ra_loader.c
===================================================================
--- subversion/libsvn_ra/ra_loader.c (revision 14982)
+++ subversion/libsvn_ra/ra_loader.c (working copy)
@@ -272,6 +272,7 @@
session = apr_pcalloc (pool, sizeof (*session));
session->vtable = vtable;
session->pool = pool;
+ session->operation_name = NULL;
/* Ask the library to open the session. */
SVN_ERR (vtable->open (session, repos_URL, callbacks, callback_baton,
@@ -281,6 +282,12 @@
return SVN_NO_ERROR;
}
+void svn_ra_set_operation_name(svn_ra_session_t *session,
+ const char *operation_name)
+{
+ session->operation_name = operation_name;
+}
+
svn_error_t *svn_ra_get_latest_revnum (svn_ra_session_t *session,
svn_revnum_t *latest_revnum,
apr_pool_t *pool)
Index: subversion/libsvn_ra/ra_loader.h
===================================================================
--- subversion/libsvn_ra/ra_loader.h (revision 14982)
+++ subversion/libsvn_ra/ra_loader.h (working copy)
@@ -207,6 +207,26 @@
/* Pool used to manage this session. */
apr_pool_t *pool;
+ /** The name of the operation the client wants to perform, to assist
+ * the server in logging the operation.
+ *
+ * If NULL, do not report an operation name to the server.
+ *
+ * Else, report the operation name exactly once, then set this field
+ * to NULL. That way, when a single operation involves multiple RA
+ * requests, the operation does not get logged multiple times.
+ *
+ * @note If you reuse the same svn_ra_session_t for multiple
+ * operations, remember to reset operation_name between them.
+ *
+ * This field is necessary because sometimes the same RA API is used
+ * for different operations, e.g., checkout and update both use
+ * svn_ra_do_update(). For the server to log the operation in a way
+ * that reflects what the client intended to do, the client must
+ * tell the RA layer the operation's name.
+ */
+ const char *operation_name;
+
/* Private data for the RA implementation. */
void *priv;
};
Index: subversion/include/svn_dav.h
===================================================================
--- subversion/include/svn_dav.h (revision 14982)
+++ subversion/include/svn_dav.h (working copy)
@@ -107,6 +107,12 @@
#define SVN_DAV_RESULT_FULLTEXT_MD5_HEADER "X-SVN-Result-Fulltext-MD5"
/** @} */
+
+/** A header possibly attached to outbound ra_dav requests which
+ announces the client operation being performed. */
+#define SVN_DAV_OPERATION_NAME_HEADER "X-SVN-Operation-Name"
+
+
/* ### should add strings for the various XML elements in the reports
### and things. also the custom prop names. etc.
*/
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 14982)
+++ subversion/include/svn_ra.h (working copy)
@@ -397,6 +397,25 @@
apr_hash_t *config,
apr_pool_t *pool);
+/* Set the client operation name for @a session to @a operation_name.
+ * This is the name the session should report to the server for the
+ * next operation, so the server can log that if it wants to.
+ *
+ * The session will only transmit this name once -- after the first
+ * time, it sets the name to NULL, so that if @a session is used
+ * multiple times as part of the same operation, the operation does
+ * not get logged multiple times.
+ *
+ * There is no corresponding get() function because only the RA code
+ * should ever need to get the operation name, and it has direct
+ * access to @a session's internals.
+ *
+ * @since New in 1.3.
+ */
+void svn_ra_set_operation_name(svn_ra_session_t *session,
+ const char *operation_name);
+
+
/**
* Get the latest revision number from the repository of @a session.
*
Index: subversion/libsvn_client/relocate.c
===================================================================
--- subversion/libsvn_client/relocate.c (revision 14982)
+++ subversion/libsvn_client/relocate.c (working copy)
@@ -39,7 +39,6 @@
const char *path;
apr_hash_t *url_uuids;
apr_pool_t *pool;
-
};
@@ -100,6 +99,16 @@
SVN_ERR (svn_client__open_ra_session_internal (&ra_session, url, NULL,
NULL, NULL, FALSE, TRUE,
b->ctx, subpool));
+
+ /* ### If we wanted to log relocation operations on the server
+ * ### side, we would do more than just call
+ * ### svn_ra_set_operation_name() here. We'd add a new field
+ * ### 'operation_name' to validator_baton_t, call
+ * ### svn_ra_set_operation_name(ra_session, b->operation_name),
+ * ### and then set b->operation_name to NULL, so that NULL, so
+ * ### the operation isn't recorded again for each working copy
+ * ### subdirectory involved.
+ */
SVN_ERR (svn_ra_get_uuid (ra_session, &ra_uuid, subpool));
ra_uuid = apr_pstrdup (pool, ra_uuid);
svn_pool_destroy (subpool);
Index: subversion/libsvn_client/switch.c
===================================================================
--- subversion/libsvn_client/switch.c (revision 14982)
+++ subversion/libsvn_client/switch.c (working copy)
@@ -146,7 +146,15 @@
We pass NULL for traversal_info because this is a switch, not an
update, and therefore we don't want to handle any externals
- except the ones directly affected by the switch. */
+ except the ones directly affected by the switch.
+
+ ### Set the operation name here, even though a better place would
+ ### be inside svn_wc_crawl_revisions2() right before
+ ### reporter->finish_report() is called. We'd have to rev the
+ ### API to svn_wc_crawl_revisions3() to do that, though, and it's
+ ### probably not worth it.
+ */
+ svn_ra_set_operation_name (ra_session, "switch");
err = svn_wc_crawl_revisions2 (path, dir_access, reporter, report_baton,
TRUE, recurse, use_commit_times,
ctx->notify_func2, ctx->notify_baton2,
Index: subversion/libsvn_client/delete.c
===================================================================
--- subversion/libsvn_client/delete.c (revision 14982)
+++ subversion/libsvn_client/delete.c (working copy)
@@ -192,6 +192,7 @@
pool));
/* Call the path-based editor driver. */
+ svn_ra_set_operation_name (ra_session, "delete");
err = svn_delta_path_driver (editor, edit_baton, SVN_INVALID_REVNUM,
targets, path_driver_cb_func,
(void *)editor, pool);
Index: subversion/libsvn_client/externals.c
===================================================================
--- subversion/libsvn_client/externals.c (revision 14982)
+++ subversion/libsvn_client/externals.c (working copy)
@@ -249,7 +249,8 @@
&(new_item->revision),
TRUE, FALSE,
ib->timestamp_sleep,
- ib->ctx, ib->pool));
+ "checkout-external", ib->ctx,
+ ib->pool));
}
else if (! new_item)
{
@@ -306,7 +307,8 @@
&(new_item->revision),
TRUE, FALSE,
ib->timestamp_sleep,
- ib->ctx, ib->pool));
+ "checkout-external", ib->ctx,
+ ib->pool));
}
else if (ib->update_unchanged)
{
@@ -345,7 +347,8 @@
&(new_item->revision),
TRUE, FALSE,
ib->timestamp_sleep,
- ib->ctx, ib->pool));
+ "update-external", ib->ctx,
+ ib->pool));
}
/* If, however, the URLs don't match, we need to relegate
the one subdir, and then checkout a new one. */
@@ -363,6 +366,7 @@
&(new_item->revision),
TRUE, FALSE,
ib->timestamp_sleep,
+ "checkout-external",
ib->ctx, ib->pool));
}
}
@@ -379,7 +383,8 @@
&(new_item->revision),
TRUE, FALSE,
ib->timestamp_sleep,
- ib->ctx, ib->pool));
+ "checkout-external", ib->ctx,
+ ib->pool));
}
}
Index: subversion/libsvn_client/export.c
===================================================================
--- subversion/libsvn_client/export.c (revision 14982)
+++ subversion/libsvn_client/export.c (working copy)
@@ -808,6 +808,7 @@
/* Step outside the editor-likeness for a moment, to actually talk
* to the repository. */
+ svn_ra_set_operation_name (ra_session, "export-file");
SVN_ERR (svn_ra_get_file (ra_session, "", revnum,
svn_stream_from_aprfile (fb->tmp_file,
pool),
@@ -866,6 +867,7 @@
TRUE, /* "help, my dir is empty!" */
NULL, pool));
+ svn_ra_set_operation_name (ra_session, "export-directory");
SVN_ERR (reporter->finish_report (report_baton, pool));
/* Special case: Due to our sly export/checkout method of
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 14982)
+++ subversion/libsvn_client/client.h (working copy)
@@ -317,7 +317,12 @@
function will sleep before returning to ensure timestamp integrity.
If TIMESTAMP_SLEEP is not NULL then the function will not sleep but
will set *TIMESTAMP_SLEEP to TRUE if a sleep is required, and will
- not change *TIMESTAMP_SLEEP if no sleep is required. */
+ not change *TIMESTAMP_SLEEP if no sleep is required.
+
+ OPERATION_NAME is the name of the actual operation the client is
+ performing (it might be checkout or switch instead of update, for
+ example), or NULL if the caller does not want the server to log the
+ precise operation. */
svn_error_t *
svn_client__update_internal (svn_revnum_t *result_rev,
const char *path,
@@ -325,6 +330,7 @@
svn_boolean_t recurse,
svn_boolean_t ignore_externals,
svn_boolean_t *timestamp_sleep,
+ const char *operation_name,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
@@ -335,7 +341,11 @@
timestamp integrity. If TIMESTAMP_SLEEP is not NULL then the
function will not sleep but will set *TIMESTAMP_SLEEP to TRUE if a
sleep is required, and will not change *TIMESTAMP_SLEEP if no sleep
- is required. */
+ is required.
+
+ OPERATION_NAME is the name of the actual operation the client is
+ performing (it might be "externals-checkout", for example), or NULL
+ if the caller does not want the server to log the precise operation. */
svn_error_t *
svn_client__checkout_internal (svn_revnum_t *result_rev,
const char *URL,
@@ -345,6 +355,7 @@
svn_boolean_t recurse,
svn_boolean_t ignore_externals,
svn_boolean_t *timestamp_sleep,
+ const char *operation_name,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
Index: subversion/libsvn_client/status.c
===================================================================
--- subversion/libsvn_client/status.c (revision 14982)
+++ subversion/libsvn_client/status.c (working copy)
@@ -312,7 +312,15 @@
/* Drive the reporter structure, describing the revisions
within PATH. When we call reporter->finish_report,
EDITOR will be driven to describe differences between our
- working copy and HEAD. */
+ working copy and HEAD.
+
+ ### Set the operation name here, even though a better
+ ### place would be inside svn_wc_crawl_revisions2() right
+ ### before reporter->finish_report() is called. We'd
+ ### have to rev the API to svn_wc_crawl_revisions3() to
+ ### do that, though, and it's probably not worth it.
+ */
+ svn_ra_set_operation_name (ra_session, "status");
SVN_ERR (svn_wc_crawl_revisions2 (path, target_access,
&lock_fetch_reporter, &rb, FALSE,
recurse, FALSE, NULL, NULL, NULL,
Index: subversion/libsvn_client/info.c
===================================================================
--- subversion/libsvn_client/info.c (revision 14982)
+++ subversion/libsvn_client/info.c (working copy)
@@ -304,6 +304,7 @@
base_name = svn_path_uri_decode(base_name, pool);
/* Get the dirent for the URL itself. */
+ svn_ra_set_operation_name (ra_session, "info");
err = svn_ra_stat (ra_session, "", rev, &the_ent, pool);
/* svn_ra_stat() will work against old versions of mod_dav_svn, but
@@ -314,6 +315,7 @@
/* Fall back to pre-1.2 strategy for fetching dirent's URL. */
svn_error_clear (err);
+ svn_ra_set_operation_name (ra_session, "info");
SVN_ERR (svn_ra_check_path (ra_session, "", rev, &url_kind, pool));
if (url_kind == svn_node_none)
return svn_error_createf (SVN_ERR_RA_ILLEGAL_URL, NULL,
Index: subversion/libsvn_client/prop_commands.c
===================================================================
--- subversion/libsvn_client/prop_commands.c (revision 14982)
+++ subversion/libsvn_client/prop_commands.c (working copy)
@@ -291,6 +291,7 @@
(set_rev, ra_session, revision, NULL, pool));
/* The actual RA call. */
+ svn_ra_set_operation_name (ra_session, "revprop-set");
SVN_ERR (svn_ra_change_rev_prop (ra_session, *set_rev, propname, propval,
pool));
@@ -580,6 +581,7 @@
SVN_ERR (svn_ra_check_path (ra_session, "", revnum, &kind, pool));
+ svn_ra_set_operation_name (ra_session, "prop-get");
SVN_ERR (remote_propget (*props, propname, url, "",
kind, revnum, ra_session,
recurse, pool));
@@ -678,6 +680,7 @@
(set_rev, ra_session, revision, NULL, pool));
/* The actual RA call. */
+ svn_ra_set_operation_name (ra_session, "revprop-get");
SVN_ERR (svn_ra_rev_prop (ra_session, *set_rev, propname, propval, pool));
return SVN_NO_ERROR;
@@ -932,6 +935,7 @@
SVN_ERR (svn_ra_check_path (ra_session, "", revnum, &kind, pool));
+ svn_ra_set_operation_name (ra_session, "prop-list");
SVN_ERR (remote_proplist (*props, url, "",
kind, revnum, ra_session,
recurse, pool, svn_pool_create (pool)));
@@ -1026,6 +1030,7 @@
(set_rev, ra_session, revision, NULL, pool));
/* The actual RA call. */
+ svn_ra_set_operation_name (ra_session, "revprop-list");
SVN_ERR (svn_ra_rev_proplist (ra_session, *set_rev, &proplist, pool));
*props = proplist;
Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 14982)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -840,6 +840,9 @@
svn_revnum_t rev;
const char *ignored_url;
+ /* ### This function should also return the svn_ra_callbacks_t
+ ### associated with *RA_SESSION_P. */
+
SVN_ERR (svn_client_url_from_path (&initial_url, path_or_url, pool));
if (! initial_url)
return svn_error_createf (SVN_ERR_ENTRY_MISSING_URL, NULL,
Index: subversion/libsvn_client/checkout.c
===================================================================
--- subversion/libsvn_client/checkout.c (revision 14982)
+++ subversion/libsvn_client/checkout.c (working copy)
@@ -49,6 +49,7 @@
svn_boolean_t recurse,
svn_boolean_t ignore_externals,
svn_boolean_t *timestamp_sleep,
+ const char *operation_name,
svn_client_ctx_t *ctx,
apr_pool_t *pool)
{
@@ -80,7 +81,8 @@
/* Get the RA connection. */
SVN_ERR (svn_client__ra_session_from_path (&ra_session, &revnum,
&URL, url, peg_revision,
- revision, ctx, pool));
+ revision,
+ ctx, pool));
SVN_ERR (svn_ra_check_path (ra_session, "", revnum, &kind, pool));
if (kind == svn_node_none)
@@ -107,7 +109,8 @@
/* Have update fix the incompleteness. */
err = svn_client__update_internal (result_rev, path, revision,
recurse, ignore_externals,
- use_sleep, ctx, pool);
+ use_sleep, operation_name, ctx,
+ pool);
}
else if (kind == svn_node_dir)
{
@@ -122,7 +125,8 @@
SVN_ERR (svn_wc_ensure_adm (path, uuid, URL, revnum, pool));
err = svn_client__update_internal (result_rev, path, revision,
recurse, ignore_externals,
- use_sleep, ctx, pool);
+ use_sleep, operation_name,
+ ctx, pool);
goto done;
}
@@ -140,7 +144,8 @@
{
err = svn_client__update_internal (result_rev, path, revision,
recurse, ignore_externals,
- use_sleep, ctx, pool);
+ use_sleep, operation_name,
+ ctx, pool);
}
else
{
@@ -202,7 +207,7 @@
{
return svn_client__checkout_internal (result_rev, URL, path, peg_revision,
revision, recurse, ignore_externals,
- NULL, ctx, pool);
+ NULL, "checkout", ctx, pool);
}
svn_error_t *
@@ -220,5 +225,5 @@
return svn_client__checkout_internal (result_rev, URL, path, &peg_revision,
revision, recurse, FALSE, NULL,
- ctx, pool);
+ "checkout", ctx, pool);
}
Index: subversion/libsvn_client/locking_commands.c
===================================================================
--- subversion/libsvn_client/locking_commands.c (revision 14982)
+++ subversion/libsvn_client/locking_commands.c (working copy)
@@ -444,6 +444,7 @@
cb.ctx = ctx;
/* Lock the paths. */
+ svn_ra_set_operation_name (ra_session, "lock");
SVN_ERR (svn_ra_lock (ra_session, path_revs, comment,
steal_lock, store_locks_callback, &cb, pool));
@@ -498,6 +499,7 @@
cb.ctx = ctx;
/* Unlock the paths. */
+ svn_ra_set_operation_name (ra_session, "unlock");
SVN_ERR (svn_ra_unlock (ra_session, path_tokens, break_lock,
store_locks_callback, &cb, pool));
Index: subversion/libsvn_client/cat.c
===================================================================
--- subversion/libsvn_client/cat.c (revision 14982)
+++ subversion/libsvn_client/cat.c (working copy)
@@ -210,6 +210,8 @@
eol_style = apr_hash_get (props, SVN_PROP_EOL_STYLE, APR_HASH_KEY_STRING);
keywords = apr_hash_get (props, SVN_PROP_KEYWORDS, APR_HASH_KEY_STRING);
+ svn_ra_set_operation_name (ra_session, "cat");
+
if (! eol_style && ! keywords)
{
/* It's a file with no special eol style or keywords. */
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 14982)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -1479,6 +1479,7 @@
SVN_ERR (svn_client__open_ra_session_internal (&ra_session, URL1, NULL,
NULL, NULL, FALSE, TRUE,
ctx, pool));
+
/* Resolve the revision numbers. */
SVN_ERR (svn_client__get_revision_number
(&start_revnum, ra_session, revision1, path1, pool));
@@ -1523,6 +1524,7 @@
SVN_ERR (reporter->set_path (report_baton, "", start_revnum, FALSE, NULL,
pool));
+ svn_ra_set_operation_name (ra_session, "merge");
SVN_ERR (reporter->finish_report (report_baton, pool));
return SVN_NO_ERROR;
@@ -1555,6 +1557,7 @@
SVN_ERR (svn_io_open_unique_file (&fp, filename,
merge_b->target, ".tmp",
FALSE, pool));
+ svn_ra_set_operation_name (ra_session, "merge-single-file");
SVN_ERR (svn_ra_get_file (ra_session, "", *rev,
svn_stream_from_aprfile (fp, pool),
NULL, props, pool));
@@ -1839,12 +1842,12 @@
}
/* Open temporary RA sessions to each URL. */
- SVN_ERR (svn_client__open_ra_session_internal (&ra_session1, url1, NULL,
- NULL, NULL, FALSE, TRUE,
- ctx, temppool));
- SVN_ERR (svn_client__open_ra_session_internal (&ra_session2, url2, NULL,
- NULL, NULL, FALSE, TRUE,
- ctx, temppool));
+ SVN_ERR (svn_client__open_ra_session_internal
+ (&ra_session1, url1, NULL, NULL, NULL, FALSE, TRUE,
+ ctx, temppool));
+ SVN_ERR (svn_client__open_ra_session_internal
+ (&ra_session2, url2, NULL, NULL, NULL, FALSE, TRUE,
+ ctx, temppool));
/* Resolve named revisions to real numbers. */
SVN_ERR (svn_client__get_revision_number
@@ -1926,6 +1929,7 @@
/* Drive the reporter; do the diff. */
SVN_ERR (reporter->set_path (report_baton, "", rev1, FALSE, NULL, pool));
+ svn_ra_set_operation_name (ra_session1, "diff-repos-to-repos");
SVN_ERR (reporter->finish_report (report_baton, pool));
return SVN_NO_ERROR;
@@ -2016,7 +2020,7 @@
SVN_ERR (svn_client__open_ra_session_internal (&ra_session, anchor_url,
NULL, NULL, NULL, FALSE, TRUE,
ctx, pool));
-
+
SVN_ERR (svn_wc_get_diff_editor3 (adm_access, target,
callbacks, callback_baton,
recurse,
@@ -2041,6 +2045,16 @@
url1,
diff_editor, diff_edit_baton, pool));
+ /* ### I'd love to set the operation name right before the call to
+ ### reporter->finish_report(). Unfortunately, that happens down
+ ### in libsvn_wc, where the sun don't shine. We could just add a
+ ### parameter and rev the API to svn_wc_crawl_revisions3(). I
+ ### think, though, that it's better just to set the operation
+ ### here, and hope that reporter->finish_report() has the good
+ ### sense to be the only place in the RA reporter code that sends
+ ### the operation name. Can we count on that? Ben? Bueller? */
+ svn_ra_set_operation_name (ra_session, "diff-repos-to-wc");
+
/* Create a txn mirror of path2; the diff editor will print
diffs in reverse. :-) */
SVN_ERR (svn_wc_crawl_revisions2 (path2, dir_access,
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 14982)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -473,6 +473,7 @@
cb_baton.resurrection = resurrection;
/* Call the path-based editor driver. */
+ svn_ra_set_operation_name (ra_session, "copy-repos-to-repos");
err = svn_delta_path_driver (editor, edit_baton, youngest, paths,
path_driver_cb_func, &cb_baton, pool);
if (err)
@@ -718,6 +719,7 @@
commit_in_progress = TRUE;
/* Perform the commit. */
+ svn_ra_set_operation_name (ra_session, "copy-wc-to-repos");
cmt_err = svn_client__do_commit (base_url, commit_items, adm_access,
editor, edit_baton,
0, /* ### any notify_path_offset needed? */
@@ -893,7 +895,7 @@
{
SVN_ERR (svn_client__checkout_internal
(NULL, src_url, dst_path, &revision, &revision,
- TRUE, FALSE, NULL, ctx, pool));
+ TRUE, FALSE, NULL, "copy-repos-to-wc-dir", ctx, pool));
if ((revision.kind == svn_opt_revision_head) && same_repositories)
{
@@ -964,6 +966,7 @@
(&fp, &new_text_path, dst_path, ".tmp", FALSE, pool));
fstream = svn_stream_from_aprfile (fp, pool);
+ svn_ra_set_operation_name (ra_session, "copy-repos-to-wc-file");
SVN_ERR (svn_ra_get_file (ra_session, "", src_revnum, fstream, &real_rev,
&new_props, pool));
SVN_ERR (svn_stream_close (fstream));
Index: subversion/libsvn_client/ls.c
===================================================================
--- subversion/libsvn_client/ls.c (revision 14982)
+++ subversion/libsvn_client/ls.c (working copy)
@@ -96,6 +96,7 @@
{
*dirents = apr_hash_make (pool);
+ svn_ra_set_operation_name (ra_session, "list-dir");
SVN_ERR (get_dir_contents (*dirents, "", rev, ra_session, recurse,
ctx, pool));
}
@@ -117,6 +118,7 @@
ctx, pool));
/* Get all parent's entries, no props. */
+ svn_ra_set_operation_name (ra_session, "list-file");
SVN_ERR (svn_ra_get_dir (ra_session, "", rev, &parent_ents,
NULL, NULL, pool));
Index: subversion/libsvn_client/blame.c
===================================================================
--- subversion/libsvn_client/blame.c (revision 14982)
+++ subversion/libsvn_client/blame.c (working copy)
@@ -560,6 +560,7 @@
We need to ensure that we get one revision before the start_rev,
if available so that we can know what was actually changed in the start
revision. */
+ svn_ra_set_operation_name (ra_session, "blame");
err = svn_ra_get_file_revs (ra_session, "",
start_revnum - (start_revnum > 0 ? 1 : 0),
end_revnum,
@@ -689,6 +690,8 @@
SVN_ERR (svn_client__open_ra_session_internal (&ra_session, reposURL, NULL,
NULL, NULL, FALSE, FALSE,
frb->ctx, pool));
+ /* ### Should this be "blame-old" instead? */
+ svn_ra_set_operation_name (ra_session, "blame");
/* Inspect the first revision's change metadata; if there are any
prior revisions, compute a new starting revision/path. If no
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 14982)
+++ subversion/libsvn_client/log.c (working copy)
@@ -198,6 +198,8 @@
SVN_ERR (svn_client__get_revision_number
(&end_revnum, ra_session, end, base_name, pool));
+ svn_ra_set_operation_name (ra_session, "log");
+
if (start_is_local || end_is_local)
{
/* ### FIXME: At least one revision is locally dynamic, that
Index: subversion/libsvn_client/update.c
===================================================================
--- subversion/libsvn_client/update.c (revision 14982)
+++ subversion/libsvn_client/update.c (working copy)
@@ -45,6 +45,7 @@
svn_boolean_t recurse,
svn_boolean_t ignore_externals,
svn_boolean_t *timestamp_sleep,
+ const char *operation_name,
svn_client_ctx_t *ctx,
apr_pool_t *pool)
{
@@ -134,7 +135,15 @@
/* Drive the reporter structure, describing the revisions within
PATH. When we call reporter->finish_report, the
- update_editor will be driven by svn_repos_dir_delta. */
+ update_editor will be driven by svn_repos_dir_delta.
+
+ ### Set the operation name here, even though a better place would
+ ### be inside svn_wc_crawl_revisions2() right before
+ ### reporter->finish_report() is called. We'd have to rev the
+ ### API to svn_wc_crawl_revisions3() to do that, though, and it's
+ ### probably not worth it.
+ */
+ svn_ra_set_operation_name (ra_session, operation_name);
err = svn_wc_crawl_revisions2 (path, dir_access, reporter, report_baton,
TRUE, recurse, use_commit_times,
ctx->notify_func2, ctx->notify_baton2,
@@ -211,7 +220,7 @@
err = svn_client__update_internal (&result_rev, path, revision,
recurse, ignore_externals,
- &sleep, ctx, subpool);
+ &sleep, "update", ctx, subpool);
if (err && err->apr_err != SVN_ERR_WC_NOT_DIRECTORY)
{
return err;
@@ -247,5 +256,5 @@
apr_pool_t *pool)
{
return svn_client__update_internal (result_rev, path, revision, recurse,
- FALSE, NULL, ctx, pool);
+ FALSE, NULL, "update", ctx, pool);
}
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 14982)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -739,12 +739,12 @@
/* ### Is svn_path_local_style() really necessary for this? */
svn_path_local_style (SVN_WC_ADM_DIR_NAME, pool));
-
- /* If an error occurred during the commit, abort the edit and return
- the error. We don't even care if the abort itself fails. */
+ svn_ra_set_operation_name (ra_session, "import");
if ((err = import (path, new_entries, editor, edit_baton,
nonrecursive, excludes, ctx, subpool)))
{
+ /* If an error occurred during the commit, abort the edit and return
+ the error. We don't even care if the abort itself fails. */
svn_error_clear (editor->abort_edit (edit_baton, subpool));
return err;
}
@@ -1467,6 +1467,7 @@
display_dir = svn_path_get_longest_ancestor (display_dir, base_dir, pool);
/* Perform the commit. */
+ svn_ra_set_operation_name (ra_session, "commit");
cmt_err = svn_client__do_commit (base_url, commit_items, base_dir_access,
editor, edit_baton,
display_dir,
Index: subversion/libsvn_client/add.c
===================================================================
--- subversion/libsvn_client/add.c (revision 14982)
+++ subversion/libsvn_client/add.c (working copy)
@@ -585,6 +585,7 @@
pool));
/* Call the path-based editor driver. */
+ svn_ra_set_operation_name (ra_session, "mkdir");
err = svn_delta_path_driver (editor, edit_baton, SVN_INVALID_REVNUM,
targets, path_driver_cb_func,
(void *)editor, pool);
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 14982)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -1317,6 +1317,19 @@
if (ua && (ap_strstr_c(ua, "SVN/") == ua))
repos->is_svn_client = TRUE;
}
+
+ /* See if the svn client has declared an operation name to log. */
+ {
+ const char *operation_name = apr_table_get(r->headers_in,
+ SVN_DAV_OPERATION_NAME_HEADER);
+ if (operation_name)
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+ "svn operation: "
+ "user '%s', repos '%s': '%s' on path '%s'",
+ repos->username ? repos->username : "anonymous",
+ repos->fs_path, operation_name,
+ repos_path ? repos_path : "(unknown)");
+ }
/* Retrieve/cache open repository */
repos_key = apr_pstrcat(r->pool, "mod_dav_svn:", fs_path, NULL);
Index: subversion/libsvn_ra_dav/file_revs.c
===================================================================
--- subversion/libsvn_ra_dav/file_revs.c (revision 14982)
+++ subversion/libsvn_ra_dav/file_revs.c (working copy)
@@ -306,7 +306,18 @@
int http_status = 0;
struct report_baton rb;
svn_error_t *err;
+ apr_hash_t *extra_headers = NULL;
+ /* If available, send a custom header describing the operation. */
+ if (session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ session->operation_name);
+ session->operation_name = NULL;
+ }
+
static const char request_head[]
= "<S:file-revs-report xmlns:S=\"" SVN_XML_NAMESPACE "\">" DEBUG_CR;
static const char request_tail[]
@@ -350,7 +361,7 @@
err = svn_ra_dav__parsed_request (ras->sess, "REPORT", final_bc_url,
request_body->data, NULL, NULL,
start_element, cdata_handler, end_element,
- &rb, NULL, &http_status, FALSE,
+ &rb, extra_headers, &http_status, FALSE,
ras->pool);
/* Map status 501: Method Not Implemented to our not implemented error.
Index: subversion/libsvn_ra_dav/merge.c
===================================================================
--- subversion/libsvn_ra_dav/merge.c (revision 14982)
+++ subversion/libsvn_ra_dav/merge.c (working copy)
@@ -35,6 +35,8 @@
#include "svn_props.h"
#include "svn_xml.h"
+#include "../libsvn_ra/ra_loader.h"
+
#include "svn_private_config.h"
#include "ra_dav.h"
@@ -641,6 +643,7 @@
svn_revnum_t *new_rev,
const char **committed_date,
const char **committed_author,
+ svn_ra_session_t *session,
svn_ra_dav__session_t *ras,
const char *repos_url,
const char *activity_url,
@@ -688,6 +691,16 @@
value);
}
+ if (session->operation_name != NULL)
+ {
+ if (! extra_headers)
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ apr_pstrdup(pool, session->operation_name));
+ session->operation_name = NULL;
+ }
+
/* Need to marshal the whole [path->token] hash to the server as
a string within the body of the MERGE request. */
if ((lock_tokens != NULL)
Index: subversion/libsvn_ra_dav/log.c
===================================================================
--- subversion/libsvn_ra_dav/log.c (revision 14982)
+++ subversion/libsvn_ra_dav/log.c (working copy)
@@ -322,6 +322,7 @@
svn_string_t bc_url, bc_relative;
const char *final_bc_url;
svn_revnum_t use_rev;
+ apr_hash_t *extra_headers = NULL;
/* ### todo: I don't understand why the static, file-global
variables shared by update and status are called `report_head'
@@ -353,8 +354,17 @@
{ "DAV:", "comment", ELEM_comment, SVN_RA_DAV__XML_CDATA },
{ NULL }
};
+
+ /* If available, send a custom header describing the operation. */
+ if (session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ session->operation_name);
+ session->operation_name = NULL;
+ }
-
/* Construct the request body. */
svn_stringbuf_appendcstr(request_body, log_request_head);
svn_stringbuf_appendcstr(request_body,
@@ -434,7 +444,7 @@
log_start_element,
log_end_element,
&lb,
- NULL,
+ extra_headers,
NULL,
FALSE,
ras->pool) );
Index: subversion/libsvn_ra_dav/ra_dav.h
===================================================================
--- subversion/libsvn_ra_dav/ra_dav.h (revision 14982)
+++ subversion/libsvn_ra_dav/ra_dav.h (working copy)
@@ -143,6 +143,9 @@
/* If <D:error> is returned, here's where the parsed result goes. */
svn_error_t *err;
+ /* Name of operation, possibly declared by client */
+ const char *operation_name;
+
/* A place for allocating fields in this structure. */
apr_pool_t *pool;
};
@@ -394,6 +397,7 @@
int depth,
const char *label,
const ne_propname *which_props,
+ apr_hash_t *extra_headers,
apr_pool_t *pool);
/* fetch a single resource's props from the server. */
@@ -708,6 +712,7 @@
svn_revnum_t *new_rev,
const char **committed_date,
const char **committed_author,
+ svn_ra_session_t *session,
svn_ra_dav__session_t *ras,
const char *repos_url,
const char *activity_url,
Index: subversion/libsvn_ra_dav/session.c
===================================================================
--- subversion/libsvn_ra_dav/session.c (revision 14982)
+++ subversion/libsvn_ra_dav/session.c (working copy)
@@ -970,6 +970,17 @@
}
}
+ /* Possibly add an X-SVN-Operation-Name: header for logging. */
+ if (lrb->operation_name
+ && ((strcmp(lrb->method, "LOCK") == 0)
+ || (strcmp(lrb->method, "UNLOCK") == 0)))
+ {
+ char *hdr = apr_psprintf(lrb->pool, "%s: %s\r\n",
+ SVN_DAV_OPERATION_NAME_HEADER,
+ lrb->operation_name);
+ ne_buffer_zappend(header, hdr);
+ }
+
/* Register a response handler capable of parsing <D:error> */
lrb->error_parser = ne_xml_create();
svn_ra_dav__add_error_handler(req, lrb->error_parser,
@@ -1005,6 +1016,7 @@
const char *comment,
svn_boolean_t force,
svn_revnum_t current_rev,
+ const char *operation_name,
apr_pool_t *pool)
{
svn_ra_dav__session_t *ras = session->priv;
@@ -1026,6 +1038,7 @@
ras->lrb->pool = pool;
ras->lrb->current_rev = current_rev;
ras->lrb->force = force;
+ ras->lrb->operation_name = operation_name;
/* Make a neon lock structure. */
nlock = ne_lock_create();
@@ -1122,7 +1135,8 @@
revnum = val;
err = shim_svn_ra_dav__lock(session, &lock, path, comment,
- force, *revnum, iterpool);
+ force, *revnum,
+ session->operation_name, iterpool);
if (err && !SVN_ERR_IS_LOCK_ERROR (err))
return err;
Index: subversion/libsvn_ra_dav/props.c
===================================================================
--- subversion/libsvn_ra_dav/props.c (revision 14982)
+++ subversion/libsvn_ra_dav/props.c (working copy)
@@ -471,13 +471,16 @@
int depth,
const char *label,
const ne_propname *which_props,
+ apr_hash_t *extra_headers,
apr_pool_t *pool)
{
svn_error_t *err = SVN_NO_ERROR;
propfind_ctx_t pc;
ne_buffer *body;
- apr_hash_t *extra_headers = apr_hash_make(pool);
+ if (extra_headers == NULL)
+ extra_headers = apr_hash_make(pool);
+
/* Add a Depth header. */
if (depth == NE_DEPTH_ZERO)
apr_hash_set(extra_headers, "Depth", 5, "0");
@@ -553,7 +556,7 @@
url_path[len - 1] = '\0';
SVN_ERR( svn_ra_dav__get_props(&props, sess, url_path, NE_DEPTH_ZERO,
- label, which_props, pool) );
+ label, which_props, NULL, pool) );
/* ### HACK. We need to have the client canonicalize paths, get rid
of double slashes and such. This check is just a check against
@@ -1186,6 +1189,7 @@
const char *url = ras->url;
const char *final_url;
apr_hash_t *resources;
+ apr_hash_t *extra_headers = NULL;
apr_hash_index_t *hi;
svn_error_t *err;
@@ -1223,9 +1227,19 @@
pool);
}
+ if (session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ session->operation_name);
+ session->operation_name = NULL;
+ }
+
/* Depth-zero PROPFIND is the One True DAV Way. */
err = svn_ra_dav__get_props(&resources, ras->sess, final_url, NE_DEPTH_ZERO,
- NULL, NULL /* all props */, pool);
+ NULL, NULL /* all props */,
+ extra_headers, pool);
if (err)
{
if (err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND)
Index: subversion/libsvn_ra_dav/commit.c
===================================================================
--- subversion/libsvn_ra_dav/commit.c (revision 14982)
+++ subversion/libsvn_ra_dav/commit.c (working copy)
@@ -96,6 +96,7 @@
typedef struct
{
+ svn_ra_session_t *session;
svn_ra_dav__session_t *ras;
const char *activity_url;
@@ -1427,6 +1428,7 @@
SVN_ERR( svn_ra_dav__merge_activity(&new_rev,
&committed_date,
&committed_author,
+ cc->session,
cc->ras,
cc->ras->root.path,
cc->activity_url,
@@ -1550,6 +1552,7 @@
/* Build the main commit editor's baton. */
cc = apr_pcalloc(pool, sizeof(*cc));
+ cc->session = session;
cc->ras = ras;
cc->valid_targets = apr_hash_make(pool);
cc->get_func = ras->callbacks->get_wc_prop;
Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 14982)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -132,6 +132,10 @@
} dir_item_t;
typedef struct {
+ /* Public session object from the client */
+ svn_ra_session_t *session;
+
+ /* Private ra_dav session object, really just session->priv */
svn_ra_dav__session_t *ras;
apr_file_t *tmpfile;
@@ -391,6 +395,7 @@
svn_ra_get_wc_prop_func_t get_wc_prop,
void *cb_baton,
svn_boolean_t use_base,
+ const char *operation_name,
apr_pool_t *pool)
{
custom_get_ctx_t cgc = { 0 };
@@ -442,6 +447,12 @@
ne_add_request_header(req, SVN_DAV_DELTA_BASE_HEADER, delta_base);
}
+ if (operation_name)
+ {
+ ne_add_request_header(req, SVN_DAV_OPERATION_NAME_HEADER,
+ operation_name);
+ }
+
/* add in a reader to capture the body of the response. */
if (ras->compression)
{
@@ -621,7 +632,7 @@
SVN_ERR( custom_get_request(sess, url, relpath,
fetch_file_reader, &frc,
get_wc_prop, cb_baton,
- TRUE, pool) );
+ TRUE, NULL, pool) );
/* close the handler, since the file reading completed successfully. */
SVN_ERR( (*frc.handler)(NULL, frc.handler_baton) );
@@ -831,7 +842,9 @@
get_file_reader, &fwc,
ras->callbacks->get_wc_prop,
ras->callback_baton,
- FALSE, pool) );
+ FALSE,
+ session->operation_name,
+ pool) );
if (fwc.do_checksum)
{
@@ -903,11 +916,23 @@
if (dirents)
{
+ apr_hash_t *extra_headers = NULL;
+
+ if (session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ session->operation_name);
+ session->operation_name = NULL;
+ }
+
/* Just like Nautilus, Cadaver, or any other browser, we do a
PROPFIND on the directory of depth 1. */
SVN_ERR( svn_ra_dav__get_props(&resources, ras->sess,
final_url, NE_DEPTH_ONE,
- NULL, NULL /* all props */, pool) );
+ NULL, NULL /* all props */,
+ extra_headers, pool) );
/* Count the number of path components in final_url. */
final_url_n_components = svn_path_component_count(final_url);
@@ -1084,11 +1109,21 @@
const char *body;
const char *vcc_url;
svn_error_t *err;
+ apr_hash_t *extra_headers = NULL;
/* Run the 'dated-rev-report' on the VCC url, which is always
guaranteed to exist. */
SVN_ERR (svn_ra_dav__get_vcc(&vcc_url, ras->sess, ras->root.path, pool));
+ /* If available, send a custom header describing the operation. */
+ if (session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ session->operation_name);
+ session->operation_name = NULL;
+ }
body = apr_psprintf(pool,
"<?xml version=\"1.0\" encoding=\"utf-8\"?>"
@@ -1104,7 +1139,9 @@
drev_report_elements,
drev_validate_element,
drev_start_element, drev_end_element,
- revision, NULL, NULL, FALSE, pool);
+ revision, extra_headers,
+ NULL, FALSE, pool);
+
if (err && err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
return svn_error_quick_wrap(err, _("Server does not support date-based "
"operations"));
@@ -1195,7 +1232,18 @@
const char *final_bc_url;
int i;
int status_code = 0;
+ apr_hash_t *extra_headers = NULL;
+ /* If available, send a custom header describing the operation. */
+ if (session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ session->operation_name);
+ session->operation_name = NULL;
+ }
+
*locations = apr_hash_make (pool);
request_body = svn_stringbuf_create("", pool);
@@ -1245,7 +1293,7 @@
err = svn_ra_dav__parsed_request(ras->sess, "REPORT", final_bc_url,
request_body->data, NULL, NULL,
gloc_start_element, NULL, NULL,
- &request_baton, NULL, &status_code,
+ &request_baton, extra_headers, &status_code,
FALSE, pool);
/* Map status 501: Method Not Implemented to our not implemented error.
@@ -1544,7 +1592,18 @@
svn_error_t *err;
int status_code = 0;
get_locks_baton_t baton;
+ apr_hash_t *extra_headers = NULL;
+ /* If available, send a custom header describing the operation. */
+ if (session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ session->operation_name);
+ session->operation_name = NULL;
+ }
+
baton.lock_hash = apr_hash_make (pool);
baton.pool = pool;
baton.scratchpool = svn_pool_create(pool);
@@ -1571,7 +1630,7 @@
getlocks_cdata_handler,
getlocks_end_element,
&baton,
- NULL, /* extra headers */
+ extra_headers,
&status_code,
FALSE,
pool);
@@ -1610,6 +1669,7 @@
svn_ra_dav_resource_t *baseline;
svn_error_t *err;
apr_hash_t *prop_changes = NULL;
+ apr_hash_t *extra_headers = NULL;
apr_array_header_t *prop_deletes = NULL;
static const ne_propname wanted_props[] =
{
@@ -1657,8 +1717,18 @@
(*((const char **) apr_array_push(prop_deletes))) = name;
}
+ /* If available, send a custom header describing the operation. */
+ if (session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ session->operation_name);
+ session->operation_name = NULL;
+ }
+
err = svn_ra_dav__do_proppatch(ras, baseline->url, prop_changes,
- prop_deletes, NULL, pool);
+ prop_deletes, extra_headers, pool);
if (err)
return
svn_error_create
@@ -2050,6 +2120,7 @@
bc_url,
NE_DEPTH_ONE,
NULL, NULL /* allprops */,
+ NULL,
TOP_DIR(rb).pool) );
/* re-index the results into a more usable hash.
@@ -2767,6 +2838,7 @@
svn_error_t *err;
const char *vcc;
int http_status;
+ apr_hash_t *extra_headers = NULL;
#define SVN_RA_DAV__REPORT_TAIL "</S:update-report>" DEBUG_CR
/* write the final closing gunk to our request body. */
@@ -2792,6 +2864,16 @@
return err;
}
+ /* If available, send a custom header describing the operation. */
+ if (rb->session->operation_name != NULL)
+ {
+ extra_headers = apr_hash_make(pool);
+ apr_hash_set(extra_headers, SVN_DAV_OPERATION_NAME_HEADER,
+ APR_HASH_KEY_STRING,
+ rb->session->operation_name);
+ rb->session->operation_name = NULL;
+ }
+
/* dispatch the REPORT. */
err = svn_ra_dav__parsed_request(rb->ras->sess, "REPORT", vcc,
NULL, rb->tmpfile, NULL,
@@ -2799,7 +2881,7 @@
cdata_handler,
end_element,
rb,
- NULL, &http_status,
+ extra_headers, &http_status,
rb->spool_response, rb->ras->pool);
/* we're done with the file */
@@ -2895,6 +2977,7 @@
rb = apr_pcalloc(pool, sizeof(*rb));
rb->ras = ras;
+ rb->session = session;
rb->scratch_pool = svn_pool_create(pool);
rb->editor = editor;
rb->edit_baton = edit_baton;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
> * Ensures consistent logs across RA implementations, because the
> client is declaring, rather than server deducing.
>
>
And what stops the client from saying "I'm reading status", then doing a
checkin? It seems frightfully easy to hack the client so that it lies to
the server, making such logs totally useless at best. When did we decide
to implicitly trust clients?
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Mon, 2005-06-06 at 19:17 -0400, Greg Hudson wrote:
>On Mon, 2005-06-06 at 16:05 -0500, kfogel@collab.net wrote:
>> Greg Hudson <gh...@MIT.EDU> writes:
>> > Over ra_svn, it is quite possible for the server to log what the client
>> > is doing (with the exception that we can't distinguish an update from a
>> > checkout)
>>
>> What about switch too? And diff vs merge?
>>
>> Is 'status -u' distinguishable from update, over ra_svn?
>
>You can tell switch from update, and status -u from update, but probably
>not diff vs. merge.
>
>I guess I don't see the need for server-side logging of stuff that only
>matters to the client. Can you demonstrate a need for that specific
>variety of logging?
Two major use cases are:
1. Gathering usage statistics for reporting.
2. Creation of audit trail for debugging or security analysis.
This variety of logging produces data which is not only useful to end
users for trend tracking and stats, but also very useful to
administrators who need to know how their servers are being utilized (or
abused).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2005-06-06 at 16:05 -0500, kfogel@collab.net wrote:
> Greg Hudson <gh...@MIT.EDU> writes:
> > Over ra_svn, it is quite possible for the server to log what the client
> > is doing (with the exception that we can't distinguish an update from a
> > checkout)
>
> What about switch too? And diff vs merge?
>
> Is 'status -u' distinguishable from update, over ra_svn?
You can tell switch from update, and status -u from update, but probably
not diff vs. merge.
I guess I don't see the need for server-side logging of stuff that only
matters to the client. Can you demonstrate a need for that specific
variety of logging?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> Over ra_svn, it is quite possible for the server to log what the client
> is doing (with the exception that we can't distinguish an update from a
> checkout)
What about switch too? And diff vs merge?
Is 'status -u' distinguishable from update, over ra_svn?
(I could discover the answers by inspection, but I'm away from my
regular computer right now, so I hope you don't mind if I just ask you
here instead.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2005-06-06 at 13:41 -0700, Daniel Rall wrote:
> On Sun, 2005-06-05 at 21:24 -0500, kfogel@collab.net wrote:
> >Greg Hudson <gh...@MIT.EDU> writes:
> >> Well, the obvious criticisms are:
> >>
> >> * Operations from 1.0 and 1.1 clients won't be logged. Having an
> >> incomplete log isn't very satisfying.
> >
> >Yes, but there is no way around that, given the impossibility of
> >deducing operations reliably given the information 1.0 and 1.1 send
> >over the wire.
> >
> >We could add a feature whereby the server has the option to reject
> >clients who don't declare that they intend to abide by the logging
> >protocol. That would address the problem, albeit in a somewhat
> >draconian manner ("Upgrade or you can't use our repository!").
>
> This be handled with a hook script or server-side configuration which
> would either reject or allow -- and log "unknown" or don't log at all --
> operations based on client capabilities (negotiated through HTTP headers
> and ra_svn's capabilities announcement mechanism).
After thinking about this for a while, I think Karl and Ben's proposal
is letting ra_dav drive the core architecture too much.
Over ra_svn, it is quite possible for the server to log what the client
is doing (with the exception that we can't distinguish an update from a
checkout), without being told by the client. And it is much, much
better for the server to log actual client operations than "what the
client told me to log".
So, I think the solution should look something like:
* We make no changes to the ra_svn client.
* svnserve logs the operations it sees from the client.
* ra_dav gets the mechanism Karl and Ben are proposing.
* The HTTP access logs can be used to get more reliable (if less
easily interpreted) information about what clients did.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Sun, 2005-06-05 at 21:24 -0500, kfogel@collab.net wrote:
>Greg Hudson <gh...@MIT.EDU> writes:
>> Well, the obvious criticisms are:
>>
>> * Operations from 1.0 and 1.1 clients won't be logged. Having an
>> incomplete log isn't very satisfying.
>
>Yes, but there is no way around that, given the impossibility of
>deducing operations reliably given the information 1.0 and 1.1 send
>over the wire.
>
>We could add a feature whereby the server has the option to reject
>clients who don't declare that they intend to abide by the logging
>protocol. That would address the problem, albeit in a somewhat
>draconian manner ("Upgrade or you can't use our repository!").
This be handled with a hook script or server-side configuration which
would either reject or allow -- and log "unknown" or don't log at all --
operations based on client capabilities (negotiated through HTTP headers
and ra_svn's capabilities announcement mechanism).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> Well, the obvious criticisms are:
>
> * Operations from 1.0 and 1.1 clients won't be logged. Having an
> incomplete log isn't very satisfying.
Yes, but there is no way around that, given the impossibility of
deducing operations reliably given the information 1.0 and 1.1 send
over the wire.
We could add a feature whereby the server has the option to reject
clients who don't declare that they intend to abide by the logging
protocol. That would address the problem, albeit in a somewhat
draconian manner ("Upgrade or you can't use our repository!").
> * You haven't solved the problem of how to log, say, a commit over DAV
> once. You'll wind up logging it once for each HTTP operation involved
> in the commit. For your patch this is no problem, since you're just
> annotating the pre-existing HTTP log information, but for a generic
> logging mechanism your proposal seems incomplete.
Yes, we have solved that problem, that's the whole point :-). The
logging is per client operation, not per HTTP operation. Each commit
gets logged once, no matter how many paths / HTTP operations were
involved.
(Well, actually there's a bug whereby each commit gets logged twice,
the second time with an empty path, but that's irrelevant here; Ben
and I already know about it and will of course fix it.)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2005-06-06 at 11:33 -0700, Daniel L. Rall wrote:
> HTTP provides a good way to do capability announcement through its
> Accept* headers. Applying this strategy to ra_dav is straightforward.
> Application to ra_svn might vary, since it uses a statefull protocol.
ra_svn already has a capabilities mechanism.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by "Daniel L. Rall" <dl...@finemaltcoding.com>.
On Mon, 2005-06-06 at 09:25 -0400, John Peacock wrote:
> Erik Huelsmann wrote:
> > It's not about versions: it's about capabilities. You can't know in
> > advance all versions of all clients-to-be-developed and the versions
> > in which they will add certain capabilities.
> >
> > In other words, I'm against adding the version number to the stream.
>
> I wasn't advocating version numbers either, merely that there could be a
> good reason to add some client->server communication for the server to
> make better decisions about serving that particular client. I agree
> that a capabilities listing (much like SMTP et al use) is a much better
> way to handle it. However, the rub comes in defining those capabilities
> in a consistent yet extensible way.
HTTP provides a good way to do capability announcement through its
Accept* headers. Applying this strategy to ra_dav is straightforward.
Application to ra_svn might vary, since it uses a statefull protocol.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by John Peacock <jp...@rowman.com>.
Erik Huelsmann wrote:
> It's not about versions: it's about capabilities. You can't know in
> advance all versions of all clients-to-be-developed and the versions
> in which they will add certain capabilities.
>
> In other words, I'm against adding the version number to the stream.
I wasn't advocating version numbers either, merely that there could be a
good reason to add some client->server communication for the server to
make better decisions about serving that particular client. I agree
that a capabilities listing (much like SMTP et al use) is a much better
way to handle it. However, the rub comes in defining those capabilities
in a consistent yet extensible way.
John
--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5748
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by Erik Huelsmann <eh...@gmail.com>.
On 6/6/05, John Peacock <jp...@rowman.com> wrote:
> steven.higgan wrote:
> > Then what about (re)considering the proposal to provide client API details
> > (as in version numbers) as a part of the mass of information that gets sent
> > to the server.
>
> That still doesn't help this issue; the old clients will not send the new
> version numbers, etc., so the server won't have anything to go on. For future
> growth, however, there might be some utility in adding this info to the stream...
It's not about versions: it's about capabilities. You can't know in
advance all versions of all clients-to-be-developed and the versions
in which they will add certain capabilities.
In other words, I'm against adding the version number to the stream.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by John Peacock <jp...@rowman.com>.
steven.higgan wrote:
> Then what about (re)considering the proposal to provide client API details
> (as in version numbers) as a part of the mass of information that gets sent
> to the server.
That still doesn't help this issue; the old clients will not send the new
version numbers, etc., so the server won't have anything to go on. For future
growth, however, there might be some utility in adding this info to the stream...
John
--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4720 Boston Way
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5747
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] operation logging
Posted by "steven.higgan" <st...@orcon.net.nz>.
Greg Hudson Wrote :
> * Operations from 1.0 and 1.1 clients won't be logged. Having an
> incomplete log isn't very satisfying.
Then what about (re)considering the proposal to provide client API details
(as in version numbers) as a part of the mass of information that gets sent
to the server.
Providing sufficient hook the server admin can decide whether to 'put up'
with this lack of logging information (allowing the old client) or refusing
the transaction based on the user-agent
---------------------------------------------------------------------------
Steven Higgan - 3rd Year B.I.T. Otago Polytechnic, Dunedin New Zealand
.net geek
-----Original Message-----
From: Greg Hudson [mailto:ghudson@MIT.EDU]
Sent: Monday, 6 June 2005 1:51 p.m.
To: kfogel@collab.net
Cc: dev@subversion.tigris.org
Subject: Re: [PATCH] operation logging
Well, the obvious criticisms are:
* Operations from 1.0 and 1.1 clients won't be logged. Having an
incomplete log isn't very satisfying.
* You haven't solved the problem of how to log, say, a commit over DAV
once. You'll wind up logging it once for each HTTP operation involved
in the commit. For your patch this is no problem, since you're just
annotating the pre-existing HTTP log information, but for a generic
logging mechanism your proposal seems incomplete.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
--
No virus found in this incoming message.
Checked by AVG Anti-Virus.
Version: 7.0.323 / Virus Database: 267.6.2 - Release Date: 4/06/2005
--
No virus found in this outgoing message.
Checked by AVG Anti-Virus.
Version: 7.0.323 / Virus Database: 267.6.2 - Release Date: 4/06/2005
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] operation logging
Posted by Greg Hudson <gh...@MIT.EDU>.
Well, the obvious criticisms are:
* Operations from 1.0 and 1.1 clients won't be logged. Having an
incomplete log isn't very satisfying.
* You haven't solved the problem of how to log, say, a commit over DAV
once. You'll wind up logging it once for each HTTP operation involved
in the commit. For your patch this is no problem, since you're just
annotating the pre-existing HTTP log information, but for a generic
logging mechanism your proposal seems incomplete.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org