Hi!

Why doesn't svn_ra_do_diff3() request that the repository sends copyfrom
args? The best explanation I've found is this commit message:

------------------------------------------------------------------------
r866577 | sussman | 2007-09-10 06:56:55 +0200 (mån, 10 sep 2007) | 87 lines

Send copyfrom-args on updates *only* if the client explicitly requests them.

This keeps the 1.5 servers backward-compatible.  Because incoming
copyfrom args cause old clients to error, they should only be
requested by new clients that understand them.  This means adding a
new boolean 'send_copyfrom_args' to the main libsvn_repos
editor-driver, svn_ra_do_update2(), and all four RA implementations.
Eesh!

[.. Rest of log message omitted for brevity ..]

I'm assuming noone saw a use case for having diff display copies back then and
since it involved passing down a lot of parameters, only svn_ra_do_updateX()
was revved. But with git diffs we have a use case for copyfrom info. How about
enable it? There's a WIP patch attached to this mail.  I haven't done any 
wrapper
for svn_ra_do_diff3() though. A log message:

[[[
Make the diff code able to supply copyfrom information even when one of
the targets is an URL. Involves passing down a 'send_copyfrom_args' to
all RA implemtations.

* subversion/libsvn_ra/ra_loader.h
  (svn_ra__vtable_t): Add 'send_copyfrom_args' parameter.

* subversion/libsvn_ra_local/ra_plugin.c
  (svn_ra_local__do_diff): Add 'send_copyfrom_args' parameter.

* subversion/include/svn_ra.h
  (svn_ra_do_diff4): New.
  (svn_ra_do_diff3): Deprecate.

* subversion/libsvn_ra_neon/fetch.c
  (svn_ra_neon__do_diff): Add 'send_copyfrom_args' parameter.

* subversion/libsvn_ra_serf/update.c
  (svn_ra_serf__do_diff): Add 'send_copyfrom_args' parameter.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__do_diff): Add 'send_copyfrom_args' parameter.

* subversion/svnserve/serve.c
  (diff): Parse the parameters for send_copyfrom_param.
]]]

Does anyone have any objections to enable copyfrom info for URL->URL && 
URL->WC diffs?

Thanks,
Daniel
Index: subversion/libsvn_ra/ra_loader.h
===================================================================
--- subversion/libsvn_ra/ra_loader.h    (revision 985618)
+++ subversion/libsvn_ra/ra_loader.h    (arbetskopia)
@@ -155,6 +155,7 @@ typedef struct svn_ra__vtable_t {
                           svn_revnum_t revision,
                           const char *diff_target,
                           svn_depth_t depth,
+                          svn_boolean_t send_copyfrom_args,
                           svn_boolean_t ignore_ancestry,
                           svn_boolean_t text_deltas,
                           const char *versus_url,
Index: subversion/libsvn_ra_local/ra_plugin.c
===================================================================
--- subversion/libsvn_ra_local/ra_plugin.c      (revision 985618)
+++ subversion/libsvn_ra_local/ra_plugin.c      (arbetskopia)
@@ -810,6 +810,7 @@ svn_ra_local__do_diff(svn_ra_session_t *session,
                       svn_revnum_t update_revision,
                       const char *update_target,
                       svn_depth_t depth,
+                      svn_boolean_t send_copyfrom_args,
                       svn_boolean_t ignore_ancestry,
                       svn_boolean_t text_deltas,
                       const char *switch_url,
@@ -825,7 +826,7 @@ svn_ra_local__do_diff(svn_ra_session_t *session,
                        switch_url,
                        text_deltas,
                        depth,
-                       FALSE,
+                       send_copyfrom_args,
                        ignore_ancestry,
                        update_editor,
                        update_baton,
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 985618)
+++ subversion/include/svn_ra.h (arbetskopia)
@@ -1291,9 +1291,29 @@ svn_ra_do_status(svn_ra_session_t *session,
  * needed, and sending too much data back, a pre-1.5 'recurse'
  * directive may be sent to the server, based on @a depth.
  *
- * @since New in 1.5.
+ * @since New in 1.7.
  */
 svn_error_t *
+svn_ra_do_diff4(svn_ra_session_t *session,
+                const svn_ra_reporter3_t **reporter,
+                void **report_baton,
+                svn_revnum_t revision,
+                const char *diff_target,
+                svn_depth_t depth,
+                svn_boolean_t send_copyfrom_args,
+                svn_boolean_t ignore_ancestry,
+                svn_boolean_t text_deltas,
+                const char *versus_url,
+                const svn_delta_editor_t *diff_editor,
+                void *diff_baton,
+                apr_pool_t *pool);
+/**
+ * Similar to svn_ra_do_diff4(), but with @c send_copyfrom_args set to
+ * FALSE.
+ *
+ * @deprecated Provided for compatibility with the 1.5 API.
+ */
+svn_error_t *
 svn_ra_do_diff3(svn_ra_session_t *session,
                 const svn_ra_reporter3_t **reporter,
                 void **report_baton,
@@ -1306,7 +1326,6 @@ svn_ra_do_diff3(svn_ra_session_t *session,
                 const svn_delta_editor_t *diff_editor,
                 void *diff_baton,
                 apr_pool_t *pool);
-
 /**
  * Similar to svn_ra_do_diff3(), but taking @c svn_ra_reporter2_t
  * instead of @c svn_ra_reporter3_t, and therefore only able to report
Index: subversion/libsvn_ra_neon/fetch.c
===================================================================
--- subversion/libsvn_ra_neon/fetch.c   (revision 985618)
+++ subversion/libsvn_ra_neon/fetch.c   (arbetskopia)
@@ -2792,6 +2792,7 @@ svn_error_t * svn_ra_neon__do_diff(svn_ra_session_
                                    svn_revnum_t revision,
                                    const char *diff_target,
                                    svn_depth_t depth,
+                                   svn_boolean_t send_copyfrom_args,
                                    svn_boolean_t ignore_ancestry,
                                    svn_boolean_t text_deltas,
                                    const char *versus_url,
@@ -2806,7 +2807,7 @@ svn_error_t * svn_ra_neon__do_diff(svn_ra_session_
                        diff_target,
                        versus_url,
                        depth,
-                       FALSE,
+                       send_copyfrom_args,
                        ignore_ancestry,
                        FALSE,
                        wc_diff,
Index: subversion/libsvn_ra_serf/update.c
===================================================================
--- subversion/libsvn_ra_serf/update.c  (revision 985618)
+++ subversion/libsvn_ra_serf/update.c  (arbetskopia)
@@ -2626,6 +2626,7 @@ svn_ra_serf__do_diff(svn_ra_session_t *ra_session,
                      svn_revnum_t revision,
                      const char *diff_target,
                      svn_depth_t depth,
+                     svn_boolean_t send_copyfrom_args,
                      svn_boolean_t ignore_ancestry,
                      svn_boolean_t text_deltas,
                      const char *versus_url,
@@ -2638,8 +2639,9 @@ svn_ra_serf__do_diff(svn_ra_session_t *ra_session,
   return make_update_reporter(ra_session, reporter, report_baton,
                               revision,
                               session->repos_url.path, versus_url, diff_target,
-                              depth, ignore_ancestry, text_deltas, FALSE,
-                              diff_editor, diff_baton, pool);
+                              depth, ignore_ancestry, text_deltas, 
+                              send_copyfrom_args, diff_editor, diff_baton, 
+                              pool);
 }
 
 svn_error_t *
Index: subversion/libsvn_ra_serf/ra_serf.h
===================================================================
--- subversion/libsvn_ra_serf/ra_serf.h (revision 985618)
+++ subversion/libsvn_ra_serf/ra_serf.h (arbetskopia)
@@ -1231,6 +1231,7 @@ svn_ra_serf__do_diff(svn_ra_session_t *session,
                      svn_revnum_t revision,
                      const char *diff_target,
                      svn_depth_t depth,
+                     svn_boolean_t send_copyfrom_args,
                      svn_boolean_t ignore_ancestry,
                      svn_boolean_t text_deltas,
                      const char *versus_url,
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 985618)
+++ subversion/svnserve/serve.c (arbetskopia)
@@ -1670,6 +1670,8 @@ static svn_error_t *diff(svn_ra_svn_conn_t *conn,
   /* Default to unknown.  Old clients won't send depth, but we'll
      handle that by converting recurse if necessary. */
   svn_depth_t depth = svn_depth_unknown;
+  apr_uint64_t send_copyfrom_param;
+  svn_boolean_t send_copyfrom_args;
 
   /* Parse the arguments. */
   if (params->nelts == 5)
@@ -1682,10 +1684,14 @@ static svn_error_t *diff(svn_ra_svn_conn_t *conn,
     }
   else
     {
-      SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbcb?w",
+      /* ### I'm only duplicating the logic in update() for parsing
+       * ### the send_copyfrom_param. Find out how the svn protocol works.
+       */
+      SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbcb?wB",
                                      &rev, &target, &recurse,
                                      &ignore_ancestry, &versus_url,
-                                     &text_deltas, &depth_word));
+                                     &text_deltas, &depth_word, 
+                                     &send_copyfrom_param));
     }
   target = svn_uri_canonicalize(target, pool);
   versus_url = svn_uri_canonicalize(versus_url, pool);
@@ -1695,6 +1701,9 @@ static svn_error_t *diff(svn_ra_svn_conn_t *conn,
   else
     depth = SVN_DEPTH_INFINITY_OR_FILES(recurse);
 
+  send_copyfrom_args = (send_copyfrom_param == SVN_RA_SVN_UNSPECIFIED_NUMBER) ?
+      FALSE : (svn_boolean_t) send_copyfrom_param;
+
   SVN_ERR(trivial_auth_request(conn, pool, b));
 
   if (!SVN_IS_VALID_REVNUM(rev))
@@ -1708,7 +1717,8 @@ static svn_error_t *diff(svn_ra_svn_conn_t *conn,
     svn_revnum_t from_rev;
     SVN_ERR(accept_report(NULL, &from_rev,
                           conn, pool, b, rev, target, versus_path,
-                          text_deltas, depth, FALSE, ignore_ancestry));
+                          text_deltas, depth, send_copyfrom_args, 
+                          ignore_ancestry));
     SVN_ERR(log_command(b, conn, pool, "%s",
                         svn_log__diff(full_path, from_rev, versus_path,
                                       rev, depth, ignore_ancestry,

Reply via email to