Hi Neels.

The new diff --summarize code presently still downloads the full text of
each deleted file to supply to the diff callbacks.  I have a patch in
progress (attached) to eliminate this, but I shan't be working on it for
a few days.  With this patch, 'diff --summarize' still works but I
haven't checked carefully (either by testing or analysis) that this is
actually eliminating all the unnecessary downloads, and it could do with
some documentation updates in the diff callbacks to say that this usage
is allowed.

Feel free to progress this if you wish, otherwise I'll get back to it
later.

- Julian

In 'diff --summarize', don't download full texts for deleted files, because
that takes time and we don't use them.
### Patch in progress.

* subversion/libsvn_client/repos_diff.c
  (edit_baton): Attempt to clarify the documentation of 'target'.
  (diff_deleted_dir, delete_entry): Don't fetch the old file if we don't
    need it.
  (apply_textdelta): If we're not expecting to receive text deltas, use the
    'noop' window handler and indicate that there was a text change even
    though we don't have the text.

* subversion/libsvn_client/repos_diff_summarize.c
  (dbg, DBG): ### Debugging.
  (cb_dir_deleted, cb_file_deleted, cb_dir_added, cb_dir_opened,
   cb_dir_closed, cb_file_added, cb_file_opened, cb_file_changed,
   cb_dir_props_changed): ### Debugging.
--This line, and those below, will be ignored--

Index: subversion/libsvn_client/repos_diff.c
===================================================================
--- subversion/libsvn_client/repos_diff.c	(revision 1160207)
+++ subversion/libsvn_client/repos_diff.c	(working copy)
@@ -51,8 +51,9 @@
 
 /* Overall crawler editor baton.  */
 struct edit_baton {
-  /* TARGET is a working-copy directory which corresponds to the base
-     URL open in RA_SESSION below. */
+  /* TARGET is the 'anchor' directory path, (supposedly in the WC?), which corresponds
+     to the base URL open in RA_SESSION below.  Paths passed into the
+     editor functions are considered to be relative to this path. */
   const char *target;
 
   /* A working copy context for TARGET, NULL if this is purely a
@@ -548,11 +549,14 @@ diff_deleted_dir(const char *dir,
 
           /* Compare a file being deleted against an empty file */
           b = make_file_baton(path, FALSE, eb, iterpool);
-          SVN_ERR(get_file_from_ra(b, FALSE, iterpool));
+          if (eb->text_deltas)
+            {
+              SVN_ERR(get_file_from_ra(b, FALSE, iterpool));
 
-          SVN_ERR(get_empty_file(b->edit_baton, &(b->path_end_revision)));
+              SVN_ERR(get_empty_file(b->edit_baton, &(b->path_end_revision)));
 
-          get_file_mime_types(&mimetype1, &mimetype2, b);
+              get_file_mime_types(&mimetype1, &mimetype2, b);
+            }
 
           SVN_ERR(eb->diff_callbacks->file_deleted(
                                 NULL, NULL, b->wcpath,
@@ -620,10 +624,13 @@ delete_entry(const char *path,
 
         /* Compare a file being deleted against an empty file */
         b = make_file_baton(path, FALSE, eb, scratch_pool);
-        SVN_ERR(get_file_from_ra(b, FALSE, scratch_pool));
-        SVN_ERR(get_empty_file(b->edit_baton, &(b->path_end_revision)));
+        if (eb->text_deltas)
+          {
+            SVN_ERR(get_file_from_ra(b, FALSE, scratch_pool));
+            SVN_ERR(get_empty_file(b->edit_baton, &(b->path_end_revision)));
 
-        get_file_mime_types(&mimetype1, &mimetype2, b);
+            get_file_mime_types(&mimetype1, &mimetype2, b);
+          }
 
         SVN_ERR(eb->diff_callbacks->file_deleted(
                      &state, &tree_conflicted, b->wcpath,
@@ -911,6 +918,19 @@ apply_textdelta(void *file_baton,
       return SVN_NO_ERROR;
     }
 
+  /* If we're not sending file text, then ignore any that we receive. */
+  if (! b->edit_baton->text_deltas)
+    {
+      /* Set temp file paths to non-null to indicate there is a text change. */
+      b->path_start_revision = "";
+      b->path_end_revision = "";
+
+      *handler = svn_delta_noop_window_handler;
+      *handler_baton = NULL;
+
+      return SVN_NO_ERROR;
+    }
+
   /* We need the expected pristine file, so go get it */
   if (!b->added)
     SVN_ERR(get_file_from_ra(b, FALSE, scratch_pool));
Index: subversion/libsvn_client/repos_diff_summarize.c
===================================================================
--- subversion/libsvn_client/repos_diff_summarize.c	(revision 1160207)
+++ subversion/libsvn_client/repos_diff_summarize.c	(working copy)
@@ -30,6 +30,14 @@
 #include "client.h"
 
 
+static void
+dbg(const char *func, const char *word, const char *path)
+{
+  SVN_DBG(("%10s '%s'\n", word, path));
+}
+#define DBG(w, path) dbg(__func__, #w, path)
+
+
 /* Diff callbacks baton.  */
 struct summarize_baton_t {
   /* The target path of the diff, relative to the anchor; "" if target == anchor. */
@@ -96,6 +104,7 @@ cb_dir_deleted(svn_wc_notify_state_t *st
 {
   struct summarize_baton_t *b = diff_baton;
 
+  DBG(dir_del, path);
   SVN_ERR(send_summary(b, path, svn_client_diff_summarize_kind_deleted,
                        FALSE, svn_node_dir, scratch_pool));
 
@@ -120,6 +129,7 @@ cb_file_deleted(svn_wc_notify_state_t *s
 {
   struct summarize_baton_t *b = diff_baton;
 
+  DBG(file_del, path);
   SVN_ERR(send_summary(b, path, svn_client_diff_summarize_kind_deleted,
                        FALSE, svn_node_file, scratch_pool));
 
@@ -142,6 +152,7 @@ cb_dir_added(svn_wc_notify_state_t *stat
              void *diff_baton,
              apr_pool_t *scratch_pool)
 {
+  DBG(dir_add, path);
   if (tree_conflicted)
     *tree_conflicted = FALSE;
   if (skip)
@@ -160,6 +171,7 @@ cb_dir_opened(svn_boolean_t *tree_confli
               void *diff_baton,
               apr_pool_t *scratch_pool)
 {
+  DBG(dir_open, path);
   if (tree_conflicted)
     *tree_conflicted = FALSE;
   if (skip)
@@ -181,6 +193,7 @@ cb_dir_closed(svn_wc_notify_state_t *con
   struct summarize_baton_t *b = diff_baton;
   svn_boolean_t prop_change;
 
+  DBG(dir_close, path);
   prop_change = apr_hash_get(b->prop_changes, path, APR_HASH_KEY_STRING) != NULL;
   if (dir_was_added || prop_change)
     SVN_ERR(send_summary(b, path,
@@ -217,6 +230,7 @@ cb_file_added(svn_wc_notify_state_t *con
 {
   struct summarize_baton_t *b = diff_baton;
 
+  DBG(file_add, path);
   SVN_ERR(send_summary(b, path, svn_client_diff_summarize_kind_added,
                        props_changed(propchanges, scratch_pool),
                        svn_node_file, scratch_pool));
@@ -238,6 +252,7 @@ cb_file_opened(svn_boolean_t *tree_confl
                void *diff_baton,
                apr_pool_t *scratch_pool)
 {
+  DBG(file_open, path);
   if (tree_conflicted)
     *tree_conflicted = FALSE;
   if (skip)
@@ -264,6 +279,7 @@ cb_file_changed(svn_wc_notify_state_t *c
   struct summarize_baton_t *b = diff_baton;
   svn_boolean_t text_change = (tmpfile2 != NULL);
 
+  DBG(file_mod, path);
   SVN_ERR(send_summary(b, path,
                        text_change ? svn_client_diff_summarize_kind_modified
                                    : svn_client_diff_summarize_kind_normal,
@@ -291,6 +307,7 @@ cb_dir_props_changed(svn_wc_notify_state
 {
   struct summarize_baton_t *b = diff_baton;
 
+  DBG(dir_prop, path);
   if (props_changed(propchanges, scratch_pool))
     apr_hash_set(b->prop_changes, path, APR_HASH_KEY_STRING, path);
 

Reply via email to