[PATCH][1.4.x!] Fix slew of blame issues

In the thread '[RFC] 1.4.x platform dependent blame output' I opened a
discussion about what blame output should look like.

If the outcome is to be that we want to change all behaviours as given
in that thread (and as ack-ed by djh), the patch below can be used to
fix the code.

I'll be out of commit range for Tuesday until Friday, so I decided to
post the patch now; even without a definitive outcome of the
forementioned thread.

Comments welcome!

(I'm not sure gmail won't munge my mail, so I'm attaching the patch too.)

bye,

Erik.

Log:
[[[
Fix slew of blame issues:

- Make blame output 'client output' instead of 'raw output'
- Eliminate blame-slowdown wrt 1.3.x (by eliminating
  translation of intermediate files)
- Make 'line' argument of svn_client_blame_receiver_t better defined
- Stop passing CR characters in the 'line' argument of
svn_client_blame_receiver_t
- Fix blame of CR-eol style file
-

* subversion/include/svn_client.h
  (svn_client_blame_receiver_t): Adjust documentation of the line argument

* subversion/libsvn_client/blame.c
  (file_rev_baton): Remove fields related to translating.
  (window_handler): Remove translation of intermediate files.
  (update_eol_style): Remove (was used for translation).
  (svn_client_blame3): Remove initialisation of removed
   file_rev_baton fields. Make sure we pass only line content
   to the receiver by translating eols to a known eol style and
   reading that back from the stream.
  (wrapped_receiver_baton_s, wrapped_receiver,
   wrap_pre3_receiver): New. Used to generate a line argument
   for older receivers by adding a CR to the line if CRLF
   is the eol style of the platform. Older clients depend
   on this behaviour (ie having the CR in the line).
  (svn_client_blame2, svn_client_blame): Wrap receiver
   to compensate for CR/no-CR differences with
   svn_client_blame3.

* subversion/svn/blame-cmd.c
  (blame_receiver): Adjust to generate the right eol marker
   for each platform (given that blame isn't 'raw output').
  (svn_cl__blame): Adapt comment to new
   svn_client_blame3 behaviour.
]]]

Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 20039)
+++ subversion/include/svn_client.h (working copy)
@@ -480,6 +480,9 @@
  *
  * @note If there is no blame information for this line, @a revision will be
  * invalid and @a author and @a date will be NULL.
+ *
+ * @note New in 1.4 is that the line is defined to contain only the line
+ * content (and no [partial] eols; which was undefined in older versions)
  */
 typedef svn_error_t *(*svn_client_blame_receiver_t)
   (void *baton,
Index: subversion/libsvn_client/blame.c
===================================================================
--- subversion/libsvn_client/blame.c (revision 20039)
+++ subversion/libsvn_client/blame.c (working copy)
@@ -62,12 +62,6 @@
   svn_boolean_t ignore_mime_type;
   /* name of file containing the previous revision of the file */
   const char *last_filename;
- /* name of file containing the previous revision of the file, translated
- to the appropriate EOL style. May be identical to the above if no
- translation was required. */
- const char *last_filename_translated;
- svn_subst_eol_style_t eol_style;
- const char *eol_str;
   struct rev *rev; /* the rev for which blame is being assigned
                           during a diff */
   struct blame *blame; /* linked list of blame chunks */
@@ -320,7 +314,6 @@
 {
   struct delta_baton *dbaton = baton;
   struct file_rev_baton *frb = dbaton->file_rev_baton;
- const char *translation_tgt = dbaton->filename;

   /* Call the wrapped handler first. */
   SVN_ERR(dbaton->wrapped_handler(window, dbaton->wrapped_baton));
@@ -337,31 +330,14 @@
     SVN_ERR(svn_io_file_close(dbaton->source_file, frb->currpool));
   SVN_ERR(svn_io_file_close(dbaton->file, frb->currpool));

- if (svn_subst_translation_required(frb->eol_style, frb->eol_str,
- NULL, FALSE, FALSE))
- {
- SVN_ERR(svn_io_open_unique_file2(NULL,
- &translation_tgt,
- frb->tmp_path,
- ".tmp",
- svn_io_file_del_on_pool_cleanup,
- frb->currpool));
- SVN_ERR(svn_subst_copy_and_translate3(dbaton->filename,
- translation_tgt,
- frb->eol_str, FALSE,
- NULL, FALSE, FALSE,
- frb->currpool));
- }
-
   /* Process this file. */
- SVN_ERR(add_file_blame(frb->last_filename_translated,
- translation_tgt, frb));
+ SVN_ERR(add_file_blame(frb->last_filename,
+ dbaton->filename, frb));

   /* Prepare for next revision. */

   /* Remember the file name so we can diff it with the next revision. */
   frb->last_filename = dbaton->filename;
- frb->last_filename_translated = translation_tgt;

   /* Switch pools. */
   {
@@ -396,25 +372,6 @@
 }

-static void
-update_eol_style(svn_subst_eol_style_t *style,
- const char **eol,
- apr_array_header_t *prop_diffs)
-{
- int i;
-
- for (i = 0; i < prop_diffs->nelts; ++i)
- {
- const svn_prop_t *prop = &APR_ARRAY_IDX(prop_diffs, i, svn_prop_t);
- if (strcmp(prop->name, SVN_PROP_EOL_STYLE) == 0)
- {
- svn_subst_eol_style_from_value
- (style, eol, prop->value ? prop->value->data : NULL);
- return;
- }
- }
-}
-
 static svn_error_t *
 file_rev_handler(void *baton, const char *path, svn_revnum_t revnum,
                  apr_hash_t *rev_props,
@@ -435,8 +392,6 @@
   if (! frb->ignore_mime_type)
     SVN_ERR(check_mimetype(prop_diffs, frb->target, frb->currpool));

- update_eol_style(&frb->eol_style, &frb->eol_str, prop_diffs);
-
   if (frb->ctx->notify_func2)
     {
       svn_wc_notify_t *notify
@@ -586,9 +541,6 @@
   frb.diff_options = diff_options;
   frb.ignore_mime_type = ignore_mime_type;
   frb.last_filename = NULL;
- frb.last_filename_translated = NULL;
- frb.eol_style = svn_subst_eol_style_none;
- frb.eol_str = NULL;
   frb.blame = NULL;
   frb.avail = NULL;

@@ -632,7 +584,8 @@
   /* Open the last file and get a stream. */
   SVN_ERR(svn_io_file_open(&file, frb.last_filename, APR_READ | APR_BUFFERED,
                            APR_OS_DEFAULT, pool));
- stream = svn_stream_from_aprfile(file, pool);
+ stream = svn_subst_stream_translated(svn_stream_from_aprfile(file, pool),
+ "\n", TRUE, NULL, FALSE, pool);

   /* Process each blame item. */
   for (walk = frb.blame; walk; walk = walk->next)
@@ -668,6 +621,58 @@
   return SVN_NO_ERROR;
 }

+
+/* svn_client_blame3 guarantees 'no eol chars' as part of the receiver
+ LINE argument. Older versions depend on the fact that if a CR is
+ required, that CR is already part of the LINE data.
+
+ Because of this difference, we need to trap old receivers and append
+ a CR to LINE before passing it on to the actual receiver on platforms
+ which want CRLF line termination.
+
+*/
+
+struct wrapped_receiver_baton_s
+{
+ svn_client_blame_receiver_t orig_receiver;
+ void *orig_baton;
+};
+
+static svn_error_t *
+wrapped_receiver(void *baton,
+ apr_int64_t line_no,
+ svn_revnum_t revision,
+ const char *author,
+ const char *date,
+ const char *line,
+ apr_pool_t *pool)
+{
+ struct wrapped_receiver_baton_s *b = baton;
+ svn_stringbuf_t *expanded_line = svn_stringbuf_create(line, pool);
+
+ svn_stringbuf_appendbytes(expanded_line,"\r",1);
+
+ return b->orig_receiver(b->orig_baton, line_no, revision, author,
+ date, expanded_line->data, pool);
+}
+
+static void
+wrap_pre3_receiver(svn_client_blame_receiver_t *receiver,
+ void **receiver_baton,
+ apr_pool_t *pool)
+{
+ if (strlen(APR_EOL_STR) > 1)
+ {
+ struct wrapped_receiver_baton_s *b = apr_palloc(pool,sizeof(*b));
+
+ b->orig_receiver = *receiver;
+ b->orig_baton = *receiver_baton;
+
+ *receiver_baton = b;
+ *receiver = wrapped_receiver;
+ }
+}
+
 svn_error_t *
 svn_client_blame2(const char *target,
                   const svn_opt_revision_t *peg_revision,
@@ -678,6 +683,7 @@
                   svn_client_ctx_t *ctx,
                   apr_pool_t *pool)
 {
+ wrap_pre3_receiver(&receiver, &receiver_baton, pool);
   return svn_client_blame3(target, peg_revision, start, end,
                            svn_diff_file_options_create(pool), FALSE,
                            receiver, receiver_baton, ctx, pool);
@@ -691,6 +697,7 @@
                  svn_client_ctx_t *ctx,
                  apr_pool_t *pool)
 {
+ wrap_pre3_receiver(&receiver, &receiver_baton, pool);
   return svn_client_blame2(target, end, start, end,
                            receiver, receiver_baton, ctx, pool);
 }
Index: subversion/svn/blame-cmd.c
===================================================================
--- subversion/svn/blame-cmd.c (revision 20039)
+++ subversion/svn/blame-cmd.c (working copy)
@@ -123,14 +123,15 @@
              abbreviations for the month and weekday names. Else, the
              line contents will be misaligned. */
           time_stdout = " -";
- return svn_stream_printf(out, pool, "%s %10s %s %s\n", rev_str,
+ return svn_stream_printf(out, pool, "%s %10s %s %s%s", rev_str,
                                author ? author : " -",
- time_stdout , line);
+ time_stdout , line, APR_EOL_STR);
     }
   else
     {
- return svn_stream_printf(out, pool, "%s %10s %s\n", rev_str,
- author ? author : " -", line);
+ return svn_stream_printf(out, pool, "%s %10s %s%s", rev_str,
+ author ? author : " -",
+ line, APR_EOL_STR);
     }
 }

@@ -206,21 +207,10 @@
       opt_state->start_revision.value.number = 1;
     }

- /* A comment about the use of svn_stream_t for column-based output,
- and stdio for XML output:
-
- stdio does newline translations for us. Since our XML routines
- from svn_xml.h produce text separated with \n, we want that
- translation to happen, making the XML more readable on some
- platforms.
-
- For the column-based output, we output contents from the file, so
- we don't want stdio to mess with the newlines. We finish lines
- by \n, but the file might contain \r characters at the end of
- lines, since svn_client_blame() splits lines at \n characters.
- That would lead to CRCRLF line endings on platforms with CRLF
- line endings. */
-
+ /* The final conclusion from issue #2431 is that blame info
+ is client output (unlike 'svn cat' which plainly cats the file),
+ so the eol style should be the platform local one.
+ */
   if (! opt_state->xml)
     SVN_ERR(svn_stream_for_stdout(&bl.out, pool));
   else

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org