[PATCH] Tests harness: multiple calls to create_python_hook_script

[Daniel Shahaf <d....@daniel.shahaf.name>]

The svntest/main.py code that installs a post-commit hook for --fsfs-packing=N
documents the following shortcoming:

    # Note that some tests (currently only commit_tests) create their own
    # post-commit hooks, which would override this one. :-(

In general, if a test calls create_python_hook_script() twice with the same
HOOK_PATH argument, all calls but the last are *silently ignored*.  That's bad.

I'm attaching a patch for this.  I ran tests through merge_reintegrate_tests.py
and it doesn't introduce any failures.  I probably won't have time to finish
it, so I'd appreciate it if someone can pick it up, finish it, and commit.

Thanks.

Daniel

[[[
Tests harness: fix a bug whereby a previously-installed hook would be silently
overwritten by a more-recently-installed one.

TODO: before commit, fix commit_tests 15 and 38 under --fsfs-packing

* subversion/tests/cmdline/svntest/main.py
  (errno): Import
  (create_repos): Undocument the bug this commit fixes.
  (create_python_hook_script): Abort if the hook script already exists.
    There is no provision for callers to disable this behaviour.

* subversion/tests/cmdline/commit_tests.py
  (hook_test, post_commit_hook_test): (...)
]]]

[[[
--- subversion/tests/cmdline/svntest/main.py
+++ subversion/tests/cmdline/svntest/main.py
@@ -37,6 +37,7 @@ import urllib
 import logging
 import hashlib
 from urlparse import urlparse
+import errno
 
 try:
   # Python >=3.0
@@ -973,8 +973,6 @@ def create_repos(path, minor_version = None):
       file_write(format_file_path, new_contents, 'wb')
 
     # post-commit
-    # Note that some tests (currently only commit_tests) create their own
-    # post-commit hooks, which would override this one. :-(
     if options.fsfs_packing:
       # some tests chdir.
       abs_path = os.path.abspath(path)
@@ -1080,6 +1081,13 @@ def create_python_hook_script(hook_path, hook_script_code,
   """Create a Python hook script at HOOK_PATH with the specified
      HOOK_SCRIPT_CODE."""
 
+  # Hook scripts belong to a repository.  Each repository is handled by at most
+  # one thread at any given time.  Therefore, an advance check suffices, i.e.,
+  # open(O_EXCL) is not needed here.
+  if os.path.exists(hook_path):
+    raise OSError(errno.EEXIST,
+                  "Won't overwrite hook script at %r" % (hook_path,))
+    
   if windows:
     if cmd_alternative is not None:
       file_write("%s.bat" % hook_path,
--- subversion/tests/cmdline/commit_tests.py
+++ subversion/tests/cmdline/commit_tests.py
@@ -840,6 +840,7 @@ fp.close()"""
   svntest.main.create_python_hook_script(pre_commit_hook,
                                          hook_format % "pre_commit_hook")
 
+  # ### This test and post_commit_hook_test() probably to be changed before commit.
   post_commit_hook = svntest.main.get_post_commit_hook_path(repo_dir)
   svntest.main.create_python_hook_script(post_commit_hook,
                                          hook_format % "post_commit_hook")
]]]