[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r38686 - in trunk/subversion: include libsvn_ra_svn svnsync

From: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 11 Aug 2009 22:10:22 +0100

On Tue, Aug 11, 2009 at 01:36:51PM -0700, Ben Collins-Sussman wrote:
> Author: sussman
> Date: Tue Aug 11 13:36:50 2009
> New Revision: 38686
>
> Log:
> Fix two serious bugs in libsvn_ra_svn and svnsync.

I only have stylistic nits.

> Modified: trunk/subversion/include/svn_error_codes.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_error_codes.h?pathrev=38686&r1=38685&r2=38686
> ==============================================================================
> --- trunk/subversion/include/svn_error_codes.h Tue Aug 11 13:27:24 2009 (r38685)
> +++ trunk/subversion/include/svn_error_codes.h Tue Aug 11 13:36:50 2009 (r38686)
> @@ -929,11 +929,17 @@ SVN_ERROR_START
> SVN_ERR_RA_SVN_CATEGORY_START + 6,
> "Client/server version mismatch")
>
> - /** @since New in 1.5. */
> + /** @since New in 1.5. */

Indentation here looks off.

> SVN_ERRDEF(SVN_ERR_RA_SVN_NO_MECHANISMS,
> SVN_ERR_RA_SVN_CATEGORY_START + 7,
> "Cannot negotiate authentication mechanism")
>
> + /** @since New in 1.6.5 / 1.7 */

Same here.

> + SVN_ERRDEF(SVN_ERR_RA_SVN_EDIT_ABORTED,
> + SVN_ERR_RA_SVN_CATEGORY_START + 8,
> + "Editor drive was aborted")
> +
> +
> /* libsvn_ra_serf errors */
> /** @since New in 1.5. */
> SVN_ERRDEF(SVN_ERR_RA_SERF_SSPI_INITIALISATION_FAILED,
>
> Modified: trunk/subversion/libsvn_ra_svn/client.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_svn/client.c?pathrev=38686&r1=38685&r2=38686
> ==============================================================================
> --- trunk/subversion/libsvn_ra_svn/client.c Tue Aug 11 13:27:24 2009 (r38685)
> +++ trunk/subversion/libsvn_ra_svn/client.c Tue Aug 11 13:36:50 2009 (r38686)
> @@ -2253,6 +2253,7 @@ ra_svn_replay_range(svn_ra_session_t *se
> svn_ra_svn__session_baton_t *sess = session->priv;
> apr_pool_t *iterpool;
> svn_revnum_t rev;
> + svn_boolean_t drive_aborted = FALSE;
>
> SVN_ERR(svn_ra_svn_write_cmd(sess->conn, pool, "replay-range", "rrrb",
> start_revision, end_revision,
> @@ -2288,7 +2289,14 @@ ra_svn_replay_range(svn_ra_session_t *se
> iterpool));
> SVN_ERR(svn_ra_svn_drive_editor2(sess->conn, iterpool,
> editor, edit_baton,
> - NULL, TRUE));
> + &drive_aborted, TRUE));
> + /* If drive_editor2() aborted the commit, do NOT try to call
> + revfinish_func and commit the transaction! */
> + if (drive_aborted) {

I like brace-on-same-line better, too, but HACKING disagrees.

> + svn_pool_destroy(iterpool);
> + return svn_error_create(SVN_ERR_RA_SVN_EDIT_ABORTED, NULL,
> + _("Commit error during replay_range()"));

We rarely mention function names in error messages, especially
in those marked for translation. I can already see the german version
"Commit Fehler während wiederhole_intervall()" and while this looks funny
it's not user-friendly.

How about: _("Commit error while replaying revision range")); ?

Stefan

> + }
> SVN_ERR(revfinish_func(rev, replay_baton,
> editor, edit_baton,
> rev_props,

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2382677
Received on 2009-08-11 23:10:49 CEST

This is an archived mail posted to the Subversion Dev mailing list.