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

Re: mod-dav-svn high level logging compatibility

From: Eric Gillespie <epg_at_pretzelnet.org>
Date: 2007-12-21 00:09:50 CET

Eric Gillespie <epg@pretzelnet.org> writes:

> - get-mergeinfo(-partial)? '/PATH'
>
> Not present in 1.4.
>
> Shouldn't we log more than the path? svn_ra_get_mergeinfo also
> takes svn_revnum_t and svn_mergeinfo_inheritance_t but they are
> not logged. Also, I don't understand the meaning of "partial";
> it looks like it's present if the caller asked for mergeinfo
> about multiple paths, but I don't see the leap from that to the
> word "partial".

Danny? You added this in r20728, and it basically hasn't changed
since then. Hmm, actually, the code seems to be dead:

0 cmdline% grep -c get-mergeinfo ops
1528
0 cmdline% grep -c get-mergeinfo-partial ops
0

Furthermore, nothing ever seems to call get_mergeinfo with
len(paths)!=1 or with inherit=FALSE. If there's really no need
to call it except with 1 path and inherit=TRUE, maybe we should
change the interface. But maybe this is all changing with all
the revamping mergeinfo is going through...

How about the patch below? Log lines look like:
[20/Dec/2007:15:02:00 -0800] jrandom merge_tests-2 get-mergeinfo (/A/C) inherit

Thanks.

------------------------------------------------------------------------
r20728 | dberlin | 2006-07-18 12:42:41 -0700 (Tue, 18 Jul 2006) | 36 lines

Make svn_ra_get_mergeinfo work over DAV, and fix bindings
so svn_ra_get_mergeinfo and svn_repos_fs_get_mergeinfo work.

(This was done for testing the DAV stuff, which is why it was part
 of this change)

* subversion/include/svn_repos.h
(svn_repos_fs_get_mergeinfo): Rename parameter for consistency.

* subversion/include/svn_ra.h
(svn_ra_get_merge_info): Ditto.

* subversion/bindings/swig/svn_ra.i: Apply MERGEHASHHASH to mergeoutput.

* subversion/bindings/swig/svn_repos.i: Apply MERGEHASH to mergeoutput.

* subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
(convert_mergeinfo_hash): New function.
(svn_swig_py_mergeinfo_hash_to_dict): Ditto.

* subversion/bindings/swig/include/svn_types.swg: Add typemap MERGEHASHASH.

* subversion/mod_dav_svn/dav_svn.h
(dav_svn__get_merge_info_report): New prototype.

* subversion/mod_dav_svn/mergeinfo.c: New file.

* subversion/mod_dav_svn/version.c: New file.

* subversion/libsvn_ra_dav/ra_dav.h: Add merge_info elements.

* subversion/libsvn_ra_dav/fetch.c: Remove svn_ra_dav__get_mergeinfo.

* subversion/libsvn_ra_dav/mergeinfo.c: New function
------------------------------------------------------------------------

Index: mergeinfo.c
===================================================================
--- mergeinfo.c (revision 28612)
+++ mergeinfo.c (working copy)
@@ -57,6 +57,9 @@
   svn_mergeinfo_inheritance_t inherit = svn_mergeinfo_explicit;
   apr_array_header_t *paths
     = apr_array_make(resource->pool, 0, sizeof(const char *));
+ /* for high-level logging */
+ svn_stringbuf_t *space_separated_paths =
+ svn_stringbuf_create("", resource->pool);
 
   /* Sanity check. */
   ns = dav_svn__find_ns(doc->namespaces, SVN_XML_NAMESPACE);
@@ -92,6 +95,13 @@
           target = svn_path_join(resource->info->repos_path, rel_path,
                                  resource->pool);
           (*((const char **)(apr_array_push(paths)))) = target;
+ /* Gather a formatted list of paths to include in our
+ operational logging. */
+ if (space_separated_paths->len > 1)
+ svn_stringbuf_appendcstr(space_separated_paths, " ");
+ svn_stringbuf_appendcstr(space_separated_paths,
+ svn_path_uri_encode(target,
+ resource->pool));
         }
       /* else unknown element; skip it */
     }
@@ -174,19 +184,9 @@
  cleanup:
 
   /* We've detected a 'high level' svn action to log. */
- if (paths->nelts == 0)
- action = "get-mergeinfo";
- else if (paths->nelts == 1)
- action = apr_psprintf(resource->pool, "get-mergeinfo %s",
- svn_path_uri_encode(APR_ARRAY_IDX
- (paths, 0, const char *),
- resource->pool));
- else
- action = apr_psprintf(resource->pool, "get-mergeinfo-partial %s",
- svn_path_uri_encode(APR_ARRAY_IDX
- (paths, 0, const char *),
- resource->pool));
-
+ action = apr_psprintf(resource->pool, "get-mergeinfo (%s)%s",
+ space_separated_paths->data,
+ inherit ? " inherit" : "");
   dav_svn__operational_log(resource->info, action);
 
   /* Flush the contents of the brigade (returning an error only if we

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Dec 21 00:10:03 2007

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