git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "Orgad Shaneh via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Orgad Shaneh <orgads@gmail.com>, Orgad Shaneh <orgads@gmail.com>,
	Orgad Shaneh <orgads@gmail.com>
Subject: [PATCH v4 1/2] hooks: allow input from stdin for commit-related hooks
Date: Thu, 19 Nov 2020 20:56:29 +0000
Message-ID: <3bd6024a236b061c89bb6b60daf3dc15ef1e32ca.1605819390.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.790.v4.git.1605819390.gitgitgadget@gmail.com>

From: Orgad Shaneh <orgads@gmail.com>

Let hooks receive user input if applicable.

Closing stdin originates in f5bbc3225 (Port git commit to C,
2007). Looks like the original shell implementation did have
stdin open.

This allows for example prompting the user to choose an issue
in prepare-commit-msg, and add "Fixes #123" to the commit message.

Another possible use-case is running sanity test on pre-commit,
and having a prompt like "This and that issue were found in your
changes. Are you sure you want to commit? [Y/N]".

Allow stdin only for commit-related hooks. Some of the other
hooks pass their own input to the hook, so don't change them.

Note: If pre-commit reads from stdin, and git commit is executed
with -F - (read message from stdin), the message is not read
correctly. This is fixed in the follow-up commit.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 commit.c                                      |  2 +-
 run-command.c                                 |  6 +--
 run-command.h                                 | 17 +++++--
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 46 ++++++++++++++++++-
 t/t7504-commit-msg-hook.sh                    | 15 ++++++
 t/t7505-prepare-commit-msg-hook.sh            | 14 ++++++
 6 files changed, 90 insertions(+), 10 deletions(-)

diff --git a/commit.c b/commit.c
index fe1fa3dc41..775019ec9d 100644
--- a/commit.c
+++ b/commit.c
@@ -1646,7 +1646,7 @@ int run_commit_hook(int editor_is_used, const char *index_file,
 		strvec_push(&hook_env, "GIT_EDITOR=:");
 
 	va_start(args, name);
-	ret = run_hook_ve(hook_env.v, name, args);
+	ret = run_hook_ve(hook_env.v, RUN_HOOK_ALLOW_STDIN, name, args);
 	va_end(args);
 	strvec_clear(&hook_env);
 
diff --git a/run-command.c b/run-command.c
index 2ee59acdc8..38ce53bee5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1343,7 +1343,7 @@ const char *find_hook(const char *name)
 	return path.buf;
 }
 
-int run_hook_ve(const char *const *env, const char *name, va_list args)
+int run_hook_ve(const char *const *env, unsigned flags, const char *name, va_list args)
 {
 	struct child_process hook = CHILD_PROCESS_INIT;
 	const char *p;
@@ -1356,7 +1356,7 @@ int run_hook_ve(const char *const *env, const char *name, va_list args)
 	while ((p = va_arg(args, const char *)))
 		strvec_push(&hook.args, p);
 	hook.env = env;
-	hook.no_stdin = 1;
+	hook.no_stdin = !(flags & RUN_HOOK_ALLOW_STDIN);
 	hook.stdout_to_stderr = 1;
 	hook.trace2_hook_name = name;
 
@@ -1369,7 +1369,7 @@ int run_hook_le(const char *const *env, const char *name, ...)
 	int ret;
 
 	va_start(args, name);
-	ret = run_hook_ve(env, name, args);
+	ret = run_hook_ve(env, 0, name, args);
 	va_end(args);
 
 	return ret;
diff --git a/run-command.h b/run-command.h
index 6472b38bde..e613e5e3f9 100644
--- a/run-command.h
+++ b/run-command.h
@@ -201,22 +201,29 @@ int run_command(struct child_process *);
  */
 const char *find_hook(const char *name);
 
+#define RUN_HOOK_ALLOW_STDIN 1
+
 /**
  * Run a hook.
- * The first argument is a pathname to an index file, or NULL
- * if the hook uses the default index file or no index is needed.
- * The second argument is the name of the hook.
+ * The env argument is an array of environment variables, or NULL
+ * if the hook uses the default environment and doesn't require
+ * additional variables.
+ * The flags argument is an OR'ed collection of feature bits like
+ * RUN_HOOK_ALLOW_STDIN defined above, which enables
+ * stdin for the child process (the default is no_stdin).
+ * The name argument is the name of the hook.
  * The further arguments correspond to the hook arguments.
  * The last argument has to be NULL to terminate the arguments list.
  * If the hook does not exist or is not executable, the return
  * value will be zero.
  * If it is executable, the hook will be executed and the exit
  * status of the hook is returned.
- * On execution, .stdout_to_stderr and .no_stdin will be set.
+ * On execution, .stdout_to_stderr will be set, and .no_stdin will be
+ * set unless RUN_HOOK_ALLOW_STDIN flag is requested.
  */
 LAST_ARG_MUST_BE_NULL
 int run_hook_le(const char *const *env, const char *name, ...);
-int run_hook_ve(const char *const *env, const char *name, va_list args);
+int run_hook_ve(const char *const *env, unsigned flags, const char *name, va_list args);
 
 /*
  * Trigger an auto-gc
diff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
index b3485450a2..7bfb7435c6 100755
--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
@@ -7,6 +7,7 @@ test_description='pre-commit and pre-merge-commit hooks'
 HOOKDIR="$(git rev-parse --git-dir)/hooks"
 PRECOMMIT="$HOOKDIR/pre-commit"
 PREMERGE="$HOOKDIR/pre-merge-commit"
+POSTCOMMIT="$HOOKDIR/post-commit"
 
 # Prepare sample scripts that write their $0 to actual_hooks
 test_expect_success 'sample script setup' '
@@ -28,11 +29,14 @@ test_expect_success 'sample script setup' '
 	echo $0 >>actual_hooks
 	test $GIT_PREFIX = "success/"
 	EOF
-	write_script "$HOOKDIR/check-author.sample" <<-\EOF
+	write_script "$HOOKDIR/check-author.sample" <<-\EOF &&
 	echo $0 >>actual_hooks
 	test "$GIT_AUTHOR_NAME" = "New Author" &&
 	test "$GIT_AUTHOR_EMAIL" = "newauthor@example.com"
 	EOF
+	write_script "$HOOKDIR/user-input.sample" <<-\EOF
+	! read -r line || echo "$line" >hook_input
+	EOF
 '
 
 test_expect_success 'root commit' '
@@ -278,4 +282,44 @@ test_expect_success 'check the author in hook' '
 	test_cmp expected_hooks actual_hooks
 '
 
+test_expect_success 'with user input' '
+	test_when_finished "rm -f \"$PRECOMMIT\" user_input hook_input" &&
+	cp "$HOOKDIR/user-input.sample" "$PRECOMMIT" &&
+	echo "user input" >user_input &&
+	echo "more" >>file &&
+	git add file &&
+	git commit -m "more" <user_input &&
+	test_cmp user_input hook_input
+'
+
+test_expect_failure 'with user input combined with -F -' '
+	test_when_finished "rm -f \"$PRECOMMIT\" user_input hook_input" &&
+	cp "$HOOKDIR/user-input.sample" "$PRECOMMIT" &&
+	echo "user input" >user_input &&
+	echo "more" >>file &&
+	git add file &&
+	git commit -F - <user_input &&
+	! test_path_is_file hook_input
+'
+
+test_expect_success 'post-commit with user input' '
+	test_when_finished "rm -f \"$POSTCOMMIT\" user_input hook_input" &&
+	cp "$HOOKDIR/user-input.sample" "$POSTCOMMIT" &&
+	echo "user input" >user_input &&
+	echo "more" >>file &&
+	git add file &&
+	git commit -m "more" <user_input &&
+	test_cmp user_input hook_input
+'
+
+test_expect_success 'with user input (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" user_input hook_input" &&
+	cp "$HOOKDIR/user-input.sample" "$PREMERGE" &&
+	echo "user input" >user_input &&
+	git checkout side &&
+	git merge -m "merge master" master <user_input &&
+	git checkout master &&
+	test_cmp user_input hook_input
+'
+
 test_done
diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh
index 31b9c6a2c1..aa76eb7e1f 100755
--- a/t/t7504-commit-msg-hook.sh
+++ b/t/t7504-commit-msg-hook.sh
@@ -294,5 +294,20 @@ test_expect_success 'hook is called for reword during `rebase -i`' '
 
 '
 
+# now a hook that accepts input and writes it as the commit message
+cat >"$HOOK" <<'EOF'
+#!/bin/sh
+! read -r line || echo "$line" >"$1"
+EOF
+chmod +x "$HOOK"
+
+test_expect_success 'hook with user input' '
+
+	echo "additional" >>file &&
+	git add file &&
+	echo "user input" | git commit -m "additional" &&
+	commit_msg_is "user input"
+
+'
 
 test_done
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
index 94f85cdf83..aa9c9375e6 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -91,6 +91,11 @@ else
 fi
 test "$GIT_EDITOR" = : && source="$source (no editor)"
 
+if read -r line
+then
+	source="$source $line"
+fi
+
 if test $rebasing = 1
 then
 	echo "$source $(get_last_cmd)" >"$1"
@@ -113,6 +118,15 @@ test_expect_success 'with hook (-m)' '
 
 '
 
+test_expect_success 'with hook (-m and input)' '
+
+	echo "more" >>file &&
+	git add file &&
+	echo "user input" | git commit -m "more" &&
+	test "$(git log -1 --pretty=format:%s)" = "message (no editor) user input"
+
+'
+
 test_expect_success 'with hook (-m editor)' '
 
 	echo "more" >> file &&
-- 
gitgitgadget


  reply	other threads:[~2020-11-19 21:00 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-17 15:02 [PATCH] hooks: allow input from stdin Orgad Shaneh via GitGitGadget
2020-11-17 19:59 ` Junio C Hamano
2020-11-19 15:50 ` [PATCH v2] " Orgad Shaneh via GitGitGadget
2020-11-19 15:56   ` [PATCH v3] hooks: allow input from stdin for commit-related hooks Orgad Shaneh via GitGitGadget
2020-11-19 19:16     ` Junio C Hamano
2020-11-19 20:41       ` Orgad Shaneh
2020-11-19 20:56     ` [PATCH v4 0/2] " Orgad Shaneh via GitGitGadget
2020-11-19 20:56       ` Orgad Shaneh via GitGitGadget [this message]
2020-11-19 21:23         ` [PATCH v4 1/2] " Eric Sunshine
2020-11-19 21:32           ` Junio C Hamano
2020-11-20  5:23           ` Orgad Shaneh
2020-11-20  6:38             ` Eric Sunshine
2020-11-20  6:48               ` Eric Sunshine
2020-11-20  7:16                 ` Orgad Shaneh
2020-11-20 18:13               ` Junio C Hamano
2020-11-20 10:59             ` Ævar Arnfjörð Bjarmason
2020-11-20 12:34               ` Orgad Shaneh
2020-11-19 20:56       ` [PATCH v4 2/2] commit: fix stdin conflict between message and hook Orgad Shaneh via GitGitGadget
2020-12-09 20:06       ` [PATCH v5 0/2] hooks: allow input from stdin for commit-related hooks Orgad Shaneh via GitGitGadget
2020-12-09 20:06         ` [PATCH v5 1/2] hooks: lay foundations for passing stdin to hooks Orgad Shaneh via GitGitGadget
2020-12-09 20:06         ` [PATCH v5 2/2] hooks: allow input from stdin for commit-related hooks Orgad Shaneh via GitGitGadget
2020-12-09 22:37           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3bd6024a236b061c89bb6b60daf3dc15ef1e32ca.1605819390.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=orgads@gmail.com \
    --subject='Re: [PATCH v4 1/2] hooks: allow input from stdin for commit-related hooks' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git