Could someone of the devs please take a closer look?
I am willing to improve the patch based on concrete suggestions.
[...]
Sure thing. I can see where you're going with the patch. I haven't taken
the time to fully consider the appropriate, so consider the following a
syntactical patch review only.
thank you for your feedback.
I have updated the patch of "libsvn_client/log.c" according to your suggestions.
Finally, patches are best sent to the mailing list with a "[PATCH] "
Additionally i have added a test to "tests/cmdline/log_tests.py" to check for
desired results.
The newly added test will pass when the above enhancement has been applied
(other tests also pass).
The combined patch is attached.
Subject: line prefix *and* a valid log message.
The log message might be:
<<<
Fix issue #3830: support "forward" logs for revision ranges where the target
has been removed before end-of-range.
* subversion/libsvn_client/log.c
(svn_client_log5): replace end of revision range with revision before item
is deleted if necessary.
* subversion/tests/cmdline/log_tests.py
(forward_log): added test for "forward" logs where target has been removed
before end-of-range.
Patch by: Dirk Thomas <web...@dirk-thomas.net>
>>>
If the modifications need further polishing please just drop me a note.
Dirk
Index: tests/cmdline/log_tests.py
===================================================================
--- tests/cmdline/log_tests.py (revision 1129130)
+++ tests/cmdline/log_tests.py (working copy)
@@ -1973,6 +1973,35 @@
log_chain = parse_log_output(out)
check_merge_results(log_chain, expected_merges)
+#----------------------------------------------------------------------
+@Issue(3830)
+def forward_log(sbox):
+ "'svn log -rM:N', when N>M, target removed from N"
+
+ guarantee_repos_and_wc(sbox)
+
+ target = os.path.join(sbox.repo_url, 'A', 'D', 'G', 'rho') + "@2"
+
+ # forward log for /A/D/G/rho although deleted in revision 5
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ '1:7', target)
+ check_log_chain(parse_log_output(output), [1, 3])
+
+ # forward log for /A/D/G/rho although newly added in 8
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ '1:HEAD', target)
+ check_log_chain(parse_log_output(output), [1, 3])
+
+ # forward log using working copy and BASE < 5
+ svntest.main.run_svn(None, 'up', '-r', '2', sbox.wc_dir)
+ target = os.path.join(sbox.wc_dir, 'A', 'D', 'G', 'rho')
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ 'BASE:HEAD', target)
+ check_log_chain(parse_log_output(output), [3])
+
########################################################################
# Run the tests
@@ -2010,6 +2039,7 @@
log_of_local_copy,
merge_sensitive_log_reverse_merges,
merge_sensitive_log_ignores_cyclic_merges,
+ forward_log,
]
if __name__ == '__main__':
Index: libsvn_client/log.c
===================================================================
--- libsvn_client/log.c (revision 1129130)
+++ libsvn_client/log.c (working copy)
@@ -261,6 +261,42 @@
return rb->receiver(rb->baton, log_entry, pool);
}
+static svn_error_t *
+check_for_deleted_rev(svn_ra_session_t *ra_session,
+ const char *url_or_path,
+ svn_revnum_t peg_revnum,
+ svn_revnum_t end_revnum,
+ svn_revnum_t *revision_deleted,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool
+)
+{
+ const char *session_url;
+ const char *rel_path;
+
+ SVN_ERR(svn_ra_get_session_url(ra_session,
+ &session_url,
+ pool));
+
+ SVN_ERR(svn_client__path_relative_to_root(&rel_path,
+ ctx->wc_ctx,
+ url_or_path,
+ session_url,
+ FALSE,
+ ra_session,
+ pool,
+ pool));
+
+ SVN_ERR(svn_ra_get_deleted_rev(ra_session,
+ rel_path,
+ peg_revnum,
+ end_revnum,
+ revision_deleted,
+ pool));
+
+ return SVN_NO_ERROR;
+}
+
/*** Public Interface. ***/
@@ -472,6 +508,9 @@
{
+ svn_revnum_t peg_revnum, session_opt_revnum;
+ svn_error_t *err;
+
/* If this is a revision type that requires access to the working copy,
* we use our initial target path to figure out where to root the RA
* session, otherwise we use our URL. */
@@ -481,11 +520,54 @@
else
ra_target = url_or_path;
- SVN_ERR(svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
- &actual_url, ra_target, NULL,
- &peg_rev, &session_opt_rev,
- ctx, pool));
+ SVN_ERR(svn_client__get_revision_number(&peg_revnum, NULL,
+ ctx->wc_ctx, ra_target,
+ ra_session, &peg_rev,
+ pool));
+ SVN_ERR(svn_client__get_revision_number(&session_opt_revnum, NULL,
+ ctx->wc_ctx, ra_target,
+ ra_session, &session_opt_rev,
+ pool));
+ err = svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
+ &actual_url, ra_target, NULL,
+ &peg_rev, &session_opt_rev,
+ ctx, pool);
+
+ if (err && err->apr_err == SVN_ERR_FS_NOT_FOUND && session_opt_revnum > peg_revnum)
+ {
+ svn_revnum_t revision_deleted = SVN_INVALID_REVNUM;
+ apr_pool_t *sesspool = svn_pool_create(pool);
+
+ SVN_ERR(svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
+ &actual_url, ra_target, NULL,
+ &peg_rev, &peg_rev,
+ ctx, sesspool));
+
+ SVN_ERR(check_for_deleted_rev(ra_session,
+ url_or_path, peg_revnum,
+ session_opt_revnum,
+ &revision_deleted,
+ ctx, sesspool));
+
+ svn_pool_destroy(sesspool);
+
+ if (SVN_IS_VALID_REVNUM(revision_deleted))
+ {
+ svn_opt_revision_t session_mod_rev;
+ session_mod_rev.kind = svn_opt_revision_number;
+ session_mod_rev.value.number = revision_deleted - 1;
+
+ svn_error_clear(err);
+
+ err = svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
+ &actual_url, ra_target, NULL,
+ &peg_rev, &session_mod_rev,
+ ctx, pool);
+ }
+ }
+ SVN_ERR(err);
+
SVN_ERR(svn_ra_has_capability(ra_session, &has_log_revprops,
SVN_RA_CAPABILITY_LOG_REVPROPS, pool));
@@ -549,7 +631,7 @@
iterpool = svn_pool_create(pool);
for (i = 0; i < revision_ranges->nelts; i++)
{
- svn_revnum_t start_revnum, end_revnum, youngest_rev = SVN_INVALID_REVNUM;
+ svn_revnum_t start_revnum, end_revnum, peg_revnum, youngest_rev = SVN_INVALID_REVNUM, revision_deleted = SVN_INVALID_REVNUM;
const char *path = APR_ARRAY_IDX(targets, 0, const char *);
const char *local_abspath_or_url;
svn_opt_revision_range_t *range;
@@ -575,6 +657,10 @@
ctx->wc_ctx, local_abspath_or_url,
ra_session, &range->end,
iterpool));
+ SVN_ERR(svn_client__get_revision_number(&peg_revnum, &youngest_rev,
+ ctx->wc_ctx, local_abspath_or_url,
+ ra_session, &peg_rev,
+ iterpool));
if (has_log_revprops)
{
@@ -603,6 +689,20 @@
passed_receiver_baton = &lb;
}
+ if (SVN_IS_VALID_REVNUM(peg_revnum) && end_revnum > peg_revnum)
+ {
+ SVN_ERR(check_for_deleted_rev(ra_session,
+ url_or_path, peg_revnum,
+ end_revnum,
+ &revision_deleted,
+ ctx, pool));
+ }
+
+ if (SVN_IS_VALID_REVNUM(revision_deleted))
+ {
+ end_revnum = revision_deleted - 1;
+ }
+
SVN_ERR(svn_ra_get_log2(ra_session,
condensed_targets,
start_revnum,
