Well, since issue 3931 was fixed today
('svn log' is returning log of unrelated path when peg revision is not related
to operative revision)
i revisited my patch for issue 3830.
+ /* try fetching ra_session again
+ * this time with modified revision argument.
+ * @todo modify already existing ra_session if possible? */
+ err = svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
+ &actual_url, ra_target, NULL,
+ &peg_rev, &session_mod_rev,
+ ctx, pool);
I don't know if it is possible to modify an already existing ra_session instead
of opening a new for different revisions.
If yes, please come up with an short description, so that performance would be
improved.
This looks expensive as well. It's a round trip to find out if the path
has been deleted. Would it be more efficient to try the log and only do
this if the log fails?
I don't see a way to achieve this.
If svn_ra_get_log2 is executed with the original revision range it will return
the log for an unrelated path!
So there is no way to call it first and only in case of errors check for
deleted revision.
(I am also not sure if svn_ra_get_log2 needs then to be fixed or if it is sufficient if
these checks are performed "outside" in svn_client_log5.)
I have updated and attached the patch for issue 3830.
It now deals correctly with all cases like forward/backward ranges,
number/HEAD/BASE/dates.
May be someone might again take a closer look.
The initial idea was to enable forward revision logs like N:HEAD even if the
path has been deleted in between.
In the meantime i do see some problematic cases what would be the expected
result for a svn log command.
E.g. see the modified unit test about what to expect from log-commands when the
effective revision range (after reducing it based on deleted revision) does no
more cover any revisions.
I will send another mail about this problem later.
Dirk
Index: tests/cmdline/log_tests.py
===================================================================
--- tests/cmdline/log_tests.py (revision 1139047)
+++ tests/cmdline/log_tests.py (working copy)
@@ -1974,7 +1974,7 @@
check_merge_results(log_chain, expected_merges)
#----------------------------------------------------------------------
-@Issue(3931,3936)
+@Issue(3931,3936,3830)
@XFail(svntest.main.is_ra_type_dav_serf)
def log_with_unrelated_peg_and_operative_revs(sbox):
"log with unrelated peg and operative rev targets"
@@ -1984,27 +1984,50 @@
target = sbox.repo_url + '/A/D/G/rho@2'
# log for /A/D/G/rho, deleted in revision 5, recreated in revision 8
- expected_error = ".*File not found.*"
- svntest.actions.run_and_verify_svn(None, None, expected_error,
- 'log', '-r', '6:7', target)
- svntest.actions.run_and_verify_svn(None, None, expected_error,
- 'log', '-r', '7:6', target)
+ #expected_error = ".*File not found.*"
+ #svntest.actions.run_and_verify_svn(None, None, expected_error,
+ # 'log', '-r', '6:7', target)
+ #svntest.actions.run_and_verify_svn(None, None, expected_error,
+ # 'log', '-r', '7:6', target)
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ '6:7', target)
+ check_log_chain(parse_log_output(output), [])
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ '7:6', target)
+ check_log_chain(parse_log_output(output), [])
- expected_error = ".*Unable to find repository location for.*"
- svntest.actions.run_and_verify_svn(None, None, expected_error,
- 'log', '-r', '2:9', target)
- svntest.actions.run_and_verify_svn(None, None, expected_error,
- 'log', '-r', '9:2', target)
+ #expected_error = ".*Unable to find repository location for.*"
+ #svntest.actions.run_and_verify_svn(None, None, expected_error,
+ # 'log', '-r', '2:9', target)
+ #svntest.actions.run_and_verify_svn(None, None, expected_error,
+ # 'log', '-r', '9:2', target)
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ '2:9', target)
+ check_log_chain(parse_log_output(output), [3])
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ '9:2', target)
+ check_log_chain(parse_log_output(output), [3])
- # Currently this test fails because instead of returning the expected
- # 'Unable to find repository location for ^/A/D/G/rho in revision 9'
- # error, the log for ^/A/D/G/rho@8 is returned, but that is an unrelated
+ # Currently this test fails because instead of returning the expected log
+ # range, the log for ^/A/D/G/rho@8 is returned, but that is an unrelated
# line of history.
- expected_error = ".*Unable to find repository location for.*"
- svntest.actions.run_and_verify_svn(None, None, expected_error,
- 'log', '-r', '2:HEAD', target)
- svntest.actions.run_and_verify_svn(None, None, expected_error,
- 'log', '-r', 'HEAD:2', target)
+ #expected_error = ".*Unable to find repository location for.*"
+ #svntest.actions.run_and_verify_svn(None, None, expected_error,
+ # 'log', '-r', '2:HEAD', target)
+ #svntest.actions.run_and_verify_svn(None, None, expected_error,
+ # 'log', '-r', 'HEAD:2', target)
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ '2:HEAD', target)
+ check_log_chain(parse_log_output(output), [3])
+ exit_code, output, err = svntest.actions.run_and_verify_svn(None, None, [],
+ 'log', '-r',
+ 'HEAD:2', target)
+ check_log_chain(parse_log_output(output), [3])
#----------------------------------------------------------------------
@Issue(3937)
@@ -2036,6 +2059,35 @@
svntest.actions.run_and_verify_svn(None, None, expected_error,
'log', '-q', bad_path_default_rev)
+#----------------------------------------------------------------------
+@Issue(3830)
+def forward_log(sbox):
+ "'svn log -rM:N', when N>M, target removed from N"
+
+ guarantee_repos_and_wc(sbox)
+
+ target = 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
@@ -2075,6 +2127,7 @@
merge_sensitive_log_ignores_cyclic_merges,
log_with_unrelated_peg_and_operative_revs,
log_on_nonexistent_path_and_valid_rev,
+ forward_log,
]
if __name__ == '__main__':
Index: libsvn_client/log.c
===================================================================
--- libsvn_client/log.c (revision 1139047)
+++ libsvn_client/log.c (working copy)
@@ -261,6 +261,43 @@
return rb->receiver(rb->baton, log_entry, pool);
}
+/* find the revision at which the node was deleted
+ and sets *REVISION_DELETED to that revision. */
+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. ***/
@@ -476,7 +513,11 @@
}
+ iterpool = svn_pool_create(pool);
{
+ 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. */
@@ -486,11 +527,86 @@
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));
+ err = svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
+ &actual_url, ra_target, NULL,
+ &peg_rev, &session_opt_rev,
+ ctx, pool);
+ /* If an error is returned due to a non-existing or unrelated path
+ * we check if operative revision is ahead of peg revision and
+ * if the path has been deleted in between. */
+ if (err && (err->apr_err == SVN_ERR_FS_NOT_FOUND
+ || err->apr_err == SVN_ERR_CLIENT_UNRELATED_RESOURCES))
+ {
+ /* ERR2 is used to keep the initial error ERR. */
+ svn_error_t *err2;
+
+ svn_pool_clear(iterpool);
+
+ err2 = svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
+ &actual_url, ra_target, NULL,
+ &peg_rev, &peg_rev,
+ ctx, iterpool);
+ if (err2)
+ {
+ svn_error_clear(err);
+ SVN_ERR(err2);
+ }
+
+ err2 = svn_client__get_revision_number(&peg_revnum, NULL,
+ ctx->wc_ctx, ra_target,
+ ra_session, &peg_rev,
+ iterpool);
+ if (err2)
+ {
+ svn_error_clear(err);
+ SVN_ERR(err2);
+ }
+
+ err2 = svn_client__get_revision_number(&session_opt_revnum, NULL,
+ ctx->wc_ctx, ra_target,
+ ra_session, &session_opt_rev,
+ iterpool);
+ if (err2)
+ {
+ svn_error_clear(err);
+ SVN_ERR(err2);
+ }
+
+ if (session_opt_revnum > peg_revnum)
+ {
+ svn_revnum_t revision_deleted = SVN_INVALID_REVNUM;
+ svn_opt_revision_t session_mod_rev;
+
+ svn_error_clear(err);
+
+ SVN_ERR(check_for_deleted_rev(ra_session,
+ actual_url, peg_revnum,
+ session_opt_revnum,
+ &revision_deleted,
+ ctx, iterpool));
+
+ if (!SVN_IS_VALID_REVNUM(revision_deleted))
+ {
+ return svn_error_create
+ (SVN_ERR_CLIENT_BAD_REVISION, NULL,
+ _("Could not determine revision where target was deleted"));
+ }
+
+ /* try fetching ra_session again
+ * this time with modified revision argument.
+ * @todo modify already existing ra_session if possible? */
+ session_mod_rev.kind = svn_opt_revision_number;
+ session_mod_rev.value.number = revision_deleted - 1;
+
+ 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));
@@ -551,10 +667,9 @@
* epg wonders if the repository could send a unified stream of log
* entries if the paths and revisions were passed down.
*/
- 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, greater_revnum, youngest_rev = 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;
@@ -580,6 +695,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)
{
@@ -608,6 +727,41 @@
passed_receiver_baton = &lb;
}
+ /* If greater revision is ahead of peg revision
+ * check if the path has been deleted in this interval
+ * and adapt revisions accordingly. */
+ greater_revnum = start_revnum > end_revnum ? start_revnum : end_revnum;
+ if (SVN_IS_VALID_REVNUM(peg_revnum) && greater_revnum > peg_revnum)
+ {
+ svn_revnum_t revision_deleted = SVN_INVALID_REVNUM;
+ SVN_ERR(check_for_deleted_rev(ra_session,
+ url_or_path, peg_revnum,
+ greater_revnum,
+ &revision_deleted,
+ ctx, pool));
+
+ if (SVN_IS_VALID_REVNUM(revision_deleted))
+ {
+ greater_revnum = revision_deleted - 1;
+
+ /* If start and end revision are beyond deletion
+ * an empty log is returned. */
+ if (start_revnum > greater_revnum && end_revnum > greater_revnum)
+ {
+ return SVN_NO_ERROR;
+ }
+
+ if (start_revnum > greater_revnum)
+ {
+ start_revnum = greater_revnum;
+ }
+ if (end_revnum > greater_revnum)
+ {
+ end_revnum = greater_revnum;
+ }
+ }
+ }
+
SVN_ERR(svn_ra_get_log2(ra_session,
condensed_targets,
start_revnum,
