[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1570936 - /subversion/trunk/subversion/libsvn_client/blame.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Sat, 22 Feb 2014 23:06:11 +0000

I think your patch doesn't change the behavior (this is more a brain dump based on te earlier related mail thread), but there are property changes that do change how we interpret the content of a file… E.g. some changes to svn:eol-style and svn:keywords.


In these cases the actual file may change while the repository form didn’t.


Blame, especially with whitespace ignores might not be interested in these cases, but there are api users that use the file revs api that are interested.



Bert






Sent from Windows Mail





From: Stefan Fuhrmann
Sent: ‎Saturday‎, ‎February‎ ‎22‎, ‎2014 ‎11‎:‎27‎ ‎PM
To: commits_at_subversion.apache.org





Author: stefan2
Date: Sat Feb 22 22:27:06 2014
New Revision: 1570936

URL: http://svn.apache.org/r1570936
Log:
Fix the blame -g implemenation to no longer depend on false positives
in the file contents change detection.

* subversion/libsvn_client/blame.c
  (update_blame): Factored out from ...
  (window_handler): ... this. Only do the diff application directly here.
  (file_rev_handler): For potential merges, trigger the blame processing
                      even if there has been no change since the last rev
                      which might have been on a different branch.

Modified:
    subversion/trunk/subversion/libsvn_client/blame.c

Modified: subversion/trunk/subversion/libsvn_client/blame.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/blame.c?rev=1570936&r1=1570935&r2=1570936&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/blame.c (original)
+++ subversion/trunk/subversion/libsvn_client/blame.c Sat Feb 22 22:27:06 2014
@@ -305,27 +305,15 @@ add_file_blame(const char *last_file,
   return SVN_NO_ERROR;
 }
 
-/* The delta window handler for the text delta between the previously seen
- * revision and the revision currently being handled.
- *
- * Record the blame information for this revision in BATON->file_rev_baton.
- *
- * Implements svn_txdelta_window_handler_t.
+/* Record the blame information for the revision in BATON->file_rev_baton.
  */
 static svn_error_t *
-window_handler(svn_txdelta_window_t *window, void *baton)
+update_blame(void *baton)
 {
   struct delta_baton *dbaton = baton;
   struct file_rev_baton *frb = dbaton->file_rev_baton;
   struct blame_chain *chain;
 
- /* Call the wrapped handler first. */
- SVN_ERR(dbaton->wrapped_handler(window, dbaton->wrapped_baton));
-
- /* We patiently wait for the NULL window marking the end. */
- if (window)
- return SVN_NO_ERROR;
-
   /* Close the source file used for the delta.
      It is important to do this early, since otherwise, they will be deleted
      before all handles are closed, which leads to failures on some platforms
@@ -382,6 +370,32 @@ window_handler(svn_txdelta_window_t *win
   return SVN_NO_ERROR;
 }
 
+/* The delta window handler for the text delta between the previously seen
+ * revision and the revision currently being handled.
+ *
+ * Record the blame information for this revision in BATON->file_rev_baton.
+ *
+ * Implements svn_txdelta_window_handler_t.
+ */
+static svn_error_t *
+window_handler(svn_txdelta_window_t *window, void *baton)
+{
+ struct delta_baton *dbaton = baton;
+
+ /* Call the wrapped handler first. */
+ if (dbaton->wrapped_handler)
+ SVN_ERR(dbaton->wrapped_handler(window, dbaton->wrapped_baton));
+
+ /* We patiently wait for the NULL window marking the end. */
+ if (window)
+ return SVN_NO_ERROR;
+
+ /* Diff and update blame info. */
+ SVN_ERR(update_blame(baton));
+
+ return SVN_NO_ERROR;
+}
+
 
 /* Calculate and record blame information for one revision of the file,
  * by comparing the file content against the previously seen revision.
@@ -430,17 +444,18 @@ file_rev_handler(void *baton, const char
   if (frb->ctx->cancel_func)
     SVN_ERR(frb->ctx->cancel_func(frb->ctx->cancel_baton));
 
- /* If there were no content changes, we couldn't care less about this
- revision now. Note that we checked the mime type above, so things
- work if the user just changes the mime type in a commit.
+ /* If there were no content changes and no (potential) merges, we couldn't
+ care less about this revision now. Note that we checked the mime type
+ above, so things work if the user just changes the mime type in a commit.
      Also note that we don't switch the pools in this case. This is important,
      since the tempfile will be removed by the pool and we need the tempfile
      from the last revision with content changes. */
- if (!content_delta_handler)
+ if (!content_delta_handler
+ && (!frb->include_merged_revisions || merged_revision))
     return SVN_NO_ERROR;
 
   /* Create delta baton. */
- delta_baton = apr_palloc(frb->currpool, sizeof(*delta_baton));
+ delta_baton = apr_pcalloc(frb->currpool, sizeof(*delta_baton));
 
   /* Prepare the text delta window handler. */
   if (frb->last_filename)
@@ -460,17 +475,9 @@ file_rev_handler(void *baton, const char
                                  svn_io_file_del_on_pool_cleanup,
                                  filepool, filepool));
 
- /* Get window handler for applying delta. */
- svn_txdelta_apply(last_stream, cur_stream, NULL, NULL,
- frb->currpool,
- &delta_baton->wrapped_handler,
- &delta_baton->wrapped_baton);
-
   /* Wrap the window handler with our own. */
   delta_baton->file_rev_baton = frb;
   delta_baton->is_merged_revision = merged_revision;
- *content_delta_handler = window_handler;
- *content_delta_baton = delta_baton;
 
   /* Create the rev structure. */
   delta_baton->rev = apr_pcalloc(frb->mainpool, sizeof(struct rev));
@@ -502,6 +509,28 @@ file_rev_handler(void *baton, const char
   /* Keep last revision for postprocessing after all changes */
   frb->last_rev = delta_baton->rev;
 
+ /* Handle all delta - even if it is empty.
+ We must do the latter to "merge" blame info from other branches. */
+ if (content_delta_handler)
+ {
+ /* Proper delta - get window handler for applying delta.
+ svn_ra_get_file_revs2 will drive the delta editor. */
+ svn_txdelta_apply(last_stream, cur_stream, NULL, NULL,
+ frb->currpool,
+ &delta_baton->wrapped_handler,
+ &delta_baton->wrapped_baton);
+ *content_delta_handler = window_handler;
+ *content_delta_baton = delta_baton;
+ }
+ else
+ {
+ /* Apply an empty delta, i.e. simply copy the old contents.
+ We can't simply use the existing file due to the pool rotation logic.
+ Trigger the blame update magic. */
+ SVN_ERR(svn_stream_copy3(last_stream, cur_stream, NULL, NULL, pool));
+ SVN_ERR(update_blame(delta_baton));
+ }
+
   return SVN_NO_ERROR;
 }
Received on 2014-02-23 00:12:32 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.