git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / 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 2/2] commit: fix stdin conflict between message and hook
Date: Thu, 19 Nov 2020 20:56:30 +0000
Message-ID: <e048a9db62ccd7dd3a0e7a4475d2f8b307785de8.1605819390.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.790.v4.git.1605819390.gitgitgadget@gmail.com>

From: Orgad Shaneh <orgads@gmail.com>

If git commit is executed with -F - (meaning read the commit message
from stdin), and pre-commit hook is also reading from stdin, the
message itself was consumed by the hook before reaching the point
where it is read for the commit message.

Fix this by detecting this case, and passing this information to
run_commit_hook.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 builtin/commit.c                                 | 14 +++++++++-----
 builtin/merge.c                                  | 12 ++++++++----
 commit.c                                         |  4 ++--
 commit.h                                         |  3 ++-
 sequencer.c                                      |  6 +++---
 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh |  2 +-
 6 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 505fe60956..074a57937f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -695,11 +695,14 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);
 	int old_display_comment_prefix;
 	int merge_contains_scissors = 0;
+	int message_from_stdin = logfile && !strcmp(logfile, "-");
+	const unsigned hook_flags = message_from_stdin ? 0 : RUN_HOOK_ALLOW_STDIN;
 
 	/* This checks and barfs if author is badly specified */
 	determine_author_info(author_ident);
 
-	if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL))
+	if (!no_verify &&
+	    run_commit_hook(use_editor, index_file, hook_flags, "pre-commit", NULL))
 		return 0;
 
 	if (squash_message) {
@@ -724,7 +727,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	if (have_option_m && !fixup_message) {
 		strbuf_addbuf(&sb, &message);
 		hook_arg1 = "message";
-	} else if (logfile && !strcmp(logfile, "-")) {
+	} else if (message_from_stdin) {
 		if (isatty(0))
 			fprintf(stderr, _("(reading log message from standard input)\n"));
 		if (strbuf_read(&sb, 0, 0) < 0)
@@ -998,7 +1001,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		return 0;
 	}
 
-	if (run_commit_hook(use_editor, index_file, "prepare-commit-msg",
+	if (run_commit_hook(use_editor, index_file, RUN_HOOK_ALLOW_STDIN, "prepare-commit-msg",
 			    git_path_commit_editmsg(), hook_arg1, hook_arg2, NULL))
 		return 0;
 
@@ -1015,7 +1018,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 
 	if (!no_verify &&
-	    run_commit_hook(use_editor, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) {
+	    run_commit_hook(use_editor, index_file, RUN_HOOK_ALLOW_STDIN, "commit-msg",
+			    git_path_commit_editmsg(), NULL)) {
 		return 0;
 	}
 
@@ -1701,7 +1705,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	repo_rerere(the_repository, 0);
 	run_auto_maintenance(quiet);
-	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
+	run_commit_hook(use_editor, get_index_file(), RUN_HOOK_ALLOW_STDIN, "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
 		commit_post_rewrite(the_repository, current_head, &oid);
 	}
diff --git a/builtin/merge.c b/builtin/merge.c
index 4c133402a6..550b38cd20 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -822,8 +822,11 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	struct strbuf msg = STRBUF_INIT;
 	const char *index_file = get_index_file();
 
-	if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
+	if (!no_verify &&
+	    run_commit_hook(0 < option_edit, index_file, RUN_HOOK_ALLOW_STDIN,
+			    "pre-merge-commit", NULL)) {
 		abort_commit(remoteheads, NULL);
+	}
 	/*
 	 * Re-read the index as pre-merge-commit hook could have updated it,
 	 * and write it out as a tree.  We must do this before we invoke
@@ -850,8 +853,9 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
 	write_merge_heads(remoteheads);
 	write_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len);
-	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
-			    git_path_merge_msg(the_repository), "merge", NULL))
+	if (run_commit_hook(0 < option_edit, get_index_file(), RUN_HOOK_ALLOW_STDIN,
+			    "prepare-commit-msg", git_path_merge_msg(the_repository),
+			    "merge", NULL))
 		abort_commit(remoteheads, NULL);
 	if (0 < option_edit) {
 		if (launch_editor(git_path_merge_msg(the_repository), NULL, NULL))
@@ -859,7 +863,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	}
 
 	if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),
-					  "commit-msg",
+					  RUN_HOOK_ALLOW_STDIN, "commit-msg",
 					  git_path_merge_msg(the_repository), NULL))
 		abort_commit(remoteheads, NULL);
 
diff --git a/commit.c b/commit.c
index 775019ec9d..3f5a50164e 100644
--- a/commit.c
+++ b/commit.c
@@ -1631,7 +1631,7 @@ size_t ignore_non_trailer(const char *buf, size_t len)
 }
 
 int run_commit_hook(int editor_is_used, const char *index_file,
-		    const char *name, ...)
+		    unsigned flags, const char *name, ...)
 {
 	struct strvec hook_env = STRVEC_INIT;
 	va_list args;
@@ -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, RUN_HOOK_ALLOW_STDIN, name, args);
+	ret = run_hook_ve(hook_env.v, flags, name, args);
 	va_end(args);
 	strvec_clear(&hook_env);
 
diff --git a/commit.h b/commit.h
index 5467786c7b..72215d57fb 100644
--- a/commit.h
+++ b/commit.h
@@ -352,6 +352,7 @@ int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused)
 int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused);
 
 LAST_ARG_MUST_BE_NULL
-int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);
+int run_commit_hook(int editor_is_used, const char *index_file, unsigned flags,
+		    const char *name, ...);
 
 #endif /* COMMIT_H */
diff --git a/sequencer.c b/sequencer.c
index 684ea9d5ce..505101c29c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1171,8 +1171,8 @@ static int run_prepare_commit_msg_hook(struct repository *r,
 	} else {
 		arg1 = "message";
 	}
-	if (run_commit_hook(0, r->index_file, "prepare-commit-msg", name,
-			    arg1, arg2, NULL))
+	if (run_commit_hook(0, r->index_file, RUN_HOOK_ALLOW_STDIN,
+			    "prepare-commit-msg", name, arg1, arg2, NULL))
 		ret = error(_("'prepare-commit-msg' hook failed"));
 
 	return ret;
@@ -1496,7 +1496,7 @@ static int try_to_commit(struct repository *r,
 		goto out;
 	}
 
-	run_commit_hook(0, r->index_file, "post-commit", NULL);
+	run_commit_hook(0, r->index_file, RUN_HOOK_ALLOW_STDIN, "post-commit", NULL);
 	if (flags & AMEND_MSG)
 		commit_post_rewrite(r, current_head, oid);
 
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 7bfb7435c6..a243b7efa1 100755
--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
@@ -292,7 +292,7 @@ test_expect_success 'with user input' '
 	test_cmp user_input hook_input
 '
 
-test_expect_failure 'with user input combined with -F -' '
+test_expect_success '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 &&
-- 
gitgitgadget

  parent 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       ` [PATCH v4 1/2] " Orgad Shaneh via GitGitGadget
2020-11-19 21:23         ` 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       ` Orgad Shaneh via GitGitGadget [this message]
2020-12-09 20:06       ` [PATCH v5 0/2] " 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=e048a9db62ccd7dd3a0e7a4475d2f8b307785de8.1605819390.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=orgads@gmail.com \
    /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 the 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