core.hooksPath cannot revert to the default locally

core.hooksPath cannot revert to the default locally

[Sam Newbold]

From my testing, reading the documentation and hunting through the
source code, it appears that if a system administrator sets the
--system core.hooksPath or a script over which you have limited control
sets the --global core.hooksPath, then there is no way for an
individual repository to configure --local core.hooksPath to get the
default behavior. The naive solution of setting core.hooksPath to be
.git/hooks breaks in the case of worktrees.

I would be happy to code a patch for this if I knew how it should be
configured. Obviously, no local setting for a config variable is meant
to inherit the global or system value. We can go all "set of the null
set" by making the empty string mean "revert to default". We could go
fancy "I know what you really mean" by interpreting ".git/hooks" (or
perhaps any path beginning with ".git") not literally in the case of
worktrees, but as relative to GIT_DIR. Or there could be another
setting set on a local repository to ignore global and system values of
core.hooksPath.

Thank you,
Sam Newbold

Re: core.hooksPath cannot revert to the default locally

[Phillip Wood]

Hi Sam

On 16/10/2024 07:26, Sam Newbold wrote:
>  From my testing, reading the documentation and hunting through the
> source code, it appears that if a system administrator sets the
> --system core.hooksPath or a script over which you have limited control
> sets the --global core.hooksPath, then there is no way for an
> individual repository to configure --local core.hooksPath to get the
> default behavior. The naive solution of setting core.hooksPath to be
> .git/hooks breaks in the case of worktrees.

I'm pretty sure I've used "git -c core.hookspath=/dev/null ..." in the 
past when I've wanted to temporarily disable my hooks path, I can't find 
anything in the documentation though. If

     git config core.hookspath /dev/null

does work for you it would be really helpful to update the documentation.

Best Wishes

Phillip

> I would be happy to code a patch for this if I knew how it should be
> configured. Obviously, no local setting for a config variable is meant
> to inherit the global or system value. We can go all "set of the null
> set" by making the empty string mean "revert to default". We could go
> fancy "I know what you really mean" by interpreting ".git/hooks" (or
> perhaps any path beginning with ".git") not literally in the case of
> worktrees, but as relative to GIT_DIR. Or there could be another
> setting set on a local repository to ignore global and system values of
> core.hooksPath.
> 
> Thank you,
> Sam Newbold
> 

Re: core.hooksPath cannot revert to the default locally

[Phillip Wood]

On 16/10/2024 10:03, Phillip Wood wrote:
> I'm pretty sure I've used "git -c core.hookspath=/dev/null ..."

Looking at the code that will unfortunately disable all hooks including
the ones in .git/hooks. It would make sense to allow an empty
core.hooksPath mean "use .git/hooks". If you're interested in working
on this then I think doing something like

diff --git a/config.c b/config.c
index a11bb85da30..91f190a1ce1 100644
--- a/config.c
+++ b/config.c
@@ -1437,7 +1437,10 @@ static int git_default_core_config(const char *var, const char *value,
  
          if (!strcmp(var, "core.hookspath")) {
                  FREE_AND_NULL(git_hooks_path);
-                return git_config_pathname(&git_hooks_path, var, value);
+                if (value)
+                        return git_config_pathname(&git_hooks_path, var, value);
+                else
+                        return 0;
          }
  
          if (!strcmp(var, "core.bare")) {

would be a good starting point

Best Wishes

Phillip

> in the 
> past when I've wanted to temporarily disable my hooks path, I can't find 
> anything in the documentation though. If
> 
>      git config core.hookspath /dev/null
> 
> does work for you it would be really helpful to update the documentation.
> 
> Best Wishes
> 
> Phillip
> 
>> I would be happy to code a patch for this if I knew how it should be
>> configured. Obviously, no local setting for a config variable is meant
>> to inherit the global or system value. We can go all "set of the null
>> set" by making the empty string mean "revert to default". We could go
>> fancy "I know what you really mean" by interpreting ".git/hooks" (or
>> perhaps any path beginning with ".git") not literally in the case of
>> worktrees, but as relative to GIT_DIR. Or there could be another
>> setting set on a local repository to ignore global and system values of
>> core.hooksPath.
>>
>> Thank you,
>> Sam Newbold
>>
> 

Re: core.hooksPath cannot revert to the default locally

[Taylor Blau]

On Wed, Oct 16, 2024 at 02:42:57PM +0100, Phillip Wood wrote:
> On 16/10/2024 10:03, Phillip Wood wrote:
> > I'm pretty sure I've used "git -c core.hookspath=/dev/null ..."
>
> Looking at the code that will unfortunately disable all hooks including
> the ones in .git/hooks. It would make sense to allow an empty
> core.hooksPath mean "use .git/hooks". If you're interested in working
> on this then I think doing something like

Thanks for providing a helpful suggestion.

I agree that the behavior you suggested here when specifying an empty
value for 'core.hooksPath' would be an improvement here as a means to
override system/global-level configuration.

> diff --git a/config.c b/config.c
> index a11bb85da30..91f190a1ce1 100644
> --- a/config.c
> +++ b/config.c
> @@ -1437,7 +1437,10 @@ static int git_default_core_config(const char *var, const char *value,
>          if (!strcmp(var, "core.hookspath")) {
>                  FREE_AND_NULL(git_hooks_path);
> -                return git_config_pathname(&git_hooks_path, var, value);
> +                if (value)

I think this should read "if (value && *value)" instead, to ensure that
things like:

    git -c core.hooksPath= ...

work as expected.

Thanks,
Taylor

Re: core.hooksPath cannot revert to the default locally

[Sam Newbold]

Great! I'll reply to this thread when I have some code to be reviewed.

On Wed, Oct 16, 2024, at 16:33, Taylor Blau wrote:
> On Wed, Oct 16, 2024 at 02:42:57PM +0100, Phillip Wood wrote:
>> On 16/10/2024 10:03, Phillip Wood wrote:
>> > I'm pretty sure I've used "git -c core.hookspath=/dev/null ..."
>>
>> Looking at the code that will unfortunately disable all hooks including
>> the ones in .git/hooks. It would make sense to allow an empty
>> core.hooksPath mean "use .git/hooks". If you're interested in working
>> on this then I think doing something like
>
> Thanks for providing a helpful suggestion.
>
> I agree that the behavior you suggested here when specifying an empty
> value for 'core.hooksPath' would be an improvement here as a means to
> override system/global-level configuration.
>
>> diff --git a/config.c b/config.c
>> index a11bb85da30..91f190a1ce1 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -1437,7 +1437,10 @@ static int git_default_core_config(const char *var, const char *value,
>>          if (!strcmp(var, "core.hookspath")) {
>>                  FREE_AND_NULL(git_hooks_path);
>> -                return git_config_pathname(&git_hooks_path, var, value);
>> +                if (value)
>
> I think this should read "if (value && *value)" instead, to ensure that
> things like:
>
>     git -c core.hooksPath= ...
>
> work as expected.
>
> Thanks,
> Taylor