Author: rhuijben
Date: Fri Feb 20 20:25:04 2015
New Revision: 1661208
URL: http://svn.apache.org/r1661208
Log:
Following up on a list of recent blame changes, make the revisions in the
'kidney blame' make a bit more sense.
The revisions we show in blame are the revisions in which something
changed... but the revision we get in the callback is the revision of
which we have the actual file data.
To make things match up the way you would expect we have to delay reporting
the revisions until we receive the next delta.
This patch reverts r1661202, and implements that behavior.
* subversion/libsvn_client/blame.c
(file_rev_baton): Add boolean and revision state.
(file_rev_handler): Remove calculating the highest revision.
Store the expected information in the revision baton, by
delaying everything a bit. Reinstate old assertion.
(svn_client_blame5): Remove ignore-revision code. Initialize baton.
* subversion/tests/cmdline/blame_tests.py
(blame_youngest_to_oldest,
blame_reverse_no_change): Update expected results.
Modified:
subversion/trunk/subversion/libsvn_client/blame.c
subversion/trunk/subversion/tests/cmdline/blame_tests.py
Modified: subversion/trunk/subversion/libsvn_client/blame.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/blame.c?rev=1661208&r1=1661207&r2=1661208&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/blame.c (original)
+++ subversion/trunk/subversion/libsvn_client/blame.c Fri Feb 20 20:25:04 2015
@@ -76,7 +76,7 @@ struct diff_baton {
/* The baton used for a file revision. Lives the entire operation */
struct file_rev_baton {
svn_revnum_t start_rev, end_rev;
- svn_revnum_t highest_rev; /* highest revision received */
+ svn_boolean_t backwards;
const char *target;
svn_client_ctx_t *ctx;
const svn_diff_file_options_t *diff_options;
@@ -97,6 +97,12 @@ struct file_rev_baton {
/* pools for files which may need to persist for more than one rev. */
apr_pool_t *filepool;
apr_pool_t *prevfilepool;
+
+ /* When blaming backwards we have to use the changes
+ on the *next* revision, as the interesting change
+ happens when we move to the previous revision */
+ svn_revnum_t last_revnum;
+ apr_hash_t *last_props;
};
/* The baton used by the txdelta window handler. Allocated per revision */
@@ -432,9 +438,6 @@ file_rev_handler(void *baton, const char
/* Clear the current pool. */
svn_pool_clear(frb->currpool);
- if (revnum > frb->highest_rev)
- frb->highest_rev = revnum;
-
if (frb->ctx->notify_func2)
{
svn_wc_notify_t *notify
@@ -493,9 +496,24 @@ file_rev_handler(void *baton, const char
/* Create the rev structure. */
delta_baton->rev = apr_pcalloc(frb->mainpool, sizeof(struct rev));
- if (merged_revision
- || (revnum >= MIN(frb->start_rev, frb->end_rev)
- && revnum <= MAX(frb->start_rev, frb->end_rev)))
+ if (frb->backwards)
+ {
+ /* Use from last round...
+ SVN_INVALID_REVNUM on first, which is exactly
+ what we want */
+ delta_baton->rev->revision = frb->last_revnum;
+ delta_baton->rev->rev_props = frb->last_props;
+
+ /* Store for next delta */
+ if (revnum >= MIN(frb->start_rev, frb->end_rev))
+ {
+ frb->last_revnum = revnum;
+ frb->last_props = svn_prop_hash_dup(rev_props, frb->mainpool);
+ }
+ /* Else: Not needed on last rev */
+ }
+ else if (merged_revision
+ || (revnum >= MIN(frb->start_rev, frb->end_rev)))
{
/* 1+ for the "youngest to oldest" blame */
SVN_ERR_ASSERT(revnum <= 1 + MAX(frb->end_rev, frb->start_rev));
@@ -508,6 +526,8 @@ file_rev_handler(void *baton, const char
{
/* We shouldn't get more than one revision outside the
specified range (unless we alsoe receive merged revisions) */
+ SVN_ERR_ASSERT((frb->last_filename == NULL)
+ || frb->include_merged_revisions);
/* The file existed before start_rev; generate no blame info for
lines from this revision (or before).
@@ -634,7 +654,6 @@ svn_client_blame5(const char *target,
svn_stream_t *stream;
const char *target_abspath_or_url;
svn_revnum_t youngest;
- svn_revnum_t ignore_rev = SVN_INVALID_REVNUM-1;
if (start->kind == svn_opt_revision_unspecified
|| end->kind == svn_opt_revision_unspecified)
@@ -709,7 +728,6 @@ svn_client_blame5(const char *target,
frb.start_rev = start_revnum;
frb.end_rev = end_revnum;
- frb.highest_rev = SVN_INVALID_REVNUM; /* -1 */
frb.target = target;
frb.ctx = ctx;
frb.diff_options = diff_options;
@@ -728,6 +746,9 @@ svn_client_blame5(const char *target,
frb.merged_chain->avail = NULL;
frb.merged_chain->pool = pool;
}
+ frb.backwards = (frb.start_rev > frb.end_rev);
+ frb.last_revnum = SVN_INVALID_REVNUM;
+ frb.last_props = NULL;
SVN_ERR(svn_ra_get_repos_root2(ra_session, &frb.repos_root_url, pool));
@@ -845,9 +866,6 @@ svn_client_blame5(const char *target,
walk_merged = frb.merged_chain->blame;
}
- if (frb.start_rev > frb.end_rev)
- ignore_rev = frb.highest_rev;
-
/* Process each blame item. */
for (walk = frb.chain->blame; walk; walk = walk->next)
{
@@ -882,7 +900,7 @@ svn_client_blame5(const char *target,
SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
if (!eof || sb->len)
{
- if (walk->rev && walk->rev->revision != ignore_rev)
+ if (walk->rev)
SVN_ERR(receiver(receiver_baton, start_revnum, end_revnum,
line_no, walk->rev->revision,
walk->rev->rev_props, merged_rev,
Modified: subversion/trunk/subversion/tests/cmdline/blame_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/blame_tests.py?rev=1661208&r1=1661207&r2=1661208&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/blame_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/blame_tests.py Fri Feb 20
20:25:04 2015
@@ -962,7 +962,7 @@ def blame_youngest_to_oldest(sbox):
sbox.simple_commit() #r4
expected_output = [
- ' %d jrandom %s\n' % (3, orig_line[:-1]),
+ ' %d jrandom %s\n' % (4, orig_line[:-1]),
]
svntest.actions.run_and_verify_svn(expected_output, [],
'blame', '-r4:1', iota_moved)
@@ -1024,22 +1024,22 @@ def blame_reverse_no_change(sbox):
expected_output = [
' - - This is the file \'iota\'.\n',
- ' 4 jrandom new line\n',
+ ' 5 jrandom new line\n',
]
svntest.actions.run_and_verify_svn(expected_output, [],
'blame', '-rHEAD:3', sbox.ospath('iota'))
expected_output = [
' - - This is the file \'iota\'.\n',
- ' 4 jrandom new line\n',
- ' 5 jrandom another new line\n',
+ ' 5 jrandom new line\n',
+ ' 6 jrandom another new line\n',
]
svntest.actions.run_and_verify_svn(expected_output, [],
'blame', '-rHEAD:4', sbox.ospath('iota'))
expected_output = [
' - - This is the file \'iota\'.\n',
- ' 5 jrandom another new line\n',
+ ' 6 jrandom another new line\n',
]
svntest.actions.run_and_verify_svn(expected_output, [],
'blame', '-rHEAD:5', sbox.ospath('iota'))
@@ -1053,7 +1053,7 @@ def blame_reverse_no_change(sbox):
expected_output = [
' - - This is the file \'iota\'.\n',
- ' 4 jrandom new line\n',
+ ' 5 jrandom new line\n',
]
svntest.actions.run_and_verify_svn(expected_output, [],
'blame', '-r5:3', sbox.ospath('iota'))