Thanks for you review. I will update documentation asap. On Wed, Jul 25, 2018 at 7:14 PM, Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > On Wed, Jul 25 2018, Luke Diamand wrote: > >> On 23 July 2018 at 12:27, Chen Bin <chenbin.sh@xxxxxxxxx> 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? > >>> >>> Signed-off-by: Chen Bin <chenbin.sh@xxxxxxxxx> >>> --- >>> 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 >>> -- help me, help you.