ra_svn problem with abort_report

Philip Martin <philip@codematters.co.uk> writes:

> Philip Martin <philip@codematters.co.uk> writes:
>
>> After reading svn_repos.h, it appears to be the callers responsibilty.
>> In that case the problem is probably in svn_wc_crawl_revisions, all
>> those SVN_ERRs bypass the abort_report: processing, in this particuar
>> case the reporter->finish_report call is the culprit.
>
> I'm trying this

Now I'm using the attached patch, but it fails over ra_svn. Is the
client allowed to call abort_report after calling finsh_report? The
problem is that svnserve receives the abort_report when it is not
expecting it (at least it does when I add a flush call).

Fix a case where transactions were getting left behind when an update
of an incomplete working copied failed due to obstruction.

* subversion/libsvn_wc/adm_crawler.c
  (wc_crawl_revisions): New function containing the bulk of the old
   svn_wc_crawl_revisions but without the abort_reporter handling.
  (svn_wc_crawl_revisions): Now a wrapper that calls wc_crawl_revisions
   and abort_reporter if there is an error.

* subversion/tests/clients/cmdline/update_tests.py
  (update_receive_illegal_name): Run the update twice and check that no
  txns get left behind.

* subversion/libsvn_ra_svn/client.c (ra_svn_abort_report): Explicitly
  flush the write buffer.

Index: subversion/libsvn_wc/adm_crawler.c
===================================================================
--- subversion/libsvn_wc/adm_crawler.c (revision 7386)
+++ subversion/libsvn_wc/adm_crawler.c (working copy)
@@ -423,20 +423,19 @@
 
 /* This is the main driver of the working copy state "reporter", used
    for updates. */
-svn_error_t *
-svn_wc_crawl_revisions (const char *path,
- svn_wc_adm_access_t *adm_access,
- const svn_ra_reporter_t *reporter,
- void *report_baton,
- svn_boolean_t restore_files,
- svn_boolean_t recurse,
- svn_boolean_t use_commit_times,
- svn_wc_notify_func_t notify_func,
- void *notify_baton,
- svn_wc_traversal_info_t *traversal_info,
- apr_pool_t *pool)
+static svn_error_t *
+wc_crawl_revisions (const char *path,
+ svn_wc_adm_access_t *adm_access,
+ const svn_ra_reporter_t *reporter,
+ void *report_baton,
+ svn_boolean_t restore_files,
+ svn_boolean_t recurse,
+ svn_boolean_t use_commit_times,
+ svn_wc_notify_func_t notify_func,
+ void *notify_baton,
+ svn_wc_traversal_info_t *traversal_info,
+ apr_pool_t *pool)
 {
- svn_error_t *err = SVN_NO_ERROR;
   const svn_wc_entry_t *entry;
   svn_revnum_t base_rev = SVN_INVALID_REVNUM;
   svn_boolean_t missing = FALSE;
@@ -466,7 +465,7 @@
   if (entry->schedule != svn_wc_schedule_delete)
     {
       apr_finfo_t info;
- err = svn_io_stat (&info, path, APR_FINFO_MIN, pool);
+ svn_error_t *err = svn_io_stat (&info, path, APR_FINFO_MIN, pool);
       if (err)
         {
           if (APR_STATUS_IS_ENOENT(err->apr_err))
@@ -481,26 +480,22 @@
         {
           /* Always report directories as missing; we can't recreate
              them locally. */
- err = reporter->delete_path (report_baton, "", pool);
- if (err)
- goto abort_report;
+ SVN_ERR (reporter->delete_path (report_baton, "", pool));
         }
       else
         {
           /* Recursively crawl ROOT_DIRECTORY and report differing
              revisions. */
- err = report_revisions (adm_access,
- "",
- base_rev,
- reporter, report_baton,
- notify_func, notify_baton,
- restore_files, recurse,
- entry->incomplete,
- use_commit_times,
- traversal_info,
- pool);
- if (err)
- goto abort_report;
+ SVN_ERR (report_revisions (adm_access,
+ "",
+ base_rev,
+ reporter, report_baton,
+ notify_func, notify_baton,
+ restore_files, recurse,
+ entry->incomplete,
+ use_commit_times,
+ traversal_info,
+ pool));
         }
     }
 
@@ -511,9 +506,7 @@
       if (missing && restore_files)
         {
           /* Recreate file from text-base. */
- err = restore_file (path, adm_access, use_commit_times, pool);
- if (err)
- goto abort_report;
+ SVN_ERR (restore_file (path, adm_access, use_commit_times, pool));
 
           /* Report the restoration to the caller. */
           if (notify_func != NULL)
@@ -556,16 +549,38 @@
              of the report (not some file in a subdirectory of a target
              directory), and that target is a file, we need to pass an
              empty string to set_path. */
- err = reporter->set_path (report_baton, "", base_rev, FALSE, pool);
- if (err)
- goto abort_report;
+ SVN_ERR (reporter->set_path (report_baton, "", base_rev, FALSE,
+ pool));
         }
     }
 
   /* Finish the report, which causes the update editor to be driven. */
   SVN_ERR (reporter->finish_report (report_baton));
 
- abort_report:
+ return SVN_NO_ERROR;
+}
+
+/* Just a wrapper round wc_crawl_revisions to call abort_report if the
+ former returns an error */
+svn_error_t *
+svn_wc_crawl_revisions (const char *path,
+ svn_wc_adm_access_t *adm_access,
+ const svn_ra_reporter_t *reporter,
+ void *report_baton,
+ svn_boolean_t restore_files,
+ svn_boolean_t recurse,
+ svn_boolean_t use_commit_times,
+ svn_wc_notify_func_t notify_func,
+ void *notify_baton,
+ svn_wc_traversal_info_t *traversal_info,
+ apr_pool_t *pool)
+{
+ svn_error_t *err = wc_crawl_revisions(path, adm_access,
+ reporter, report_baton,
+ restore_files, recurse,
+ use_commit_times,
+ notify_func, notify_baton,
+ traversal_info, pool);
   if (err)
     {
       /* Clean up the fs transaction. */
Index: subversion/tests/clients/cmdline/update_tests.py
===================================================================
--- subversion/tests/clients/cmdline/update_tests.py (revision 7386)
+++ subversion/tests/clients/cmdline/update_tests.py (working copy)
@@ -936,11 +936,21 @@
   svntest.actions.run_and_verify_svn(None, None, [],
                                      'mv', '-m', 'log msg',
                                      legal_url, illegal_url)
- out, err = svntest.main.run_svn(1, 'up', wc_dir)
- for line in err:
- if line.find("Obstructed update") != -1:
- break
- else:
+
+ # Do the update twice, both should fail. After the first failure
+ # the wc will be marked "incomplete".
+ for n in range(2):
+ out, err = svntest.main.run_svn(1, 'up', wc_dir)
+ for line in err:
+ if line.find("Obstructed update") != -1:
+ break
+ else:
+ raise svntest.Failure
+
+ # At one stage an obstructed update in an incomplete wc would leave
+ # a txn behind
+ out, err = svntest.main.run_svnadmin('lstxns', sbox.repo_dir)
+ if out or err:
     raise svntest.Failure
 
 #----------------------------------------------------------------------
Index: subversion/libsvn_ra_svn/client.c
===================================================================
--- subversion/libsvn_ra_svn/client.c (revision 7386)
+++ subversion/libsvn_ra_svn/client.c (working copy)
@@ -234,6 +234,7 @@
   ra_svn_reporter_baton_t *b = baton;
 
   SVN_ERR(svn_ra_svn_write_cmd(b->conn, b->pool, "abort-report", ""));
+ SVN_ERR(svn_ra_svn_flush(b->conn, b->pool));
   return SVN_NO_ERROR;
 }
 

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org