Thank you for your feedback, Philip.
I have attached an updated patch.


A short comment along the lines 'Sets *REVISION_DELETED ...'

I have added a comment for function check_for_deleted_rev.


No whitespace before ')'.

Split long line before '&&  session_opt_revnum'

I have updated the formatting of the source code.


You don't clear err, so if the following two calls return an error then
err leaks.

I have modified the logic how the errors are handled (and now correctly 
cleared).

I needed to introduce an error when the REVISION_DELETED can not be determined:
  return svn_error_create
    (SVN_ERR_CLIENT_BAD_REVISION, NULL,
     _("Could not determine revision where target was deleted"));

A temporary session is used to identify the deleted revision, and then
another session is created to run log.  Since the temporary session
"works" could it be permanent and used for log?

I have modified the code to reuse the iterpool which is later used for fetching 
the log.
I hope it is used correctly - i am not very familiar with this stuff yet.


Is there any more feedback which i should integrate?

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,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. ***/
 
@@ -471,7 +508,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. */
@@ -481,11 +522,58 @@
     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;
+        svn_opt_revision_t session_mod_rev;
+
+        svn_error_clear(err);
+
+        svn_pool_clear(iterpool);
+
+        SVN_ERR(svn_client__ra_session_from_path(&ra_session, &ignored_revnum,
+                                                 &actual_url, ra_target, NULL,
+                                                 &peg_rev, &peg_rev,
+                                                 ctx, iterpool));
+
+        SVN_ERR(check_for_deleted_rev(ra_session,
+                                      url_or_path, 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"));
+          }
+
+        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));
 
@@ -546,10 +634,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, 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 +662,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 +694,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,

Reply via email to