"Ben Keene via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Ben Keene <seraphire@xxxxxxxxx>
>
> Add new command line option --no-verify:
>
> Add a new command line option "--no-verify" to the Submit command of
> git-p4.py. This option will function in the spirit of the existing
> --no-verify command line option found in git commit. It will cause the
> P4 Submit function to ignore the existing p4-pre-submit.
>
> Change the execution of the existing trigger p4-pre-submit to honor the
> --no-verify option. Before exiting on failure of this hook, display
> text to the user explaining which hook has failed and the impact
> of using the --no-verify option.
>
> Change the call of the p4-pre-submit hook to use the new run_git_hook
> function. This is in preparation of additional hooks to be added.
>
> Signed-off-by: Ben Keene <seraphire@xxxxxxxxx>
> ---
> Documentation/git-p4.txt | 10 ++++++++--
> Documentation/githooks.txt | 5 ++++-
> git-p4.py | 30 +++++++++++++++++++-----------
> 3 files changed, 31 insertions(+), 14 deletions(-)
Nicely done. If your strategy is to "add a feature and use it in
the same patch as the feature is added", which is what is done for
the new "no-verify" feature that is applied to existing p4-pre-submit
hook, then the code that runs p4-pre-submit in the original should
be changed to use run_git_hook() in the previous step, which added
the new run_git_hook() feature.
I see new print() that is not protected with "if verbose:"; is it
debugging cruft added during development, or is it a useful addition
for end-users to see under --verbose mode? I suspect it is the
latter.
Thanks.
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index 3494a1db3e..362b50eb21 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -374,14 +374,20 @@ 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
> -~~~~~~~~~~~~~~~
> +Hooks for submit
> +----------------
> +
> +p4-pre-submit
> +~~~~~~~~~~~~~
> +
> The `p4-pre-submit` hook is executed if it exists and is executable.
> The hook takes no parameters and nothing from standard input. Exiting with
> non-zero status from this script prevents `git-p4 submit` from launching.
> +It can be bypassed with the `--no-verify` command line option.
>
> 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 50365f2914..8cf6b08b55 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -520,7 +520,10 @@ p4-pre-submit
>
> 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.
> +`git-p4 submit` from launching. It can be bypassed with the `--no-verify`
> +command line option. Run `git-p4 submit --help` for details.
> +
> +
>
> post-index-change
> ~~~~~~~~~~~~~~~~~
> diff --git a/git-p4.py b/git-p4.py
> index d4c39f112b..b377484464 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1583,13 +1583,17 @@ def __init__(self):
> "work from a local git branch that is not master"),
> optparse.make_option("--disable-p4sync", dest="disable_p4sync", action="store_true",
> help="Skip Perforce sync of p4/master after submit or shelve"),
> + optparse.make_option("--no-verify", dest="no_verify", action="store_true",
> + help="Bypass p4-pre-submit"),
> ]
> 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 parameters and nothing from standard input. Exiting with
> - non-zero status from this script prevents `git-p4 submit` from launching.
> + The `p4-pre-submit` hook is executed if it exists and is executable. It
> + can be bypassed with the `--no-verify` command line option. The hook takes
> + no parameters and nothing from standard input. Exiting with a non-zero status
> + from this script prevents `git-p4 submit` from launching.
>
> - One usage scenario is to run unit tests in the hook."""
> + One usage scenario is to run unit tests in the hook.
> + """
>
> self.usage += " [name of git branch to submit into perforce depot]"
> self.origin = ""
> @@ -1607,6 +1611,7 @@ def __init__(self):
> self.exportLabels = False
> self.p4HasMoveCommand = p4_has_move_command()
> self.branch = None
> + self.no_verify = False
>
> if gitConfig('git-p4.largeFileSystem'):
> die("Large file system not supported for git-p4 submit command. Please remove it from config.")
> @@ -1993,6 +1998,9 @@ def applyCommit(self, id):
> applyPatchCmd = patchcmd + "--check --apply -"
> patch_succeeded = True
>
> + if verbose:
> + print("TryPatch: %s" % tryPatchCmd)
> +
> if os.system(tryPatchCmd) != 0:
> fixed_rcs_keywords = False
> patch_succeeded = False
> @@ -2032,6 +2040,7 @@ def applyCommit(self, id):
> print("Retrying the patch with RCS keywords cleaned up")
> if os.system(tryPatchCmd) == 0:
> patch_succeeded = True
> + print("Patch succeesed this time")
>
> if not patch_succeeded:
> for f in editedFiles:
> @@ -2400,13 +2409,12 @@ 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)
> + if not self.no_verify:
> + if not run_git_hook("p4-pre-submit"):
> + print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nYou can skip " \
> + "this pre-submission check by adding\nthe command line option '--no-verify', " \
> + "however,\nthis will also skip the p4-changelist hook as well.")
> + sys.exit(1)
>
> #
> # Apply the commits, one at a time. On failure, ask if should
