You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2003/10/12 14:49:40 UTC
ra_svn problem with abort_report
Philip Martin <ph...@codematters.co.uk> writes:
> Philip Martin <ph...@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
Re: ra_svn problem with abort_report
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> Julian Foad <ju...@btopenworld.com> writes:
>
>>FAIL: update_tests.py 13: bail when receive a file or dir named .svn
>>
>>I'm getting this failure in "make svncheck" but "make check" works
>>properly. It is caused by an obstructed update leaving a dangling
>>transaction.
>
> Are you running an up-to-date svnserve, i.e. the one you just built?
A-ha! No, the one I started on October 6th is still running! Must change my test script to kill the old one and start a new one, or use the run-once mode via SSH.
Thank you.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn problem with abort_report
Posted by Philip Martin <ph...@codematters.co.uk>.
Julian Foad <ju...@btopenworld.com> writes:
> FAIL: update_tests.py 13: bail when receive a file or dir named .svn
>
> I'm getting this failure in "make svncheck" but "make check" works
> properly. It is caused by an obstructed update leaving a dangling
> transaction.
Are you running an up-to-date svnserve, i.e. the one you just built?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn problem with abort_report
Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> Philip Martin <ph...@codematters.co.uk> writes:
>
>>Was my original idea correct? Is svn_repos_finish_report responsible
>>for deleting the transactions if an error occurs driving the update
>>editor?
>
> This also solves the dangling txns problem, and it works over all RA
> layers. From the comment I changed it appears that the original
> behaviour was intentional, so is my change correct?
>
>
> Index: subversion/libsvn_repos/reporter.c
...
FAIL: update_tests.py 13: bail when receive a file or dir named .svn
I'm getting this failure in "make svncheck" but "make check" works properly. It is caused by an obstructed update leaving a dangling transaction.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn problem with abort_report
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:
> Was my original idea correct? Is svn_repos_finish_report responsible
> for deleting the transactions if an error occurs driving the update
> editor?
This also solves the dangling txns problem, and it works over all RA
layers. From the comment I changed it appears that the original
behaviour was intentional, so is my change correct?
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 7386)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -446,6 +446,7 @@
svn_fs_root_t *root1, *root2;
svn_repos_report_baton_t *rbaton = report_baton;
const char *tgt_path;
+ svn_error_t *err1, *err2 = SVN_NO_ERROR, *err3 = SVN_NO_ERROR;
/* If nothing was described, then we have an error */
if (! SVN_IS_VALID_REVNUM (rbaton->txn_base_rev))
@@ -481,26 +482,37 @@
NULL);
/* Drive the update-editor. */
- SVN_ERR (svn_repos_dir_delta (root1,
- rbaton->base_path,
- rbaton->target,
- root2,
- tgt_path,
- rbaton->update_editor,
- rbaton->update_edit_baton,
- rbaton->text_deltas,
- rbaton->recurse,
- TRUE,
- rbaton->ignore_ancestry,
- rbaton->pool));
+ err1 = svn_repos_dir_delta (root1,
+ rbaton->base_path,
+ rbaton->target,
+ root2,
+ tgt_path,
+ rbaton->update_editor,
+ rbaton->update_edit_baton,
+ rbaton->text_deltas,
+ rbaton->recurse,
+ TRUE,
+ rbaton->ignore_ancestry,
+ rbaton->pool);
- /* Still here? Great! Throw out the transactions. */
+ /* Throw out the transactions. */
if (rbaton->txn)
- SVN_ERR (svn_fs_abort_txn (rbaton->txn));
+ err2 = svn_fs_abort_txn (rbaton->txn);
if (rbaton->txn2)
- SVN_ERR (svn_fs_abort_txn (rbaton->txn2));
-
- return SVN_NO_ERROR;
+ err3 = svn_fs_abort_txn (rbaton->txn2);
+
+ if (err1)
+ {
+ svn_error_clear (err3);
+ svn_error_clear (err2);
+ return err1;
+ }
+ if (err2)
+ {
+ svn_error_clear (err3);
+ return err2;
+ }
+ return err3;
}
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn problem with abort_report
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> Philip Martin <ph...@codematters.co.uk> writes:
>
>> 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).
>
> Oops... the docs may not be clear, but I'm certain that when we
> designed the API, we meant abort_report() and finish_report() to be
> mutually exclusive ways of terminating the report procedure. It's not
> a surprise to me that any RA layer behaves this way.
>
> finish_report() is supposed to mean: "I'm completely done with the
> report." abort_report() is supposed to mean: "Never mind, I'm not
> going to finish this report-in-progress."
Was my original idea correct? Is svn_repos_finish_report responsible
for deleting the transactions if an error occurs driving the update
editor? Is the bug that this code in svn_repos_finish_report
/* Drive the update-editor. */
SVN_ERR (svn_repos_dir_delta (root1,
rbaton->base_path,
rbaton->target,
root2,
tgt_path,
rbaton->update_editor,
rbaton->update_edit_baton,
rbaton->text_deltas,
rbaton->recurse,
TRUE,
rbaton->ignore_ancestry,
rbaton->pool));
/* Still here? Great! Throw out the transactions. */
if (rbaton->txn)
SVN_ERR (svn_fs_abort_txn (rbaton->txn));
if (rbaton->txn2)
SVN_ERR (svn_fs_abort_txn (rbaton->txn2));
should always throw out the transactions even if svn_repos_dir_delta
fails?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: ra_svn problem with abort_report
Posted by Ben Collins-Sussman <su...@collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> 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).
Oops... the docs may not be clear, but I'm certain that when we
designed the API, we meant abort_report() and finish_report() to be
mutually exclusive ways of terminating the report procedure. It's not
a surprise to me that any RA layer behaves this way.
finish_report() is supposed to mean: "I'm completely done with the
report." abort_report() is supposed to mean: "Never mind, I'm not
going to finish this report-in-progress."
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org