You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Scott Collins <sc...@mozilla.org> on 2003/12/27 00:28:53 UTC
[PATCH] add pool parameter to (finish|abort)_report APIs (was: Re: Another API wart: {finish,abort}_report pools)
Here's a log comment and patch implementing the API change suggested by
Greg Hudson. One thing this patch does _not_ do is remove the
remembered pool from the reporter baton, nor change uses of that
remembered pool. Since I could make this API change without doing
that, and since the result builds and passes all tests, I figured such
a change might be logically distinct and deserve its own patch. It
almost certainly deserves its own discussion.
Greg Hudson wrote (in part):
> and enter it as a 1.0 candidate
Does that mean I should file an issue, note the issue in this log
comment, and do some other things of which I'm not exactly sure (e.g.,
attach the patch to the issue, set some flag on the issue)? I'm happy
to do so, just tell me what or where to look to figure it out for
myself. Also, although this patch seems pretty mechanical to me, but
I'm ready to act on any feedback.
Since I've included the patch in-line, I elected not to PGP sign this
message.
Hope this helps,
__________
Scott Collins <http://ScottCollins.net/>
[[[
Add a pool parameter to the finish_report and abort_report families of
signatures.
* subversion/include/svn_repos.h
(svn_repos_finish_report, svn_repos_abort_report): Added pool
parameter.
* subversion/include/svn_ra.h
(svn_ra_reporter_t): Added pool parameter to the finish_report and
abort_report members.
* subversion/libsvn_ra_local/ra_plugin.c
(reporter_finish_report, reporter_abort_report): Added a pool
parameter and
fixed their calls to svn_repos_finish_report and
svn_repos_abort_report to
pass the pool.
* subversion/libsvn_repos/reporter.c
(finish_report, svn_repos_finish_report, svn_repos_abort_report):
Added a pool
parameter.
* subversion/libsvn_ra_svn/client.c
(ra_svn_finish_report, ra_svn_abort_report): Added a pool parameter.
* subversion/libsvn_ra_dav/fetch.c
(reporter_finish_report, reporter_abort_report): Added a pool
parameter.
* subversion/libsvn_wc/adm_crawler.c
* subversion/libsvn_client/export.c
* subversion/libsvn_client/diff.c
* contrib/client-side/svn-push/svn-push.c
* subversion/mod_dav_svn/update.c
* subversion/svnserve/serve.c
Fixed all callers.
]]]
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 8089)
+++ subversion/include/svn_repos.h (working copy)
@@ -322,7 +322,8 @@
* aborted even if the editor drive fails, so the caller does not need
* to clean up.
*/
-svn_error_t *svn_repos_finish_report (void *report_baton);
+svn_error_t *svn_repos_finish_report (void *report_baton,
+ apr_pool_t *pool);
/** The report-driver is bailing, so abort the fs transaction. This
@@ -330,7 +331,8 @@
* called. No other reporting functions should be called after calling
* this function.
*/
-svn_error_t *svn_repos_abort_report (void *report_baton);
+svn_error_t *svn_repos_abort_report (void *report_baton,
+ apr_pool_t *pool);
/* ---------------------------------------------------------------*/
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 8089)
+++ subversion/include/svn_ra.h (working copy)
@@ -163,12 +163,14 @@
* or files not explicitly `set' above are assumed to be at the
* baseline revision originally passed into @c do_update().
*/
- svn_error_t *(*finish_report) (void *report_baton);
+ svn_error_t *(*finish_report) (void *report_baton,
+ apr_pool_t *pool);
/** If an error occurs during a report, this routine should cause the
* filesystem transaction to be aborted & cleaned up.
*/
- svn_error_t *(*abort_report) (void *report_baton);
+ svn_error_t *(*abort_report) (void *report_baton,
+ apr_pool_t *pool);
} svn_ra_reporter_t;
Index: subversion/libsvn_wc/adm_crawler.c
===================================================================
--- subversion/libsvn_wc/adm_crawler.c (revision 8089)
+++ subversion/libsvn_wc/adm_crawler.c (working copy)
@@ -563,14 +563,14 @@
}
/* Finish the report, which causes the update editor to be driven. */
- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));
abort_report:
if (err)
{
/* Clean up the fs transaction. */
svn_error_t *fserr;
- if ((fserr = reporter->abort_report (report_baton)))
+ if ((fserr = reporter->abort_report (report_baton, pool)))
{
fserr = svn_error_quick_wrap (fserr, "Error aborting
report");
svn_error_compose (err, fserr);
Index: subversion/libsvn_ra_local/ra_plugin.c
===================================================================
--- subversion/libsvn_ra_local/ra_plugin.c (revision 8089)
+++ subversion/libsvn_ra_local/ra_plugin.c (working copy)
@@ -101,18 +101,20 @@
static svn_error_t *
-reporter_finish_report (void *reporter_baton)
+reporter_finish_report (void *reporter_baton,
+ apr_pool_t *pool)
{
reporter_baton_t *rbaton = reporter_baton;
- return svn_repos_finish_report (rbaton->report_baton);
+ return svn_repos_finish_report (rbaton->report_baton, pool);
}
static svn_error_t *
-reporter_abort_report (void *reporter_baton)
+reporter_abort_report (void *reporter_baton,
+ apr_pool_t *pool)
{
reporter_baton_t *rbaton = reporter_baton;
- return svn_repos_abort_report (rbaton->report_baton);
+ return svn_repos_abort_report (rbaton->report_baton, pool);
}
Index: subversion/libsvn_client/export.c
===================================================================
--- subversion/libsvn_client/export.c (revision 8089)
+++ subversion/libsvn_client/export.c (working copy)
@@ -697,7 +697,7 @@
TRUE, /* "help, my dir is empty!" */
pool));
- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));
/* Special case: Due to our sly export/checkout method of
* updating an empty directory, no target will have been created
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 8089)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -1274,7 +1274,7 @@
SVN_ERR (reporter->set_path (report_baton, "", start_revnum, FALSE,
pool));
- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));
return SVN_NO_ERROR;
}
@@ -1641,7 +1641,7 @@
/* Drive the reporter; do the diff. */
SVN_ERR (reporter->set_path (report_baton, "", rev1, FALSE, pool));
- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));
return SVN_NO_ERROR;
}
Index: subversion/mod_dav_svn/update.c
===================================================================
--- subversion/mod_dav_svn/update.c (revision 8089)
+++ subversion/mod_dav_svn/update.c (working copy)
@@ -1274,7 +1274,7 @@
/* we require the `rev' attribute for this to make sense */
if (! SVN_IS_VALID_REVNUM (rev))
{
- svn_error_clear(svn_repos_abort_report(rbaton));
+ svn_error_clear(svn_repos_abort_report(rbaton,
resource->pool));
serr = svn_error_create (SVN_ERR_XML_ATTRIB_NOT_FOUND,
NULL, "Missing XML attribute:
rev");
return dav_svn_convert_err(serr,
HTTP_INTERNAL_SERVER_ERROR,
@@ -1294,7 +1294,7 @@
start_empty, resource->pool);
if (serr != NULL)
{
- svn_error_clear(svn_repos_abort_report(rbaton));
+ svn_error_clear(svn_repos_abort_report(rbaton,
resource->pool));
return dav_svn_convert_err(serr,
HTTP_INTERNAL_SERVER_ERROR,
"A failure occurred while "
"recording one of the items
of "
@@ -1326,7 +1326,7 @@
serr = svn_repos_delete_path(rbaton, path, resource->pool);
if (serr != NULL)
{
- svn_error_clear(svn_repos_abort_report(rbaton));
+ svn_error_clear(svn_repos_abort_report(rbaton,
resource->pool));
return dav_svn_convert_err(serr,
HTTP_INTERNAL_SERVER_ERROR,
"A failure occurred while "
"recording one of the
(missing) "
@@ -1338,7 +1338,7 @@
/* this will complete the report, and then drive our editor to
generate
the response to the client. */
- serr = svn_repos_finish_report(rbaton);
+ serr = svn_repos_finish_report(rbaton, resource->pool);
if (serr)
return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
"A failure occurred while "
@@ -1408,7 +1408,7 @@
resource-walker... */
if (serr != NULL)
{
- svn_error_clear(svn_repos_abort_report(rbaton));
+ svn_error_clear(svn_repos_abort_report(rbaton, resource->pool));
return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
"A failure occurred during the
completion "
"and response generation for the
update "
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 8089)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -430,7 +430,8 @@
* any txns being aborted.
*/
static svn_error_t *
-finish_report (void *report_baton)
+finish_report (void *report_baton,
+ apr_pool_t *pool)
{
svn_fs_root_t *root1, *root2;
report_baton_t *rbaton = report_baton;
@@ -491,10 +492,11 @@
* finish_report returns an error.
*/
svn_error_t *
-svn_repos_finish_report (void *report_baton)
+svn_repos_finish_report (void *report_baton,
+ apr_pool_t *pool)
{
- svn_error_t *err1 = finish_report (report_baton);
- svn_error_t *err2 = svn_repos_abort_report (report_baton);
+ svn_error_t *err1 = finish_report (report_baton, pool);
+ svn_error_t *err2 = svn_repos_abort_report (report_baton, pool);
if (err1)
{
svn_error_clear (err2);
@@ -505,7 +507,8 @@
svn_error_t *
-svn_repos_abort_report (void *report_baton)
+svn_repos_abort_report (void *report_baton,
+ apr_pool_t *pool)
{
report_baton_t *rbaton = report_baton;
Index: subversion/libsvn_ra_svn/client.c
===================================================================
--- subversion/libsvn_ra_svn/client.c (revision 8089)
+++ subversion/libsvn_ra_svn/client.c (working copy)
@@ -345,7 +345,8 @@
return SVN_NO_ERROR;
}
-static svn_error_t *ra_svn_finish_report(void *baton)
+static svn_error_t *ra_svn_finish_report(void *baton,
+ apr_pool_t *pool)
{
ra_svn_reporter_baton_t *b = baton;
@@ -357,7 +358,8 @@
return SVN_NO_ERROR;
}
-static svn_error_t *ra_svn_abort_report(void *baton)
+static svn_error_t *ra_svn_abort_report(void *baton,
+ apr_pool_t *pool)
{
ra_svn_reporter_baton_t *b = baton;
Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 8089)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -2239,7 +2239,8 @@
}
-static svn_error_t * reporter_abort_report(void *report_baton)
+static svn_error_t * reporter_abort_report(void *report_baton,
+ apr_pool_t *pool)
{
report_baton_t *rb = report_baton;
@@ -2249,7 +2250,8 @@
}
-static svn_error_t * reporter_finish_report(void *report_baton)
+static svn_error_t * reporter_finish_report(void *report_baton,
+ apr_pool_t *pool)
{
report_baton_t *rb = report_baton;
svn_error_t *err;
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 8089)
+++ subversion/svnserve/serve.c (working copy)
@@ -290,7 +290,7 @@
/* No arguments to parse. */
SVN_ERR(trivial_auth_request(conn, pool, b->sb));
if (!b->err)
- b->err = svn_repos_finish_report(b->report_baton);
+ b->err = svn_repos_finish_report(b->report_baton, pool);
return SVN_NO_ERROR;
}
@@ -300,7 +300,7 @@
report_driver_baton_t *b = baton;
/* No arguments to parse. */
- svn_error_clear(svn_repos_abort_report(b->report_baton));
+ svn_error_clear(svn_repos_abort_report(b->report_baton, pool));
return SVN_NO_ERROR;
}
Index: contrib/client-side/svn-push/svn-push.c
===================================================================
--- contrib/client-side/svn-push/svn-push.c (revision 8089)
+++ contrib/client-side/svn-push/svn-push.c (working copy)
@@ -135,7 +135,7 @@
SVN_ERR (reporter->set_path (report_baton, "", start_rev, 0, pool));
- SVN_ERR (reporter->finish_report (report_baton));
+ SVN_ERR (reporter->finish_report (report_baton, pool));
return SVN_NO_ERROR;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Casting 1.0 votes was Re: [PATCH] add pool parameter to
(finish|abort)_report APIs (was: Re: Another API wart:
{finish,abort}_report pools)
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sat, 2003-12-27 at 04:47, Justin Erenkrantz wrote:
> --On Saturday, December 27, 2003 1:40 AM -0500 Greg Hudson <gh...@MIT.EDU>
> wrote:
>
> > file. (I'll vote for it on the 8th, since Karl has asked that we not
> > vote until we can see the whole list of what's being proposed.)
>
> Err, when (and why) did he say that? I missed that.
>
Hmm... I guess I took that from the gestalt of all of his comments about
1.0 process, and from his behavior. I can't find a specific request in
the mail archive.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Casting 1.0 votes
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Saturday, December 27, 2003 1:54 AM -0800 Ben Reser <be...@reser.org> wrote:
> He didn't. He said votes needed to be cast by the 8th. He asked
> everyone to wait until all the issues were determined before voting
> however.
My take is that we're unlikely to say, "Oh, *now* here's all of the candidates
for 1.0." It'll never happen because someone will always add things they want
until the deadline. And, I think if they get the 3 +1s before our deadline,
then more power to them - it should go in.
After reviewing what's been proposed in STATUS so far, my take is that they
either fall into the 'obvious' correctness fix (which we'd want anyway in our
next public release), or they are specifically geared to expanding our API in
a model that we'd want to support once we 'hit' 1.0 and beyond.
So, I think for the most part, people have the right scope in the issues
listed in STATUS. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Casting 1.0 votes was Re: [PATCH] add pool parameter to (finish|abort)_report APIs (was: Re: Another API wart: {finish,abort}_report pools)
Posted by kf...@collab.net.
Ben Reser <be...@reser.org> writes:
> He didn't. He said votes needed to be cast by the 8th. He asked
> everyone to wait until all the issues were determined before voting
> however.
s/all/mostly all/, yeah. I mean, a new bug could be found at any
time, or an API problem realized at any time. But it does look like
we've got most things listed by now.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Casting 1.0 votes was Re: [PATCH] add pool parameter to (finish|abort)_report APIs (was: Re: Another API wart: {finish,abort}_report pools)
Posted by Ben Reser <be...@reser.org>.
On Sat, Dec 27, 2003 at 01:47:33AM -0800, Justin Erenkrantz wrote:
> Err, when (and why) did he say that? I missed that.
He didn't. He said votes needed to be cast by the 8th. He asked
everyone to wait until all the issues were determined before voting
however.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Casting 1.0 votes was Re: [PATCH] add pool parameter to
(finish|abort)_report APIs (was: Re: Another API wart: {finish,abort}_report
pools)
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Saturday, December 27, 2003 1:40 AM -0500 Greg Hudson <gh...@MIT.EDU>
wrote:
> file. (I'll vote for it on the 8th, since Karl has asked that we not
> vote until we can see the whole list of what's being proposed.)
Err, when (and why) did he say that? I missed that.
I was going through the httpd STATUS file and casting votes there. So, since
I was in patch review mode already, I just went through the Subversion list
too.
I don't see why it'd hurt if people cast votes now if they have the
inclination now. *shrug* -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] add pool parameter to (finish|abort)_report APIs (was: Re: Another API wart: {finish,abort}_report pools)
Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> Thanks! The patch looks good to me; I've entered it into the status
> file. (I'll vote for it on the 8th, since Karl has asked that we not
> vote until we can see the whole list of what's being proposed.)
Oops, I didn't mean to say that. Earlier, I had asked that we hold
off voting until the majority of issues were listed. That criterion
has been met for some time (we had an actual date, which has passed --
I don't remember exactly what day it was). IMHO, go ahead and vote.
I would, just haven't had a chance to thoroughly review any patches
yet.
We do want voting to be *done* by the 8th, so we can then set a date
for 1.0. That was what I meant to convey, sorry if I was confusing.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] add pool parameter to (finish|abort)_report APIs (was:
Re: Another API wart: {finish,abort}_report pools)
Posted by Greg Hudson <gh...@MIT.EDU>.
Thanks! The patch looks good to me; I've entered it into the status
file. (I'll vote for it on the 8th, since Karl has asked that we not
vote until we can see the whole list of what's being proposed.)
Your log message and patch were word-wrapped in your mail message; the
copies in the issue are fine, but you might figure out how to get your
mailer to turn off word-wrapping, or send patches as text attachments.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] add pool parameter to (finish|abort)_report APIs (was: Re: Another API wart: {finish,abort}_report pools)
Posted by Scott Collins <sc...@ScottCollins.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Issue #1671 filed, patch attached, targeted as 1.0-candidate. See
http://subversion.tigris.org/issues/show_bug.cgi?id=1671
__________
Scott Collins <http://ScottCollins.net/>
-----BEGIN PGP SIGNATURE-----
Version: PGP 8.0.3
iQA/AwUBP+0YhcN8VLjezbXrEQLZ6ACfV/dkpon2AqkgrPxC4DbCX6kXYVgAnAoc
39P3GnC4O/5Mnuvk3E62Q9gU
=6X+T
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org