You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2005/02/18 17:45:28 UTC
Re: svn commit: r12961 - in trunk/subversion: include libsvn_repos mod_dav_svn svnadmin
cmpilato@tigris.org writes:
> Modified:
> trunk/subversion/include/svn_repos.h
> trunk/subversion/libsvn_repos/commit.c
> trunk/subversion/libsvn_repos/hooks.c
> trunk/subversion/libsvn_repos/load.c
> trunk/subversion/mod_dav_svn/version.c
> trunk/subversion/svnadmin/main.c
> Log:
>
> [...]
>
> * subversion/libsvn_repos/hooks.c
> (run_hook_cmd): Rename 'check_exitcode' to 'read_errstream'.
Looks like you did more than just rename the parameter, you also
rearranged some code that uses it, no?
> --- trunk/subversion/libsvn_repos/hooks.c (original)
> +++ trunk/subversion/libsvn_repos/hooks.c Wed Feb 9 22:25:04 2005
> @@ -36,11 +36,11 @@
> /*** Hook drivers. ***/
>
> /* NAME, CMD and ARGS are the name, path to and arguments for the hook
> - program that is to be run. If CHECK_EXITCODE is TRUE then the hook's
> + program that is to be run. If READ_ERRSTREAM is TRUE then the hook's
> exit status will be checked, and if an error occurred the hook's stderr
> output will be added to the returned error.
>
> - If CHECK_EXITCODE is FALSE the hook's exit status will be ignored.
> + If READ_ERRSTREAM is FALSE the hook's exit status will be ignored.
>
> If STDIN_HANDLE is non-null, pass it as the hook's stdin, else pass
> no stdin to the hook. */
> @@ -48,7 +48,7 @@
> run_hook_cmd (const char *name,
> const char *cmd,
> const char **args,
> - svn_boolean_t check_exitcode,
> + svn_boolean_t read_errstream,
> apr_file_t *stdin_handle,
> apr_pool_t *pool)
> {
> @@ -90,20 +90,30 @@
> (SVN_ERR_REPOS_HOOK_FAILURE, err, _("Failed to run '%s' hook"), cmd);
> }
>
> - if (!err && check_exitcode)
> + if (!err)
> {
> /* Command failed. */
> if (! APR_PROC_CHECK_EXIT (exitwhy) || exitcode != 0)
> {
> svn_stringbuf_t *error;
>
> - /* Read the file's contents into a stringbuf, allocated in POOL. */
> - SVN_ERR (svn_stringbuf_from_aprfile (&error, read_errhandle, pool));
> -
> - err = svn_error_createf
> - (SVN_ERR_REPOS_HOOK_FAILURE, err,
> - _("'%s' hook failed with error output:\n%s"),
> - name, error->data);
> + if (read_errstream)
> + {
> + /* Read the file's contents into a stringbuf, allocated
> + in POOL. */
> + SVN_ERR (svn_stringbuf_from_aprfile (&error, read_errhandle,
> + pool));
> + err = svn_error_createf
> + (SVN_ERR_REPOS_HOOK_FAILURE, err,
> + _("'%s' hook failed with error output:\n%s"),
> + name, error->data);
> + }
> + else
> + {
> + err = svn_error_createf
> + (SVN_ERR_REPOS_HOOK_FAILURE, err,
> + _("'%s' hook failed; no error output available"), name);
> + }
> }
> }
The above is what I'm talking about.
> --- trunk/subversion/libsvn_repos/load.c (original)
> +++ trunk/subversion/libsvn_repos/load.c Wed Feb 9 22:25:04 2005
> @@ -1183,9 +1185,22 @@
> new_rev = old_rev + 1;
> *old_rev = rb->rev;
>
> - err = svn_fs_commit_txn (&conflict_msg, &(*new_rev), rb->txn, rb->pool);
Huh, I wonder why we ever had "&(*new_rev)" instead of just "new_rev"
there. Oh well, gone now.
> @@ -1322,8 +1349,30 @@
> parent_dir,
> pool));
>
> + /* Heh. We know this is a parse_baton. This file made it. So
> + cast away, and set our hook booleans. */
> + pb = parse_baton;
> + pb->use_pre_commit_hook = use_pre_commit_hook;
> + pb->use_post_commit_hook = use_post_commit_hook;
> +
> SVN_ERR (svn_repos_parse_dumpstream2 (dumpstream, parser, parse_baton,
> cancel_func, cancel_baton, pool));
I don't really understand what that comment is trying to say, but the
code looks fine :-).
> --- trunk/subversion/mod_dav_svn/version.c (original)
> +++ trunk/subversion/mod_dav_svn/version.c Wed Feb 9 22:25:04 2005
> @@ -1163,7 +1163,13 @@
> /* all righty... commit the bugger. */
> serr = svn_repos_fs_commit_txn(&conflict, source->info->repos->repos,
> &new_rev, txn, pool);
> - if (serr != NULL)
> +
> + /* If the error was just a post-commit hook failure, we ignore it.
> + Otherwise, we deal with it.
> + ### TODO: Figure out if the MERGE response can grow a means by
> + which to marshal back both the success of the commit (and its
> + commit info) and the failure of the post-commit hook. */
> + if (serr && (serr->apr_err != SVN_ERR_REPOS_POST_COMMIT_HOOK_FAILED))
> {
> const char *msg;
> svn_error_clear(svn_fs_abort_txn(txn, pool));
> @@ -1182,6 +1188,8 @@
>
> return dav_svn_convert_err(serr, HTTP_CONFLICT, msg, pool);
> }
> + else if (serr)
> + svn_error_clear(serr);
>
> /* Commit was successful, so schedule deltification. */
> register_deltification_cleanup(source->info->repos->repos, new_rev,
Dang, you stud, you even preserved the coding style of that file.
"Ladies and gentlemen, cmpilato has LEFT THE BUILDING!"
Nice commit.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12961 - in trunk/subversion: include libsvn_repos
mod_dav_svn svnadmin
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
>cmpilato@tigris.org writes:
>
>
>>+ else if (serr)
>>+ svn_error_clear(serr);
>>
>> /* Commit was successful, so schedule deltification. */
>> register_deltification_cleanup(source->info->repos->repos, new_rev,
>>
>>
>
>Dang, you stud, you even preserved the coding style of that file.
>
>"Ladies and gentlemen, cmpilato has LEFT THE BUILDING!"
>
>
... leaving behind an unnecessary test before calling svn_error_clear...
:-)
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r12961 - in trunk/subversion: include libsvn_repos mod_dav_svn svnadmin
Posted by "C. Michael Pilato" <cm...@collab.net>.
kfogel@collab.net writes:
> > * subversion/libsvn_repos/hooks.c
> > (run_hook_cmd): Rename 'check_exitcode' to 'read_errstream'.
>
> Looks like you did more than just rename the parameter, you also
> rearranged some code that uses it, no?
Fixed.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org