Hi!

[[[ 
Do not display a binary property diff in the interactive conflict
resolver. 

* subversion/include/svn_io.h
  (svn_stream_detect_binary_mimetype): New. Declare.

* subversion/libsvn_subr/stream.c
  (svn_stream_detect_binary_mimetype): Check if atleast 85 % of the
    bytes fall within the ascii range of printable characters.

* subversion/libsvn_wc/props.c
  (maybe_generate_propconflict): Call ..binary_mimetype() on the
    mergestream. Set the conflict description according to the outcome.

* subversion/svn/conflict-callbacks.c
  (svn_cl__conflict_handler): Print err msg if desc->is_binary when
    invoking 'diff-full'.

Patch by: Daniel Näslund <daniel{_AT_}longitudo.com> 
]]]

A recent thread started by Mike Samuel [1] discussed various strategies
for improving the detection of 'mergeability'. This patch only uses
existing solutions. I've taken a piece of code from
svn_io_detect_mimetype2() for detecting if a file buffer is binary. If
it is then the mime_type is set to "application/octet-stream". From the
mime-type it is determined if the property is binary or not.

In a follow-up patch I intend to extract the duplicated code for
detecting binary content into a libsvn_subr private function. Why not
get crazy and link in libmagic (used by the UNIX file command) to
determine what sort of content we have? Right now it says that anything
besides printable ascii is binary.

I'm attaching a script for testing this patch. Since I'm checking if the
content is binary on the mergestream I will end up with the diff beeing
displayed if it's below a certain treshold (when the number of
printable unidiff characters is more than 15 % of the total amount of
bytes.

Is it ok to have it like that or should I test for each of the original
streams instead? This was an easy solution. Perhaps too easy?

make check passed.

/Daniel

[

Attachment: merge-conflict.sh
Description: Bourne shell script

Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c	(revision 881392)
+++ subversion/libsvn_subr/stream.c	(arbetskopia)
@@ -1347,3 +1347,44 @@
 
   return SVN_NO_ERROR;
 }
+
+svn_error_t *
+svn_stream_detect_binary_mimetype(const char **mimetype,
+                                  svn_stream_t *stream)
+{
+  static const char * const generic_binary = "application/octet-stream";
+  char block[1024];
+  apr_size_t amt_read = sizeof(block);
+
+  /* Default return value is NULL. */
+  *mimetype = NULL;
+
+  SVN_ERR(svn_stream_read(stream, block, &amt_read));
+
+  if (amt_read > 0)
+    {
+      apr_size_t i;
+      apr_size_t binary_count = 0;
+
+      for (i = 0; i < amt_read; i++)
+        {
+          if (block[i] == 0)
+            {
+              binary_count = amt_read;
+              break;
+            }
+          if ((block[i] < 0x07)
+              || ((block[i] > 0x0D) && (block[i] < 0x20))
+              || (block[i] > 0x7F))
+            {
+              binary_count++;
+            }
+        }
+      if (((binary_count * 1000) / amt_read) > 850)
+        {
+          *mimetype = generic_binary;
+          return SVN_NO_ERROR;
+        }
+    }
+  return SVN_NO_ERROR;
+}
Index: subversion/svn/conflict-callbacks.c
===================================================================
--- subversion/svn/conflict-callbacks.c	(revision 881392)
+++ subversion/svn/conflict-callbacks.c	(arbetskopia)
@@ -615,6 +615,21 @@
             }
           else if (strcmp(answer, "df") == 0)
             {
+              if (desc->is_binary)
+                {
+                  const char *node;
+                  if (desc->kind == svn_wc_conflict_kind_property)
+                    node = _("property");
+                  else
+                    node = _("file");
+
+                  SVN_ERR(svn_cmdline_fprintf(stderr, subpool,
+                                              "%s %s.\n\n",
+                                              _("Invalid option; cannot "
+                                                "display conflicts for a "
+                                                "binary"), node));
+                  continue;
+                }
               if (! diff_allowed)
                 {
                   SVN_ERR(svn_cmdline_fprintf(stderr, subpool,
Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h	(revision 881392)
+++ subversion/include/svn_io.h	(arbetskopia)
@@ -1186,7 +1186,16 @@
                        apr_pool_t *result_pool,
                        apr_pool_t *scratch_pool);
 
+/**
+ * Examine utf8-encoded @a stream to determine if it has binary content. If
+ * so set @a *mimetype to a character string representing a general binary
+ * MIME type, else set it to @c NULL. 
+ */
+svn_error_t *
+svn_stream_detect_binary_mimetype(const char **mimetype,
+                                   svn_stream_t *stream);
 
+
 /** @} */
 
 /** Set @a *result to a string containing the contents of @a
Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c	(revision 881392)
+++ subversion/libsvn_wc/props.c	(arbetskopia)
@@ -939,9 +939,9 @@
                             apr_pool_t *scratch_pool)
 {
   svn_wc_conflict_result_t *result = NULL;
-  svn_string_t *mime_propval = NULL;
   apr_pool_t *filepool = svn_pool_create(scratch_pool);
   svn_wc_conflict_description2_t *cdesc;
+  const char *mimetype = NULL;
   const char *dirpath = svn_dirent_dirname(local_abspath, filepool);
 
   if (cancel_func)
@@ -1046,17 +1046,20 @@
                   (mergestream, diff, the_val, working_val, new_val,
                    NULL, NULL, NULL, NULL,
                    svn_diff_conflict_display_modified_latest, filepool));
+
+          /* We read the stream from the beginning again. */
+          SVN_ERR(svn_stream_reset(mergestream));
+          SVN_ERR(svn_stream_detect_binary_mimetype(&mimetype, mergestream));
+
+
           svn_stream_close(mergestream);
         }
     }
 
   /* Build the rest of the description object: */
-  if (!is_dir && working_props)
-    mime_propval = apr_hash_get(working_props, SVN_PROP_MIME_TYPE,
-                                APR_HASH_KEY_STRING);
-  cdesc->mime_type = mime_propval ? mime_propval->data : NULL;
-  cdesc->is_binary = mime_propval ?
-      svn_mime_type_is_binary(mime_propval->data) : FALSE;
+  cdesc->mime_type = mimetype ? mimetype : NULL;
+  cdesc->is_binary = mimetype ? 
+    svn_mime_type_is_binary(mimetype) : FALSE;
 
   if (!old_val && new_val)
     cdesc->action = svn_wc_conflict_action_add;

Reply via email to