Skip to content

cmd/gitaly-hooks: Resolve hook name by zeroth argument

Patrick Steinhardt requested to merge pks-git-hooks-via-arg0 into master

The old way of executing the gitaly-hooks binary works via a wrapper script which resolves the location of the binary via an environment variable and then executes that script with its initial argument set to the hook name we want to execute. With the new temporary hooks directory created at runtime we're still essentially doing the same thing, except that we don't need the environment variable anymore because we already write the path of the gitaly-hooks binary into the wrapper script directly.

This is needlessly roundabout though: instead of using a wrapper script we can simply create symlinks which directly point to gitaly-hooks. The only problem is that we cannot resolve the hook name in that case because we don't set the first argument to it anymore. This can be worked around though: when executing a symlink, the zeroth argument will be set to the location of the symlink and not to the location of the linked binary. We can thus use it to resolve the hook name instead of having to pass an extra argument. While we need to be able to continue handling non-hook cases like e.g. when executing "gitaly-hooks check", the basename in that case will be gitaly-hooks and thus easily discernable, as well.

Refactor the code to do so. Due to backwards compatibiliy concerns we need to retain both old and new way to resolve the hook name. The upgrade path is easy enough:

- We either have a basename of "gitaly-hooks" in the zeroth argument
  and will then take the first argument as hook name. This is
  currently used by the `gitlab-shell-hook` wrapper script and when
  executing the binary directly.

- Or we have a basename different of "gitaly-hooks", where we then
  use the zeroth argument to resolve the hook name.

The only outlier is the temporary wrapper script we write in via the Git command factory, which both has its zeroth and first arguments set to the hook name. Given that this code is guarded by a feature flag though which hasn't yet been rolled out this is not much of a concern and adjusted while at it.

Furthermore, because the command factory now uses the zeroth argument while the gitlab-shell-hook wrapper uses the first argument to execute the hook, we implicitly test that gitaly-hooks handles both ways as expected because all of our tests randomly switch between both hook setups.

Edited by Patrick Steinhardt

Merge request reports

Loading