git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	Magne Land <magne.land@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 1/1] switch/restore: consistently pass through exit code of `post-checkout`
Date: Thu, 10 Oct 2019 05:01:39 -0700 (PDT)
Message-ID: <83305732371f3f511b3f4acf2060610c80ad12fa.1570708898.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.385.git.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Currently, the exit code of the `post-checkout` hook becomes the exit
code of the `git restore` (or the equivalent `git checkout`) command,
_iff_ no error occurred during the `restore` operation.

That allows versatile scripting where the `post-checkout` failure modes
can be discerned from the `git restore` failure modes.

There is a problem, though: if that `git restore` operation fails
_before_ the `post-checkout` hook is run, -1 is returned from the
`checkout_paths()` function, which yields the misleading exit code 127
(which is reserved to indicate "command not found").

Another problem: a non-zero exit code of the `post-checkout` hook as run
at the end of a `git switch` run will always causes that command to exit
with exit code 1.

This is inconsistent. Let's pass through the exit code of the
`post-checkout` hook verbatim (unless the Git operation fails, in which
case we want to exit with code 1, even if the hook returns another exit
code).

Document this, and add a regression test for this behavior, too.

Inspired-by-a-patch-by: Magne Land <magne.land@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/githooks.txt    | 3 ++-
 builtin/checkout.c            | 8 +++++---
 t/t5403-post-checkout-hook.sh | 9 +++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 50365f2914..12c756a37b 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -193,7 +193,8 @@ worktree.  The hook is given three parameters: the ref of the previous HEAD,
 the ref of the new HEAD (which may or may not have changed), and a flag
 indicating whether the checkout was a branch checkout (changing branches,
 flag=1) or a file checkout (retrieving a file from the index, flag=0).
-This hook cannot affect the outcome of `git switch` or `git checkout`.
+If this hook exits with an exit code other than 0, it causes the calling
+`git switch` or `git checkout` command to fail with the same exit code.
 
 It is also run after linkgit:git-clone[1], unless the `--no-checkout` (`-n`) option is
 used. The first parameter given to the hook is the null-ref, the second the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 1283727761..56afc3c1a8 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -400,7 +400,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 	static char *ps_matched;
 	struct object_id rev;
 	struct commit *head;
-	int errs = 0;
+	int errs = 0, ret;
 	struct lock_file lock_file = LOCK_INIT;
 	int checkout_index;
 
@@ -548,8 +548,8 @@ static int checkout_paths(const struct checkout_opts *opts,
 	read_ref_full("HEAD", 0, &rev, NULL);
 	head = lookup_commit_reference_gently(the_repository, &rev, 1);
 
-	errs |= post_checkout_hook(head, head, 0);
-	return errs;
+	ret = post_checkout_hook(head, head, 0);
+	return !errs ? ret : (errs < 0 ? 1 : errs);
 }
 
 static void show_local_changes(struct object *head,
@@ -1062,6 +1062,8 @@ static int switch_branches(const struct checkout_opts *opts,
 
 	ret = post_checkout_hook(old_branch_info.commit, new_branch_info->commit, 1);
 	free(path_to_free);
+	if (ret > 0)
+		return ret;
 	return ret || writeout_error;
 }
 
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index a39b3b5c78..60ce81f6db 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -73,4 +73,13 @@ test_expect_success 'post-checkout hook is triggered by clone' '
 	test -f clone3/.git/post-checkout.args
 '
 
+test_expect_success 'exit code of post-checkout hook is passed through' '
+	mkdir -p exit-code &&
+	echo "exit 123" | write_script exit-code/post-checkout &&
+	test_expect_code 123 \
+	git -c core.hooksPath="$PWD/exit-code" switch -c trigger-exit-code &&
+	test_expect_code 123 \
+	git -c core.hooksPath="$PWD/exit-code" restore .
+'
+
 test_done
-- 
gitgitgadget

  reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10 12:01 [PATCH 0/1] Pass through the exit code of post-checkout Johannes Schindelin via GitGitGadget
2019-10-10 12:01 ` Johannes Schindelin via GitGitGadget [this message]
2019-10-11  4:22 ` Junio C Hamano

Reply instructions:

You may reply publically 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=83305732371f3f511b3f4acf2060610c80ad12fa.1570708898.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=jrnieder@gmail.com \
    --cc=magne.land@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)

Archives are clonable:
	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

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.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

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