Re: [PATCH 1/1] add hook pre-p4-submit

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 25, 2018, 09:14 UTC
Message-ID: <87tvonbrso.fsf@evledraar.gmail.com>
In-Reply-To: <CAE5ih79ndEbnEeV_muQyZwG+01_8Kg0J74rZtOoK1_V40E0z7g@mail.gmail.com>

On Wed, Jul 25 2018, Luke Diamand wrote:

Show 10 quoted lines

> On 23 July 2018 at 12:27, Chen Bin <chenbin.sh@gmail.com> wrote:
>> Hook pre-p4-submit is executed before git-p4 actually submits code.
>> If the hook exits with non-zero value, submit process will abort.
>
>
> Looks good to me - could you add some documentation please
> (Documentation/git-p4.txt).
>
> Thanks!
> Luke

This looks correct (and you'd know better), but I was surprised that we wouldn't just document this in githooks(5), but looking at git-p4 I see that we have its config documented there, not in git-config(1) (ditto some git-svn config stuff).

Shouldn't we at least update githooks & git-config to say that the config / hooks documentation for these utilities can be found there?

Show 60 quoted lines

>>
>> Signed-off-by: Chen Bin <chenbin.sh@gmail.com>
>> ---
>>  git-p4.py               |  6 ++++++
>>  t/t9800-git-p4-basic.sh | 22 ++++++++++++++++++++++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index b449db1cc..69ee9ce41 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -2303,6 +2303,12 @@ def run(self, args):
>>              sys.exit("number of commits (%d) must match number of shelved changelist (%d)" %
>>                       (len(commits), num_shelves))
>>
>> +        # locate hook at `.git/hooks/pre-p4-submit`
>> +        hook_file = os.path.join(read_pipe("git rev-parse --git-dir").strip(), "hooks", "pre-p4-submit")
>> +        # Execute hook. If it exit with non-zero value, do NOT continue.
>> +        if os.path.isfile(hook_file) and os.access(hook_file, os.X_OK) and subprocess.call([hook_file]) != 0:
>> +            sys.exit(1)
>> +
>>          #
>>          # Apply the commits, one at a time.  On failure, ask if should
>>          # continue to try the rest of the patches, or quit.
>> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
>> index 4849edc4e..48b768fa7 100755
>> --- a/t/t9800-git-p4-basic.sh
>> +++ b/t/t9800-git-p4-basic.sh
>> @@ -261,6 +261,28 @@ test_expect_success 'unresolvable host in P4PORT should display error' '
>>         )
>>  '
>>
>> +# Test following scenarios:
>> +#   - Without hook ".git/hooks/pre-p4-submit", submit should continue
>> +#   - With hook returning 0, submit should continue
>> +#   - With hook returning 1, submit should abort
>> +test_expect_success 'run hook pre-p4-submit before submit' '
>> +       test_when_finished cleanup_git &&
>> +       git p4 clone --dest="$git" //depot &&
>> +       (
>> +               cd "$git" &&
>> +               echo "hello world" >hello.txt &&
>> +               git add hello.txt &&
>> +               git commit -m "add hello.txt" &&
>> +               git config git-p4.skipSubmitEdit true &&
>> +               git p4 submit --dry-run | grep "Would apply" &&
>> +               mkdir -p .git/hooks &&
>> +               : >.git/hooks/pre-p4-submit && chmod +x .git/hooks/pre-p4-submit &&
>> +               git p4 submit --dry-run | grep "Would apply" &&
>> +               echo #!/bin/sh && echo exit 1 >.git/hooks/pre-p4-submit &&
>> +               git p4 submit --dry-run | grep "Would apply" || echo "Abort submit"
>> +       )
>> +'
>> +
>>  test_expect_success 'submit from detached head' '
>>         test_when_finished cleanup_git &&
>>         git p4 clone --dest="$git" //depot &&
>> --
>> 2.18.0
>>