Re: Feature request: hooks directory
flat view
From: Christian Couder <hidden>
Date: 2018-09-03 04:00:12
Hi Wesley, On Sun, Sep 2, 2018 at 11:38 PM, Wesley Schwengle [off-list ref] wrote:
Hi all, I've made some progress with the hook.d implementation. It isn't finished, as it is my first C project I'm still somewhat rocky with how pointers and such work, but I'm getting somewhere. I haven't broken any tests \o/.
Great! Welcome to the Git community!
You have a nice testsuite btw. Feel free to comment on the code. I've moved some of the hooks-code found in run-command.c to hooks.c You can see the progress on gitlab: https://gitlab.com/waterkip/git or on github: https://github.com/waterkip/git The output of format-patch is down below.
It would be nicer if you could give links that let us see more directly the commits you made, for example: https://gitlab.com/waterkip/git/commits/multi-hooks
I have some questions regarding the following two functions in run-command.c: * run_hook_le * run_hook_ve What do the postfixes le and ve mean?
It's about the arguments the function accepts, in a similar way to
exec*() functions, see `man execve` and `man execle`.
In short 'l' means list, 'v' means array of pointer to strings and 'e'
means environment.
format-patch: From 129d8aff8257b22210beadc155cdbcae99b0fc4b Mon Sep 17 00:00:00 2001 From: Wesley Schwengle <redacted> Date: Sun, 2 Sep 2018 02:40:04 +0200 Subject: [PATCH] WIP: Add hook.d support in git
This is not the best way to embed a patch in an email. There is
Documentation/SubmittingPatches in the code base, that should explain
better ways to send patches to the mailing list.
Add a generic mechanism to find and execute one or multiple hooks found
in $GIT_DIR/hooks/<hook> and/or $GIT_DIR/hooks/<hooks>.d/*
The API is as follows:
#include "hooks.h"
array hooks = find_hooks('pre-receive');
int hooks_ran = run_hooks(hooks);
The implemented behaviour is:
* If we find a hooks/<hook>.d directory and the hooks.multiHook flag isn't
set we make use of that directory.
* If we find a hooks/<hook>.d and we also have hooks/<hook> and the
hooks.multiHook isn't set or set to false we don't use the hook.d
directory. If the hook isn't set we issue a warning to the user
telling him/her that we support multiple hooks via the .d directory
structure
* If the hooks.multiHook is set to true we use the hooks/<hook> and all
the entries found in hooks/<hook>.d
* All the scripts are executed and fail on the first errorMaybe the above documentation should be fully embedded as comments in "hooks.h" (or perhaps added to a new file in "Documentation/technical/", though it looks like we prefer to embed doc in header files these days).
quoted hunk ↗ jump to hunk
diff --git a/hooks.h b/hooks.h new file mode 100644 index 000000000..278d63344 --- /dev/null +++ b/hooks.h@@ -0,0 +1,35 @@ +#ifndef HOOKS_H +#define HOOKS_H + +#ifndef NO_PTHREADS +#include <pthread.h> +#endif +/* + * Returns all the hooks found in either + * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/ + * + * Note that this points to static storage that will be + * overwritten by further calls to find_hooks and run_hook_*. + */ +//extern const struct string_list *find_hooks(const char *name);
The above comment is using "//" which we forbid and should probably be
removed anyway.
quoted hunk ↗ jump to hunk
+extern const char *find_hooks(const char *name); + +/* Unsure what this does/is/etc */ +//LAST_ARG_MUST_BE_NULL
This is to make it easier for tools like compilers to check that a
function is used correctly. You should not remove such macros.
+/* + * Run all the runnable hooks found in + * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/ + * + */ +//extern int run_hooks_le(const char *const *env, const char *name, ...); +//extern int run_hooks_ve(const char *const *env, const char *name, va_list args);
Strange that these functions are commented out.
quoted hunk ↗ jump to hunk
+#endif + +/* Workings of hooks + * + * array_of_hooks = find_hooks(name); + * number_of_hooks_ran = run_hooks(array_of_hooks); + * + */
This kind of documentation should probably be at the beginning of the
file, see strbuf.h for example.
Thanks,
Christian.