From: Ben Keene <seraphire@gmail.com>
Date: Mon, 10 Feb 2020 19:03:08 GMT
Subject: Re: [PATCH v3 2/5] git-p4: create new function run_git_hook
Message-ID: <2ab5f3cf-51f4-21d9-5741-0ec53f3035dd@gmail.com>
In-Reply-To: <xmqqo8ubec4n.fsf@gitster-ct.c.googlers.com>


On 2/6/2020 2:42 PM, Junio C Hamano wrote:
> "Ben Keene via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> +def run_git_hook(cmd, param=[]):
>> +    """Execute a hook if the hook exists."""
>> +    if verbose:
>> +        sys.stderr.write("Looking for hook: %s\n" % cmd)
>> +        sys.stderr.flush()
>> +
>> +    hooks_path = gitConfig("core.hooksPath")
>> +    if len(hooks_path) <= 0:
>> +        hooks_path = os.path.join(os.environ.get("GIT_DIR", ".git"), "hooks")
> Using the .get() with default is misleading, I think (see an earlier reply).
Looking back through the original discussion, I see that it was decided that
the application uses "rev-parse --git-dir" and "rev-parse --show-cdup" 
to find
the top directory of the git repo. At least for testing, being able to 
resolve
the .git directory directly is useful.  I will modify the code to throw an
exception if neither GIT_DIR or .git resolve correctly.
>
>> +    if not isinstance(param, list):
>> +        param=[param]
>> +
>> +    # resolve hook file name, OS depdenent
>> +    hook_file = os.path.join(hooks_path, cmd)
>> +    if platform.system() == 'Windows':
>> +        if not os.path.isfile(hook_file):
>> +            # look for the file with an extension
>> +            files = glob.glob(hook_file + ".*")
>> +            if not files:
>> +                return True
>> +            files.sort()
>> +            hook_file = files[0]
>> +
>> +    if not os.path.isfile(hook_file) or not os.access(hook_file, os.X_OK):
>> +        return True
>> +
>> +    return run_hook_command(hook_file, param) == 0
>> +
>> +def run_hook_command(cmd, param):
>> +    """Executes a git hook command
>> +       cmd = the command line file to be executed. This can be
>> +       a file that is run by OS association.
>> +
>> +       param = a list of parameters to pass to the cmd command
>> +
>> +       On windows, the extension is checked to see if it should
>> +       be run with the Git for Windows Bash shell.  If there
>> +       is no file extension, the file is deemed a bash shell
>> +       and will be handed off to sh.exe. Otherwise, Windows
>> +       will be called with the shell to handle the file assocation.
>> +
>> +       For non Windows operating systems, the file is called
>> +       as an executable.
>> +    """
>> +    cli = [cmd] + param
>> +    use_shell = False
>> +    if platform.system() == 'Windows':
>> +        (root,ext) = os.path.splitext(cmd)
>> +        if ext == "":
>> +            exe_path = os.environ.get("EXEPATH")
>> +            if exe_path is None:
>> +                exe_path = ""
>> +            else:
>> +                exe_path = os.path.join(exe_path, "bin")
>> +            cli = [os.path.join(exe_path, "SH.EXE")] + cli
>> +        else:
>> +            use_shell = True
> Please ask somebody familiar with Windows to review this "if
> Windows" and the other one in run_git_hook().
>
>> +    return subprocess.call(cli, shell=use_shell)
>> +
>> +
>>   def write_pipe(c, stdin):
>>       if verbose:
>>           sys.stderr.write('Writing pipe: %s\n' % str(c))
>> @@ -4125,7 +4187,6 @@ def printUsage(commands):
>>       "unshelve" : P4Unshelve,
>>   }
>>   
>> -
>>   def main():
>>       if len(sys.argv[1:]) == 0:
>>           printUsage(commands.keys())