Re: [PATCH] Issue 443 : post-commit hook (error) output lost: Step 6

On 10/19/05, Madan U Sreenivasan <madan@collab.net> wrote:
> Hi,
> Pl. find attached the final step for issue no. 443(oh, well, actually,
> the pre-revprop part has not been fixed yet).

I haven't actually tried the patch yet, but here are a few comments
that occurred to me as I was reading over it.

+ /* get the post-commit hook stderr, if any */

+ if (post_commit_err)

+ {

+ post_commit_header_info = apr_psprintf (pool,

+ " xmlns:S=\"%s\"",

+ SVN_XML_NAMESPACE);

+ post_commit_err_elem = apr_psprintf (pool,

+ "<S:post-commit-err>%s"

+ "</S:post-commit-err>",

+ post_commit_err);

+ }

+ else

+ {

+ post_commit_header_info = "" ;

+ post_commit_err_elem = "" ;

Kind of weird having a space before the semicolons there, don't think
I've seen that in any other svn code.

Index: subversion/tests/clients/cmdline/commit_tests.py

===================================================================

--- subversion/tests/clients/cmdline/commit_tests.py (revision 16776)

+++ subversion/tests/clients/cmdline/commit_tests.py (working copy)

@@ -1955,6 +1955,47 @@

                                      os.path.join(wc_dir, 'A', 'yu'))

+#----------------------------------------------------------------------

+# Test if the post-commit error message is returned back to the svn

+# client and is displayed as a warning.

+#

+def post_commit_hook_test(sbox):

+ "post commit hook failure case testing"

+

+ sbox.build()

+

+ # Get paths to the working copy and repository

+ wc_dir = sbox.wc_dir

+ repo_dir = sbox.repo_dir

+

+ # Setup the hook configs to echo data back

+ post_commit_hook = svntest.main.get_post_commit_hook_path (repo_dir)

+ svntest.main.file_append (post_commit_hook,

+ """#!/bin/sh

+ echo "Post-commit Hook says nothing doing
on stderr" > /dev/stderr

+ exit -1

+ """)

+ os.chmod (post_commit_hook, 0755)

This test won't work on windows... I'm not sure how other tests that
include hook scripts work, but looking at what they do seems like a
reasonable place to start. Also, the echo line is wrapping past 80
columns, that should probably be fixed.
@@ -899,7 +902,10 @@

                            apr_array_header_t *params, void *baton)

 {

   server_baton_t *b = baton;

- const char *log_msg, *date, *author;

+ const char *log_msg = NULL,

+ *date = NULL,

+ *author = NULL,

+ *post_commit_err = NULL;

Any reason these are now being initialized to NULL?

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Oct 19 23:01:53 2005