You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2006/01/14 22:15:46 UTC

[PATCH][Issue 2287] svn_client_log should take a peg revision

On Sat, 14 Jan 2006, Peter N. Lundblad wrote:

> I also find it easier (and more likely:-) to review patches sent to dev@
> and it is easier to reply with comments inline as well.
 
Here's what I attached to the issue (which pass log_tests.py), with a
change log.  I'll pick it back up on Monday.  Also, a few issues:

o The previous log function is passing its "end" parameter for
"peg_revision" (as done by svn_client_blame), but the doc string for
svn_client__open_ra_session_from_path makes me think that perhaps
svn_opt_revision_unspecified might be more appropriate?

o In the new call to svn_client__open_ra_session_from_path, the local
variable session_url is never used after being set.  What should be
done with it (e.g. replace base_url)?  Similarly, end_revnum is used,
but perhaps no longer exactly right.

o Peter suggested leaving svn_client_log() calling svn_client_log2().


[[[
* subversion/libsvn_client/log.c
  (svn_client_log3): New function based on the previous incarnation of
   svn_client_log2() which accepts a peg revision argument.

  (svn_client_log2): Delegate to svn_client_log3(), passing "end" for
   the "peg_revision" parameter.

  (svn_client_log): Delegate to svn_client_log3() instead of
   svn_client_log2() (in the same fashion as that function).


* subversion/include/svn_client.h
  (svn_client_log3): New declaration based on the previous incarnation
   of svn_client_log2() which accepts a peg revision argument.

  (svn_client_log2): Deprecated, and adjusted doc string.
]]]



Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c	(revision 18103)
+++ subversion/libsvn_client/log.c	(working copy)
@@ -45,7 +45,8 @@
 
 
 svn_error_t *
-svn_client_log2 (const apr_array_header_t *targets,
+svn_client_log3 (const apr_array_header_t *targets,
+                 const svn_opt_revision_t *peg_revision,
                  const svn_opt_revision_t *start,
                  const svn_opt_revision_t *end,
                  int limit,
@@ -58,7 +59,7 @@
 {
   svn_ra_session_t *ra_session;
   const char *path;
-  const char *base_url;
+  const char *base_url, *session_url;
   const char *base_name = NULL;
   apr_array_header_t *condensed_targets;
   svn_revnum_t start_revnum, end_revnum;
@@ -159,11 +160,20 @@
 
   /* Open a repository session to the BASE_URL. */
   SVN_ERR (svn_path_condense_targets (&base_name, NULL, targets, TRUE, pool)); 
-  SVN_ERR (svn_client__open_ra_session_internal (&ra_session, base_url, 
-                                                 base_name, NULL, NULL,
-                                                 (NULL != base_name), TRUE, 
-                                                 ctx, pool));
+  /* ### This function duplicates some, but not all, of what
+     ### svn_client__ra_session_from_path() does.  Some of that
+     ### processing should be removed from this function to avoid
+     ### extra round-trips. Compare'em and see what needs to be
+     ### changed.
 
+     ### what are we going to do with session_url?
+
+     ### end_revnum shouldn't be set twice
+  */
+  SVN_ERR (svn_client__ra_session_from_path (&ra_session, &end_revnum,
+                                             &session_url, base_name,
+                                             peg_revision, end, ctx, pool));
+
   /* It's a bit complex to correctly handle the special revision words
    * such as "BASE", "COMMITTED", and "PREV".  For example, if the
    * user runs
@@ -194,7 +204,7 @@
       SVN_ERR (svn_client__get_revision_number
                (&start_revnum, ra_session, start, base_name, pool));
 
-    if (! end_is_local)
+    if (! end_is_local && end_revnum != SVN_INVALID_REVNUM)
       SVN_ERR (svn_client__get_revision_number
                (&end_revnum, ra_session, end, base_name, pool));
 
@@ -270,6 +280,23 @@
 }
 
 svn_error_t *
+svn_client_log2 (const apr_array_header_t *targets,
+                 const svn_opt_revision_t *start,
+                 const svn_opt_revision_t *end,
+                 int limit,
+                 svn_boolean_t discover_changed_paths,
+                 svn_boolean_t strict_node_history,
+                 svn_log_message_receiver_t receiver,
+                 void *receiver_baton,
+                 svn_client_ctx_t *ctx,
+                 apr_pool_t *pool)
+{
+  return svn_client_log3 (targets, end, start, end, limit,
+                          discover_changed_paths, strict_node_history,
+                          receiver, receiver_baton, ctx, pool);
+}
+
+svn_error_t *
 svn_client_log (const apr_array_header_t *targets,
                 const svn_opt_revision_t *start,
                 const svn_opt_revision_t *end,
@@ -282,7 +309,7 @@
 {
   svn_error_t *err = SVN_NO_ERROR;
 
-  err = svn_client_log2 (targets, start, end, 0, discover_changed_paths,
+  err = svn_client_log3 (targets, end, start, end, 0, discover_changed_paths,
                          strict_node_history, receiver, receiver_baton, ctx,
                          pool);
     
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h	(revision 18103)
+++ subversion/include/svn_client.h	(working copy)
@@ -1177,7 +1177,8 @@
  * for which log messages are desired.  The repository info is
  * determined by taking the common prefix of the target entries' URLs.
  * @a receiver is invoked only on messages whose revisions involved a
- * change to some path in @a targets.
+ * change to some path in @a targets.  @a peg_revision indicates in
+ * which revision @a targets are valid.
  *
  * If @a limit is non-zero only invoke @a receiver on the first @a limit
  * logs.
@@ -1202,6 +1203,27 @@
  * If @a ctx->notify_func2 is non-null, then call @a ctx->notify_func2/baton2
  * with a 'skip' signal on any unversioned targets.
  *
+ * @since New in 1.4.
+ */
+svn_error_t *
+svn_client_log3 (const apr_array_header_t *targets,
+                 const svn_opt_revision_t *peg_revision,
+                 const svn_opt_revision_t *start,
+                 const svn_opt_revision_t *end,
+                 int limit,
+                 svn_boolean_t discover_changed_paths,
+                 svn_boolean_t strict_node_history,
+                 svn_log_message_receiver_t receiver,
+                 void *receiver_baton,
+                 svn_client_ctx_t *ctx,
+                 apr_pool_t *pool);
+
+
+/**
+ * Similar to svn_client_log3(), but with the @a peg_revision
+ * parameter set to @c end.
+ *
+ * @deprecated Provided for backward compatibility with the 1.2 API.
  * @since New in 1.2.
  */
 svn_error_t *


...
> I reviewed and S.Ramaswamy posted another patch with some good stuff
> in it. Did you look at it?  (I haven't looked at your latest patch,
> so I don't know.)

No, I didn't see it.  Way back on Jun 29 -- okay, I'll take a look, thanks.
-- 

Daniel Rall