Re: [PATCH 1/1] Add the `p4-pre-submit` hook
Chen Bin writes: > The `p4-pre-submit` hook is executed before git-p4 submits code. > If the hook exits with non-zero value, submit process won't start. > > Signed-off-by: Chen Bin > --- I see that the only difference between this and what has been queued on 'pu', i.e. https://github.com/gitster/git/commit/fb78b040c5dc5b80a093d028d13f8cd32ade17cd is that this is missing the update in https://public-inbox.org/git/xmqq1sbkfneo@gitster-ct.c.googlers.com/ and also Luke's "Reviewed-by:". I recall that you had trouble getting "git p4" in the test (not "git-p4") working for some reason. Has that been resolved (iow have you figured out why it was failing?) Thanks.
[PATCH 1/1] Add the `p4-pre-submit` hook
The `p4-pre-submit` hook is executed before git-p4 submits code.
If the hook exits with non-zero value, submit process won't start.
Signed-off-by: Chen Bin
---
Documentation/git-p4.txt | 8
Documentation/githooks.txt | 7 +++
git-p4.py | 16 +++-
t/t9800-git-p4-basic.sh| 29 +
4 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index f0de3b891..a7aac1b92 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -374,6 +374,14 @@ These options can be used to modify 'git p4 submit'
behavior.
been submitted. Implies --disable-rebase. Can also be set with
git-p4.disableP4Sync. Sync with origin/master still goes ahead if possible.
+Hook for submit
+~~~
+The `p4-pre-submit` hook is executed if it exists and is executable.
+The hook takes no parameter and nothing from standard input. Exiting with
+non-zero status from this script prevents `git-p4 submit` from launching.
+
+One usage scenario is to run unit tests in the hook.
+
Rebase options
~~
These options can be used to modify 'git p4 rebase' behavior.
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index e3c283a17..22fcabbe2 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -485,6 +485,13 @@ The exit status determines whether git will use the data
from the
hook to limit its search. On error, it will fall back to verifying
all files and folders.
+p4-pre-submit
+~
+
+This hook is invoked by `git-p4 submit`. It takes no parameter and nothing
+from standard input. Exiting with non-zero status from this script prevent
+`git-p4 submit` from launching. Run `git-p4 submit --help` for details.
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/git-p4.py b/git-p4.py
index b449db1cc..879abfd2b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1494,7 +1494,13 @@ def __init__(self):
optparse.make_option("--disable-p4sync",
dest="disable_p4sync", action="store_true",
help="Skip Perforce sync of p4/master
after submit or shelve"),
]
-self.description = "Submit changes from git to the perforce depot."
+self.description = """Submit changes from git to the perforce depot.\n
+The `p4-pre-submit` hook is executed if it exists and is executable.
+The hook takes no parameter and nothing from standard input. Exiting with
+non-zero status from this script prevents `git-p4 submit` from launching.
+
+One usage scenario is to run unit tests in the hook."""
+
self.usage += " [name of git branch to submit into perforce depot]"
self.origin = ""
self.detectRenames = False
@@ -2303,6 +2309,14 @@ def run(self, args):
sys.exit("number of commits (%d) must match number of shelved
changelist (%d)" %
(len(commits), num_shelves))
+hooks_path = gitConfig("core.hooksPath")
+if len(hooks_path) <= 0:
+hooks_path = os.path.join(os.environ.get("GIT_DIR", ".git"),
"hooks")
+
+hook_file = os.path.join(hooks_path, "p4-pre-submit")
+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..729cd2577 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -261,6 +261,35 @@ test_expect_success 'unresolvable host in P4PORT should
display error' '
)
'
+# Test following scenarios:
+# - Without ".git/hooks/p4-pre-submit" , submit should continue
+# - With the hook returning 0, submit should continue
+# - With the hook returning 1, submit should abort
+test_expect_success 'run hook p4-pre-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 >out &&
+ grep "Would apply" out &&
+ mkdir -p .git/hooks &&
+ write_script .git/hooks/p4-pre-submit <<-\EOF &&
+ exit 0
+ EOF
+ git p4 submit --dry-run >out &&
+ grep "Would apply" out &&
+ write_script .git/hooks/p4-pre-submit <<-\EOF &&
+ exit 1
+ EOF
+ test_must_fail git p4 submit --dry-run >errs 2>&1 &&
+ ! grep "Would apply" errs
+ )
+'
+
test_expect_success 'submit from detached head' '
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
chen bin writes:
> I updated the patch. But for some reason the test keep failing at this line,
> `test_must_fail git p4 submit --dry-run >errs 2>&1 &&`.
>
> If I change this line to `test_must_fail git-p4 submit --dry-run >errs
> 2>&1 &&` the test will pass.
Hmph. I somehow suspect that the test also will pass if you changed
it like this:
test_must_fail false >errs 2>&1 &&
IOW, my suspicion is that the shell fails to find "git-p4" [*1*] and
that is why your `test_must_fail git-p4 whatever` lets your test
pass, which is different from the way you _want_ your test_must_fail
succeed, i.e. "git p4 submit" is run with the "--dry-run" option and
exit with non-zero status.
Of course, if the shell cannot find "git-p4", `errs` would not have
the string "Would apply" in it, so the next test also would pass.
On 31 July 2018 at 10:46, SZEDER Gábor wrote:
>> + test_must_fail git-p4 submit --dry-run >errs 2>&1 &&>
>> + ! grep "Would apply" err
[Footnote]
*1* As I do not use (nor install) p4, I cannot test "'git p4' works
but 'git-p4' should not work" myself, but by running t0001 with
a trial patch like the following, you can see that we do not
find the dashed form `git-init` on $PATH and the test indeed
fails.
t/t0001-init.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 0681300707..8c598a0d84 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -31,7 +31,7 @@ check_config () {
}
test_expect_success 'plain' '
- git init plain &&
+ git-init plain &&
check_config plain/.git false unset
'
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
I updated the patch. But for some reason the test keep failing at this line, `test_must_fail git p4 submit --dry-run >errs 2>&1 &&`. If I change this line to `test_must_fail git-p4 submit --dry-run >errs 2>&1 &&` the test will pass. Could you help me to resolve this issue? I'm really confused. I've already added latest `p4d` and `p4` to $PATH. The commit is at https://github.com/redguardtoo/git/commit/b88c38b9ce6cfb1c1fefef15527adfa92d78daf2 On Wed, Aug 1, 2018 at 5:54 AM, Luke Diamand wrote: > On 31 July 2018 at 16:40, Junio C Hamano wrote: >> Luke Diamand writes: >> >>> I think there is an error in the test harness. >>> >>> On 31 July 2018 at 10:46, SZEDER Gábor wrote: > + test_must_fail git-p4 submit --dry-run >errs 2>&1 &&> > + ! grep "Would apply" err >>> >>> It writes to the file "errs" but then looks for the message in "err". >>> >>> Luke >> >> Sigh. Thanks for spotting, both of you. >> >> Here is what I'd locally squash in. If there is anything else, I'd >> rather see a refreshed final one sent by the author. >> >> Thanks. >> >> t/t9800-git-p4-basic.sh | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh >> index 2b7baa95d2..729cd25770 100755 >> --- a/t/t9800-git-p4-basic.sh >> +++ b/t/t9800-git-p4-basic.sh >> @@ -274,19 +274,19 @@ test_expect_success 'run hook p4-pre-submit before >> submit' ' >> git add hello.txt && >> git commit -m "add hello.txt" && >> git config git-p4.skipSubmitEdit true && >> - git-p4 submit --dry-run >out && >> + git p4 submit --dry-run >out && >> grep "Would apply" out && >> mkdir -p .git/hooks && >> write_script .git/hooks/p4-pre-submit <<-\EOF && >> exit 0 >> EOF >> - git-p4 submit --dry-run >out && >> + git p4 submit --dry-run >out && >> grep "Would apply" out && >> write_script .git/hooks/p4-pre-submit <<-\EOF && >> exit 1 >> EOF >> - test_must_fail git-p4 submit --dry-run >errs 2>&1 && >> - ! grep "Would apply" err >> + test_must_fail git p4 submit --dry-run >errs 2>&1 && >> + ! grep "Would apply" errs >> ) >> ' > > That set of deltas works for me. > > Sorry, I should have run the tests myself earlier and I would have > picked up on this. -- help me, help you.
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
On 31 July 2018 at 16:40, Junio C Hamano wrote: > Luke Diamand writes: > >> I think there is an error in the test harness. >> >> On 31 July 2018 at 10:46, SZEDER Gábor wrote: + test_must_fail git-p4 submit --dry-run >errs 2>&1 &&> + ! grep "Would apply" err >> >> It writes to the file "errs" but then looks for the message in "err". >> >> Luke > > Sigh. Thanks for spotting, both of you. > > Here is what I'd locally squash in. If there is anything else, I'd > rather see a refreshed final one sent by the author. > > Thanks. > > t/t9800-git-p4-basic.sh | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh > index 2b7baa95d2..729cd25770 100755 > --- a/t/t9800-git-p4-basic.sh > +++ b/t/t9800-git-p4-basic.sh > @@ -274,19 +274,19 @@ test_expect_success 'run hook p4-pre-submit before > submit' ' > git add hello.txt && > git commit -m "add hello.txt" && > git config git-p4.skipSubmitEdit true && > - git-p4 submit --dry-run >out && > + git p4 submit --dry-run >out && > grep "Would apply" out && > mkdir -p .git/hooks && > write_script .git/hooks/p4-pre-submit <<-\EOF && > exit 0 > EOF > - git-p4 submit --dry-run >out && > + git p4 submit --dry-run >out && > grep "Would apply" out && > write_script .git/hooks/p4-pre-submit <<-\EOF && > exit 1 > EOF > - test_must_fail git-p4 submit --dry-run >errs 2>&1 && > - ! grep "Would apply" err > + test_must_fail git p4 submit --dry-run >errs 2>&1 && > + ! grep "Would apply" errs > ) > ' That set of deltas works for me. Sorry, I should have run the tests myself earlier and I would have picked up on this.
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
Luke Diamand writes: > I think there is an error in the test harness. > > On 31 July 2018 at 10:46, SZEDER Gábor wrote: >>> + test_must_fail git-p4 submit --dry-run >errs 2>&1 &&> >>> + ! grep "Would apply" err > > It writes to the file "errs" but then looks for the message in "err". > > Luke Sigh. Thanks for spotting, both of you. Here is what I'd locally squash in. If there is anything else, I'd rather see a refreshed final one sent by the author. Thanks. t/t9800-git-p4-basic.sh | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh index 2b7baa95d2..729cd25770 100755 --- a/t/t9800-git-p4-basic.sh +++ b/t/t9800-git-p4-basic.sh @@ -274,19 +274,19 @@ test_expect_success 'run hook p4-pre-submit before submit' ' git add hello.txt && git commit -m "add hello.txt" && git config git-p4.skipSubmitEdit true && - git-p4 submit --dry-run >out && + git p4 submit --dry-run >out && grep "Would apply" out && mkdir -p .git/hooks && write_script .git/hooks/p4-pre-submit <<-\EOF && exit 0 EOF - git-p4 submit --dry-run >out && + git p4 submit --dry-run >out && grep "Would apply" out && write_script .git/hooks/p4-pre-submit <<-\EOF && exit 1 EOF - test_must_fail git-p4 submit --dry-run >errs 2>&1 && - ! grep "Would apply" err + test_must_fail git p4 submit --dry-run >errs 2>&1 && + ! grep "Would apply" errs ) '
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
I think there is an error in the test harness. On 31 July 2018 at 10:46, SZEDER Gábor wrote: >> + test_must_fail git-p4 submit --dry-run >errs 2>&1 &&> >> + ! grep "Would apply" err It writes to the file "errs" but then looks for the message in "err". Luke
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh > index 4849edc4e..2b7baa95d 100755 > --- a/t/t9800-git-p4-basic.sh > +++ b/t/t9800-git-p4-basic.sh > @@ -261,6 +261,35 @@ test_expect_success 'unresolvable host in P4PORT should > display error' ' > ) > ' > > +# Test following scenarios: > +# - Without ".git/hooks/p4-pre-submit" , submit should continue > +# - With the hook returning 0, submit should continue > +# - With the hook returning 1, submit should abort > +test_expect_success 'run hook p4-pre-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 >out && This must be 'git p4 ...', i.e. without dash. The dashed form causes the test to fail: <...> +git config git-p4.skipSubmitEdit true +git-p4 submit --dry-run t9800-git-p4-basic.sh: 12: eval: git-p4: not found error: last command exited with $?=127 not ok 19 - run hook p4-pre-submit before submit > + grep "Would apply" out && > + mkdir -p .git/hooks && > + write_script .git/hooks/p4-pre-submit <<-\EOF && > + exit 0 > + EOF > + git-p4 submit --dry-run >out && Likewise. > + grep "Would apply" out && > + write_script .git/hooks/p4-pre-submit <<-\EOF && > + exit 1 > + EOF > + test_must_fail git-p4 submit --dry-run >errs 2>&1 && Likewise. > + ! grep "Would apply" err > + ) > +' > + > test_expect_success 'submit from detached head' ' > test_when_finished cleanup_git && > git p4 clone --dest="$git" //depot && > -- > 2.18.0 > >
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
Luke Diamand writes: > Possibly it should say "it takes no parameters" rather than "it takes > no parameter"; it is confusing that in English, zero takes the plural > rather than the singular. There's a PhD in linguistics there for > someone! I count three instances. Will squash in the following while queuing with your Reviewed-by. Thanks. Documentation/git-p4.txt | 2 +- Documentation/githooks.txt | 2 +- git-p4.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt index a7aac1b920..41780a5aa9 100644 --- a/Documentation/git-p4.txt +++ b/Documentation/git-p4.txt @@ -377,7 +377,7 @@ These options can be used to modify 'git p4 submit' behavior. Hook for submit ~~~ The `p4-pre-submit` hook is executed if it exists and is executable. -The hook takes no parameter and nothing from standard input. Exiting with +The hook takes no parameters and nothing from standard input. Exiting with non-zero status from this script prevents `git-p4 submit` from launching. One usage scenario is to run unit tests in the hook. diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 22fcabbe21..959044347e 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -488,7 +488,7 @@ all files and folders. p4-pre-submit ~ -This hook is invoked by `git-p4 submit`. It takes no parameter and nothing +This hook is invoked by `git-p4 submit`. It takes no parameters and nothing from standard input. Exiting with non-zero status from this script prevent `git-p4 submit` from launching. Run `git-p4 submit --help` for details. diff --git a/git-p4.py b/git-p4.py index 879abfd2b1..7fab255584 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1496,7 +1496,7 @@ def __init__(self): ] self.description = """Submit changes from git to the perforce depot.\n The `p4-pre-submit` hook is executed if it exists and is executable. -The hook takes no parameter and nothing from standard input. Exiting with +The hook takes no parameters and nothing from standard input. Exiting with non-zero status from this script prevents `git-p4 submit` from launching. One usage scenario is to run unit tests in the hook."""
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
On 30 July 2018 at 20:07, Junio C Hamano wrote:
> Chen Bin writes:
>
>> The `p4-pre-submit` hook is executed before git-p4 submits code.
>> If the hook exits with non-zero value, submit process not start.
>>
>> Signed-off-by: Chen Bin
>> ---
>
> Luke, does this version look good to you?
>
Yes, I think so, We might find in the future that we do need an
additional hook *after* the change has been applied, but as per Chen's
comment, it sounds like that's not what is needed here; it can easily
be added in the future if necessary.
And there's no directly equivalent existing git-hook which could be
used instead, so this seems like a useful addition.
Possibly it should say "it takes no parameters" rather than "it takes
no parameter"; it is confusing that in English, zero takes the plural
rather than the singular. There's a PhD in linguistics there for
someone!
Luke
> I still think the addition to self.description is a bit too much but
> other than that the incremental since the last one looked sensible
> to my untrained eyes ;-)
>
> Thanks, both.
>
>> Documentation/git-p4.txt | 8
>> Documentation/githooks.txt | 7 +++
>> git-p4.py | 16 +++-
>> t/t9800-git-p4-basic.sh| 29 +
>> 4 files changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
>> index f0de3b891..a7aac1b92 100644
>> --- a/Documentation/git-p4.txt
>> +++ b/Documentation/git-p4.txt
>> @@ -374,6 +374,14 @@ These options can be used to modify 'git p4 submit'
>> behavior.
>> been submitted. Implies --disable-rebase. Can also be set with
>> git-p4.disableP4Sync. Sync with origin/master still goes ahead if
>> possible.
>>
>> +Hook for submit
>> +~~~
>> +The `p4-pre-submit` hook is executed if it exists and is executable.
>> +The hook takes no parameter and nothing from standard input. Exiting with
>> +non-zero status from this script prevents `git-p4 submit` from launching.
>> +
>> +One usage scenario is to run unit tests in the hook.
>> +
>> Rebase options
>> ~~
>> These options can be used to modify 'git p4 rebase' behavior.
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> index e3c283a17..22fcabbe2 100644
>> --- a/Documentation/githooks.txt
>> +++ b/Documentation/githooks.txt
>> @@ -485,6 +485,13 @@ The exit status determines whether git will use the
>> data from the
>> hook to limit its search. On error, it will fall back to verifying
>> all files and folders.
>>
>> +p4-pre-submit
>> +~
>> +
>> +This hook is invoked by `git-p4 submit`. It takes no parameter and nothing
>> +from standard input. Exiting with non-zero status from this script prevent
>> +`git-p4 submit` from launching. Run `git-p4 submit --help` for details.
>> +
>> GIT
>> ---
>> Part of the linkgit:git[1] suite
>> diff --git a/git-p4.py b/git-p4.py
>> index b449db1cc..879abfd2b 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -1494,7 +1494,13 @@ def __init__(self):
>> optparse.make_option("--disable-p4sync",
>> dest="disable_p4sync", action="store_true",
>> help="Skip Perforce sync of p4/master
>> after submit or shelve"),
>> ]
>> -self.description = "Submit changes from git to the perforce depot."
>> +self.description = """Submit changes from git to the perforce
>> depot.\n
>> +The `p4-pre-submit` hook is executed if it exists and is executable.
>> +The hook takes no parameter and nothing from standard input. Exiting
>> with
>> +non-zero status from this script prevents `git-p4 submit` from
>> launching.
>> +
>> +One usage scenario is to run unit tests in the hook."""
>> +
>> self.usage += " [name of git branch to submit into perforce depot]"
>> self.origin = ""
>> self.detectRenames = False
>> @@ -2303,6 +2309,14 @@ def run(self, args):
>> sys.exit("number of commits (%d) must match number of shelved
>> changelist (%d)" %
>> (len(commits), num_shelves))
>>
>> +hooks_path = gitConfig("core.hooksPath")
>> +if len(hooks_path) <= 0:
>> +hooks_path = os.path.join(os.environ.get("GIT_DIR", ".git"),
>> "hooks")
>> +
>> +hook_file = os.path.join(hooks_path, "p4-pre-submit")
>> +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..2b7baa95d 100755
>> --- a/t/t9800-git-p4-basic.sh
>> +++ b/t/t9800-git-p4-basic.sh
>> @@ -261,6 +261,35 @@ test_expect_success 'unresolvable host in P4PORT should
>> display error' '
>> )
>> '
>>
>> +# Test
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
Chen Bin writes:
> The `p4-pre-submit` hook is executed before git-p4 submits code.
> If the hook exits with non-zero value, submit process not start.
>
> Signed-off-by: Chen Bin
> ---
Luke, does this version look good to you?
I still think the addition to self.description is a bit too much but
other than that the incremental since the last one looked sensible
to my untrained eyes ;-)
Thanks, both.
> Documentation/git-p4.txt | 8
> Documentation/githooks.txt | 7 +++
> git-p4.py | 16 +++-
> t/t9800-git-p4-basic.sh| 29 +
> 4 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index f0de3b891..a7aac1b92 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -374,6 +374,14 @@ These options can be used to modify 'git p4 submit'
> behavior.
> been submitted. Implies --disable-rebase. Can also be set with
> git-p4.disableP4Sync. Sync with origin/master still goes ahead if
> possible.
>
> +Hook for submit
> +~~~
> +The `p4-pre-submit` hook is executed if it exists and is executable.
> +The hook takes no parameter and nothing from standard input. Exiting with
> +non-zero status from this script prevents `git-p4 submit` from launching.
> +
> +One usage scenario is to run unit tests in the hook.
> +
> Rebase options
> ~~
> These options can be used to modify 'git p4 rebase' behavior.
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index e3c283a17..22fcabbe2 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -485,6 +485,13 @@ The exit status determines whether git will use the data
> from the
> hook to limit its search. On error, it will fall back to verifying
> all files and folders.
>
> +p4-pre-submit
> +~
> +
> +This hook is invoked by `git-p4 submit`. It takes no parameter and nothing
> +from standard input. Exiting with non-zero status from this script prevent
> +`git-p4 submit` from launching. Run `git-p4 submit --help` for details.
> +
> GIT
> ---
> Part of the linkgit:git[1] suite
> diff --git a/git-p4.py b/git-p4.py
> index b449db1cc..879abfd2b 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1494,7 +1494,13 @@ def __init__(self):
> optparse.make_option("--disable-p4sync",
> dest="disable_p4sync", action="store_true",
> help="Skip Perforce sync of p4/master
> after submit or shelve"),
> ]
> -self.description = "Submit changes from git to the perforce depot."
> +self.description = """Submit changes from git to the perforce
> depot.\n
> +The `p4-pre-submit` hook is executed if it exists and is executable.
> +The hook takes no parameter and nothing from standard input. Exiting with
> +non-zero status from this script prevents `git-p4 submit` from launching.
> +
> +One usage scenario is to run unit tests in the hook."""
> +
> self.usage += " [name of git branch to submit into perforce depot]"
> self.origin = ""
> self.detectRenames = False
> @@ -2303,6 +2309,14 @@ def run(self, args):
> sys.exit("number of commits (%d) must match number of shelved
> changelist (%d)" %
> (len(commits), num_shelves))
>
> +hooks_path = gitConfig("core.hooksPath")
> +if len(hooks_path) <= 0:
> +hooks_path = os.path.join(os.environ.get("GIT_DIR", ".git"),
> "hooks")
> +
> +hook_file = os.path.join(hooks_path, "p4-pre-submit")
> +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..2b7baa95d 100755
> --- a/t/t9800-git-p4-basic.sh
> +++ b/t/t9800-git-p4-basic.sh
> @@ -261,6 +261,35 @@ test_expect_success 'unresolvable host in P4PORT should
> display error' '
> )
> '
>
> +# Test following scenarios:
> +# - Without ".git/hooks/p4-pre-submit" , submit should continue
> +# - With the hook returning 0, submit should continue
> +# - With the hook returning 1, submit should abort
> +test_expect_success 'run hook p4-pre-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 >out &&
> + grep "Would apply" out &&
> + mkdir -p .git/hooks &&
> + write_script
Re: [PATCH 1/1] Add the `p4-pre-submit` hook
On Fri, Jul 27, 2018 at 7:22 AM Chen Bin wrote: > The `p4-pre-submit` hook is executed before git-p4 submits code. > If the hook exits with non-zero value, submit process not start. > > Signed-off-by: Chen Bin > --- > diff --git a/git-p4.py b/git-p4.py > @@ -1494,7 +1494,13 @@ def __init__(self): > -self.description = "Submit changes from git to the perforce depot." > +self.description = """Submit changes from git to the perforce > depot.\n It seemed odd to have an explicit \n within a """...""" string as opposed to simply using a blank line, but I see there is existing precedence in git-p4.py when setting (at least one) self.description. Okay. > +The `p4-pre-submit` hook is executed if it exists and is executable. > +The hook takes no parameter and nothing from standard input. Exiting with > +non-zero status from this script prevents `git-p4 submit` from launching. > + > +One usage scenario is to run unit tests in the hook."""
[PATCH 1/1] Add the `p4-pre-submit` hook
The `p4-pre-submit` hook is executed before git-p4 submits code.
If the hook exits with non-zero value, submit process not start.
Signed-off-by: Chen Bin
---
Documentation/git-p4.txt | 8
Documentation/githooks.txt | 7 +++
git-p4.py | 16 +++-
t/t9800-git-p4-basic.sh| 29 +
4 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index f0de3b891..a7aac1b92 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -374,6 +374,14 @@ These options can be used to modify 'git p4 submit'
behavior.
been submitted. Implies --disable-rebase. Can also be set with
git-p4.disableP4Sync. Sync with origin/master still goes ahead if possible.
+Hook for submit
+~~~
+The `p4-pre-submit` hook is executed if it exists and is executable.
+The hook takes no parameter and nothing from standard input. Exiting with
+non-zero status from this script prevents `git-p4 submit` from launching.
+
+One usage scenario is to run unit tests in the hook.
+
Rebase options
~~
These options can be used to modify 'git p4 rebase' behavior.
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index e3c283a17..22fcabbe2 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -485,6 +485,13 @@ The exit status determines whether git will use the data
from the
hook to limit its search. On error, it will fall back to verifying
all files and folders.
+p4-pre-submit
+~
+
+This hook is invoked by `git-p4 submit`. It takes no parameter and nothing
+from standard input. Exiting with non-zero status from this script prevent
+`git-p4 submit` from launching. Run `git-p4 submit --help` for details.
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/git-p4.py b/git-p4.py
index b449db1cc..879abfd2b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1494,7 +1494,13 @@ def __init__(self):
optparse.make_option("--disable-p4sync",
dest="disable_p4sync", action="store_true",
help="Skip Perforce sync of p4/master
after submit or shelve"),
]
-self.description = "Submit changes from git to the perforce depot."
+self.description = """Submit changes from git to the perforce depot.\n
+The `p4-pre-submit` hook is executed if it exists and is executable.
+The hook takes no parameter and nothing from standard input. Exiting with
+non-zero status from this script prevents `git-p4 submit` from launching.
+
+One usage scenario is to run unit tests in the hook."""
+
self.usage += " [name of git branch to submit into perforce depot]"
self.origin = ""
self.detectRenames = False
@@ -2303,6 +2309,14 @@ def run(self, args):
sys.exit("number of commits (%d) must match number of shelved
changelist (%d)" %
(len(commits), num_shelves))
+hooks_path = gitConfig("core.hooksPath")
+if len(hooks_path) <= 0:
+hooks_path = os.path.join(os.environ.get("GIT_DIR", ".git"),
"hooks")
+
+hook_file = os.path.join(hooks_path, "p4-pre-submit")
+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..2b7baa95d 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -261,6 +261,35 @@ test_expect_success 'unresolvable host in P4PORT should
display error' '
)
'
+# Test following scenarios:
+# - Without ".git/hooks/p4-pre-submit" , submit should continue
+# - With the hook returning 0, submit should continue
+# - With the hook returning 1, submit should abort
+test_expect_success 'run hook p4-pre-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 >out &&
+ grep "Would apply" out &&
+ mkdir -p .git/hooks &&
+ write_script .git/hooks/p4-pre-submit <<-\EOF &&
+ exit 0
+ EOF
+ git-p4 submit --dry-run >out &&
+ grep "Would apply" out &&
+ write_script .git/hooks/p4-pre-submit <<-\EOF &&
+ exit 1
+ EOF
+ test_must_fail git-p4 submit --dry-run >errs 2>&1 &&
+ ! grep "Would apply" err
+ )
+'
+
test_expect_success 'submit from detached head' '
