ssh: Fix overwriting shared gitaly-hooks binary

With the upcoming bundled Git changes, we'll start to see failures in
the ssh tests. The test failures are such that some tests seemingly
weren't able to find the Git executable anymore, but interestingly
enough output also contained references to a temporary directory which
ought to contain an "output" file. As it turns out, this output stems
from a testcase which writes its own gitaly-hooks script. So somehow,
this script is leaking into other tests. And given that this script also
contains the path to the Git binary hosted in our current temporary Git
execution env, it's not much of a surprise that subsequent tests cannot
find it anymore.

As it turns out, the root cause for this is the recent introduction of
using hardlinks for linking binaries which were built just-in-time into
place in a073666ab (testcfg: Try to fix ETXTBSY errors with
gitaly-git2go executable, 2021-11-03). Because the test is first
building gitaly-hooks and then overwrites the resulting binary again,
the end result is that we overwrite the shared gitaly-hooks binary.

Fix this issue by not building the gitaly-hooks binary anymore.
Furthermore, to help avoid such issues in the future,
`WriteExecutable()` as changed such that it will fail if the target
file exists already. This requires one more fix for merge tests, which
write the same script Git hook twice.

2 files changed, 12 insertions, 5 deletions

diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go
index 02d8893cf..3acc5e46f 100644
--- a/internal/gitaly/service/ssh/upload_pack_test.go
+++ b/internal/gitaly/service/ssh/upload_pack_test.go
@@ -279,11 +279,11 @@ func TestUploadPackWithPackObjectsHook(t *testing.T) {
 	outputPath := filepath.Join(filterDir, "output")
 	cfg.BinDir = filterDir
 
-	testcfg.BuildGitalyHooks(t, cfg)
 	testcfg.BuildGitalySSH(t, cfg)
 
 	hookScript := fmt.Sprintf(
-		`#!/bin/sh
+		`#!/bin/bash
+set -eo pipefail
 echo 'I was invoked' >'%s'
 shift
 exec '%s' "$@"
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 759bf9b06..f54a992ed 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -1,6 +1,7 @@
 package testhelper
 
 import (
+	"bytes"
 	"context"
 	"crypto/ecdsa"
 	"crypto/elliptic"
@@ -195,12 +196,18 @@ func TempDir(t testing.TB) string {
 // immediately after they are returned from a test helper
 type Cleanup func()
 
-// WriteExecutable ensures that the parent directory exists, and writes an executable with provided content
+// WriteExecutable ensures that the parent directory exists, and writes an executable with provided
+// content. The executable must not exist previous to writing it.
 func WriteExecutable(t testing.TB, path string, content []byte) {
 	dir := filepath.Dir(path)
-
 	require.NoError(t, os.MkdirAll(dir, 0o755))
-	require.NoError(t, os.WriteFile(path, content, 0o755))
+
+	executable, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o755)
+	require.NoError(t, err)
+	defer MustClose(t, executable)
+
+	_, err = io.Copy(executable, bytes.NewReader(content))
+	require.NoError(t, err)
 
 	t.Cleanup(func() {
 		assert.NoError(t, os.RemoveAll(dir))