[PATCH 1/2] t5403: simplify by using a single repository

[PATCH 1/2] t5403: simplify by using a single repository

[orgads]

From: Orgad Shaneh <orgads@gmail.com>

There is no strong reason to use separate clones to run
these tests; just use a single test repository prepared
with more modern test_commit shell helper function.

While at it, replace three "awk '{print $N}'" on the same
file with shell built-in "read" into three variables.

Revert d42ec126aa717d00549e387d5a95fd55683c2e2c which is a workaround for
Cygwin that is no longer needed.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 78 +++++++++++------------------------
 1 file changed, 23 insertions(+), 55 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index fc898c9eac..1d15a1031f 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -7,82 +7,50 @@ test_description='Test the post-checkout hook.'
 . ./test-lib.sh
 
 test_expect_success setup '
-	echo Data for commit0. >a &&
-	echo Data for commit0. >b &&
-	git update-index --add a &&
-	git update-index --add b &&
-	tree0=$(git write-tree) &&
-	commit0=$(echo setup | git commit-tree $tree0) &&
-	git update-ref refs/heads/master $commit0 &&
-	git clone ./. clone1 &&
-	git clone ./. clone2 &&
-	GIT_DIR=clone2/.git git branch new2 &&
-	echo Data for commit1. >clone2/b &&
-	GIT_DIR=clone2/.git git add clone2/b &&
-	GIT_DIR=clone2/.git git commit -m new2
-'
-
-for clone in 1 2; do
-    cat >clone${clone}/.git/hooks/post-checkout <<'EOF'
-#!/bin/sh
-echo $@ > $GIT_DIR/post-checkout.args
-EOF
-    chmod u+x clone${clone}/.git/hooks/post-checkout
-done
-
-test_expect_success 'post-checkout runs as expected ' '
-	GIT_DIR=clone1/.git git checkout master &&
-	test -e clone1/.git/post-checkout.args
+	mkdir -p .git/hooks &&
+	write_script .git/hooks/post-checkout <<-\EOF &&
+	echo "$@" >.git/post-checkout.args
+	EOF
+	test_commit one &&
+	test_commit two &&
+	test_commit three
 '
 
 test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
-	old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+	test_when_finished "rm -f .git/post-checkout.args" &&
+	git checkout master &&
+	read old new flag <.git/post-checkout.args &&
 	test $old = $new && test $flag = 1
 '
 
-test_expect_success 'post-checkout runs as expected ' '
-	GIT_DIR=clone1/.git git checkout master &&
-	test -e clone1/.git/post-checkout.args
-'
-
 test_expect_success 'post-checkout args are correct with git checkout -b ' '
-	GIT_DIR=clone1/.git git checkout -b new1 &&
-	old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+	test_when_finished "rm -f .git/post-checkout.args" &&
+	git checkout -b new1 &&
+	read old new flag <.git/post-checkout.args &&
 	test $old = $new && test $flag = 1
 '
 
 test_expect_success 'post-checkout receives the right args with HEAD changed ' '
-	GIT_DIR=clone2/.git git checkout new2 &&
-	old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+	test_when_finished "rm -f .git/post-checkout.args" &&
+	git checkout two &&
+	read old new flag <.git/post-checkout.args &&
 	test $old != $new && test $flag = 1
 '
 
 test_expect_success 'post-checkout receives the right args when not switching branches ' '
-	GIT_DIR=clone2/.git git checkout master b &&
-	old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+	test_when_finished "rm -f .git/post-checkout.args" &&
+	git checkout master -- three.t &&
+	read old new flag <.git/post-checkout.args &&
 	test $old = $new && test $flag = 0
 '
 
-if test "$(git config --bool core.filemode)" = true; then
-mkdir -p templates/hooks
-cat >templates/hooks/post-checkout <<'EOF'
-#!/bin/sh
-echo $@ > $GIT_DIR/post-checkout.args
-EOF
-chmod +x templates/hooks/post-checkout
-
 test_expect_success 'post-checkout hook is triggered by clone' '
+	mkdir -p templates/hooks &&
+	write_script templates/hooks/post-checkout <<-\EOF &&
+	echo "$@" >$GIT_DIR/post-checkout.args
+	EOF
 	git clone --template=templates . clone3 &&
 	test -f clone3/.git/post-checkout.args
 '
-fi
 
 test_done
-- 
2.20.1

[PATCH 2/2] Rebase: Run post-checkout hook on checkout

[orgads]

From: Orgad Shaneh <orgads@gmail.com>

The scripted version of rebase used to run this hook on the initial
checkout. The transition to built-in introduced a regression.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 builtin/rebase.c              | 12 ++++++++++--
 t/t5403-post-checkout-hook.sh | 20 ++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index b5c99ec10c..8402765a79 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
 
 #define RESET_HEAD_DETACH (1<<0)
 #define RESET_HEAD_HARD (1<<1)
+#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
 
 static int reset_head(struct object_id *oid, const char *action,
 		      const char *switch_to_branch, unsigned flags,
@@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
 {
 	unsigned detach_head = flags & RESET_HEAD_DETACH;
 	unsigned reset_hard = flags & RESET_HEAD_HARD;
+	unsigned run_hook = flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
 	struct object_id head_oid;
 	struct tree_desc desc[2] = { { NULL }, { NULL } };
 	struct lock_file lock = LOCK_INIT;
@@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
 			ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
 					 UPDATE_REFS_MSG_ON_ERR);
 	}
+	if (run_hook)
+		run_hook_le(NULL, "post-checkout",
+			    oid_to_hex(orig ? orig : &null_oid),
+			    oid_to_hex(oid), "1", NULL);
 
 leave_reset_head:
 	strbuf_release(&msg);
@@ -1465,7 +1471,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 					    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
 					    options.switch_to);
 				if (reset_head(&oid, "checkout",
-					       options.head_name, 0,
+					       options.head_name,
+					       RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
 					       NULL, buf.buf) < 0) {
 					ret = !!error(_("could not switch to "
 							"%s"),
@@ -1539,7 +1546,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	strbuf_addf(&msg, "%s: checkout %s",
 		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
 	if (reset_head(&options.onto->object.oid, "checkout", NULL,
-		       RESET_HEAD_DETACH, NULL, msg.buf))
+		       RESET_HEAD_DETACH | RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
+		       NULL, msg.buf))
 		die(_("Could not detach HEAD"));
 	strbuf_release(&msg);
 
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 1d15a1031f..a539ffc080 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -13,6 +13,8 @@ test_expect_success setup '
 	EOF
 	test_commit one &&
 	test_commit two &&
+	test_commit rebase-on-me &&
+	git reset --hard HEAD^ &&
 	test_commit three
 '
 
@@ -44,6 +46,24 @@ test_expect_success 'post-checkout receives the right args when not switching br
 	test $old = $new && test $flag = 0
 '
 
+test_expect_success 'post-checkout is triggered on rebase' '
+	test_when_finished "rm -f .git/post-checkout.args" &&
+	git checkout -b rebase-test master &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag <.git/post-checkout.args &&
+	test $old != $new && test $flag = 1
+'
+
+test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
+	test_when_finished "rm -f .git/post-checkout.args" &&
+	git checkout -b ff-rebase-test rebase-on-me^ &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag <.git/post-checkout.args &&
+	test $old != $new && test $flag = 1
+'
+
 test_expect_success 'post-checkout hook is triggered by clone' '
 	mkdir -p templates/hooks &&
 	write_script templates/hooks/post-checkout <<-\EOF &&
-- 
2.20.1

Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout

[Johannes Schindelin]

Hi Orgad,

On Sat, 29 Dec 2018, orgads@gmail.com wrote:

> From: Orgad Shaneh <orgads@gmail.com>
> 
> The scripted version of rebase used to run this hook on the initial
> checkout. The transition to built-in introduced a regression.
> 
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>

ACK!

For lurkers: there was a "pre-review" at
https://github.com/git-for-windows/git/pull/1992

Ciao,
Dscho

> ---
>  builtin/rebase.c              | 12 ++++++++++--
>  t/t5403-post-checkout-hook.sh | 20 ++++++++++++++++++++
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index b5c99ec10c..8402765a79 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
>  
>  #define RESET_HEAD_DETACH (1<<0)
>  #define RESET_HEAD_HARD (1<<1)
> +#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
>  
>  static int reset_head(struct object_id *oid, const char *action,
>  		      const char *switch_to_branch, unsigned flags,
> @@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
>  {
>  	unsigned detach_head = flags & RESET_HEAD_DETACH;
>  	unsigned reset_hard = flags & RESET_HEAD_HARD;
> +	unsigned run_hook = flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
>  	struct object_id head_oid;
>  	struct tree_desc desc[2] = { { NULL }, { NULL } };
>  	struct lock_file lock = LOCK_INIT;
> @@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
>  			ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
>  					 UPDATE_REFS_MSG_ON_ERR);
>  	}
> +	if (run_hook)
> +		run_hook_le(NULL, "post-checkout",
> +			    oid_to_hex(orig ? orig : &null_oid),
> +			    oid_to_hex(oid), "1", NULL);
>  
>  leave_reset_head:
>  	strbuf_release(&msg);
> @@ -1465,7 +1471,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  					    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
>  					    options.switch_to);
>  				if (reset_head(&oid, "checkout",
> -					       options.head_name, 0,
> +					       options.head_name,
> +					       RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
>  					       NULL, buf.buf) < 0) {
>  					ret = !!error(_("could not switch to "
>  							"%s"),
> @@ -1539,7 +1546,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	strbuf_addf(&msg, "%s: checkout %s",
>  		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
>  	if (reset_head(&options.onto->object.oid, "checkout", NULL,
> -		       RESET_HEAD_DETACH, NULL, msg.buf))
> +		       RESET_HEAD_DETACH | RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
> +		       NULL, msg.buf))
>  		die(_("Could not detach HEAD"));
>  	strbuf_release(&msg);
>  
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 1d15a1031f..a539ffc080 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -13,6 +13,8 @@ test_expect_success setup '
>  	EOF
>  	test_commit one &&
>  	test_commit two &&
> +	test_commit rebase-on-me &&
> +	git reset --hard HEAD^ &&
>  	test_commit three
>  '
>  
> @@ -44,6 +46,24 @@ test_expect_success 'post-checkout receives the right args when not switching br
>  	test $old = $new && test $flag = 0
>  '
>  
> +test_expect_success 'post-checkout is triggered on rebase' '
> +	test_when_finished "rm -f .git/post-checkout.args" &&
> +	git checkout -b rebase-test master &&
> +	rm -f .git/post-checkout.args &&
> +	git rebase rebase-on-me &&
> +	read old new flag <.git/post-checkout.args &&
> +	test $old != $new && test $flag = 1
> +'
> +
> +test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
> +	test_when_finished "rm -f .git/post-checkout.args" &&
> +	git checkout -b ff-rebase-test rebase-on-me^ &&
> +	rm -f .git/post-checkout.args &&
> +	git rebase rebase-on-me &&
> +	read old new flag <.git/post-checkout.args &&
> +	test $old != $new && test $flag = 1
> +'
> +
>  test_expect_success 'post-checkout hook is triggered by clone' '
>  	mkdir -p templates/hooks &&
>  	write_script templates/hooks/post-checkout <<-\EOF &&
> -- 
> 2.20.1
> 
> 

[PATCH] Rebase: Run post-checkout hook on checkout

[orgads]

From: Orgad Shaneh <orgads@gmail.com>

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 builtin/rebase.c              | 11 +++++++++--
 t/t5403-post-checkout-hook.sh | 20 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index b5c99ec10c..7f7a2c738e 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
 
 #define RESET_HEAD_DETACH (1<<0)
 #define RESET_HEAD_HARD (1<<1)
+#define RESET_HEAD_RUN_HOOK (1<<2)
 
 static int reset_head(struct object_id *oid, const char *action,
 		      const char *switch_to_branch, unsigned flags,
@@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
 {
 	unsigned detach_head = flags & RESET_HEAD_DETACH;
 	unsigned reset_hard = flags & RESET_HEAD_HARD;
+	unsigned run_hook = flags & RESET_HEAD_RUN_HOOK;
 	struct object_id head_oid;
 	struct tree_desc desc[2] = { { NULL }, { NULL } };
 	struct lock_file lock = LOCK_INIT;
@@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
 			ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
 					 UPDATE_REFS_MSG_ON_ERR);
 	}
+	if (run_hook)
+		run_hook_le(NULL, "post-checkout",
+			    oid_to_hex(orig ? orig : &null_oid),
+			    oid_to_hex(oid), "1", NULL);
 
 leave_reset_head:
 	strbuf_release(&msg);
@@ -1465,7 +1471,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 					    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
 					    options.switch_to);
 				if (reset_head(&oid, "checkout",
-					       options.head_name, 0,
+					       options.head_name,
+					       RESET_HEAD_RUN_HOOK,
 					       NULL, buf.buf) < 0) {
 					ret = !!error(_("could not switch to "
 							"%s"),
@@ -1539,7 +1546,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	strbuf_addf(&msg, "%s: checkout %s",
 		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
 	if (reset_head(&options.onto->object.oid, "checkout", NULL,
-		       RESET_HEAD_DETACH, NULL, msg.buf))
+		       RESET_HEAD_DETACH | RESET_HEAD_RUN_HOOK, NULL, msg.buf))
 		die(_("Could not detach HEAD"));
 	strbuf_release(&msg);
 
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 9f9a5163c5..5b4e582caa 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -13,6 +13,8 @@ test_expect_success setup '
 	EOF
 	test_commit one &&
 	test_commit two &&
+	test_commit rebase-on-me &&
+	git reset --hard HEAD^ &&
 	test_commit three three
 '
 
@@ -51,6 +53,24 @@ test_expect_success 'post-checkout receives the right args when not switching br
 	rm -f .git/post-checkout.args
 '
 
+test_expect_success 'post-checkout is triggered on rebase' '
+	git checkout -b rebase-test master &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag < .git/post-checkout.args &&
+	test $old != $new && test $flag = 1 &&
+	rm -f .git/post-checkout.args
+'
+
+test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
+	git checkout -b ff-rebase-test rebase-on-me^ &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag < .git/post-checkout.args &&
+	test $old != $new && test $flag = 1 &&
+	rm -f .git/post-checkout.args
+'
+
 if test "$(git config --bool core.filemode)" = true; then
 mkdir -p templates/hooks
 write_script templates/hooks/post-checkout <<-\EOF
-- 
2.20.1

[PATCH 2/2] Rebase: Run post-checkout hook on checkout

[orgads]

From: Orgad Shaneh <orgads@gmail.com>

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 builtin/rebase.c              | 11 +++++++++--
 t/t5403-post-checkout-hook.sh | 20 ++++++++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index b5c99ec10c..7f7a2c738e 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
 
 #define RESET_HEAD_DETACH (1<<0)
 #define RESET_HEAD_HARD (1<<1)
+#define RESET_HEAD_RUN_HOOK (1<<2)
 
 static int reset_head(struct object_id *oid, const char *action,
 		      const char *switch_to_branch, unsigned flags,
@@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
 {
 	unsigned detach_head = flags & RESET_HEAD_DETACH;
 	unsigned reset_hard = flags & RESET_HEAD_HARD;
+	unsigned run_hook = flags & RESET_HEAD_RUN_HOOK;
 	struct object_id head_oid;
 	struct tree_desc desc[2] = { { NULL }, { NULL } };
 	struct lock_file lock = LOCK_INIT;
@@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
 			ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
 					 UPDATE_REFS_MSG_ON_ERR);
 	}
+	if (run_hook)
+		run_hook_le(NULL, "post-checkout",
+			    oid_to_hex(orig ? orig : &null_oid),
+			    oid_to_hex(oid), "1", NULL);
 
 leave_reset_head:
 	strbuf_release(&msg);
@@ -1465,7 +1471,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 					    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
 					    options.switch_to);
 				if (reset_head(&oid, "checkout",
-					       options.head_name, 0,
+					       options.head_name,
+					       RESET_HEAD_RUN_HOOK,
 					       NULL, buf.buf) < 0) {
 					ret = !!error(_("could not switch to "
 							"%s"),
@@ -1539,7 +1546,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	strbuf_addf(&msg, "%s: checkout %s",
 		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
 	if (reset_head(&options.onto->object.oid, "checkout", NULL,
-		       RESET_HEAD_DETACH, NULL, msg.buf))
+		       RESET_HEAD_DETACH | RESET_HEAD_RUN_HOOK, NULL, msg.buf))
 		die(_("Could not detach HEAD"));
 	strbuf_release(&msg);
 
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 9f9a5163c5..5b4e582caa 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -13,6 +13,8 @@ test_expect_success setup '
 	EOF
 	test_commit one &&
 	test_commit two &&
+	test_commit rebase-on-me &&
+	git reset --hard HEAD^ &&
 	test_commit three three
 '
 
@@ -51,6 +53,24 @@ test_expect_success 'post-checkout receives the right args when not switching br
 	rm -f .git/post-checkout.args
 '
 
+test_expect_success 'post-checkout is triggered on rebase' '
+	git checkout -b rebase-test master &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag < .git/post-checkout.args &&
+	test $old != $new && test $flag = 1 &&
+	rm -f .git/post-checkout.args
+'
+
+test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
+	git checkout -b ff-rebase-test rebase-on-me^ &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag < .git/post-checkout.args &&
+	test $old != $new && test $flag = 1 &&
+	rm -f .git/post-checkout.args
+'
+
 if test "$(git config --bool core.filemode)" = true; then
 mkdir -p templates/hooks
 write_script templates/hooks/post-checkout <<-\EOF
-- 
2.20.1

Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout

[Junio C Hamano]

orgads@gmail.com writes:

> From: Orgad Shaneh <orgads@gmail.com>

> Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout

There is no explanation here?  

Is this a regression fix (i.e. scripted version of "rebase" used to
run the hook)?  Or a new feature (i.e. no earlier version of
"rebase" run the hook but you think it ought to run it)?

> Signed-off-by: Orgad Shaneh <orgads@gmail.com>
> ---
>  builtin/rebase.c              | 11 +++++++++--
>  t/t5403-post-checkout-hook.sh | 20 ++++++++++++++++++++
>  2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index b5c99ec10c..7f7a2c738e 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
>  
>  #define RESET_HEAD_DETACH (1<<0)
>  #define RESET_HEAD_HARD (1<<1)
> +#define RESET_HEAD_RUN_HOOK (1<<2)

Would it be plausible that the only possible hook that can be run by
reset_head() function will always be post-checkout and nothing else,
I wonder?  Shouldn't this bit be called *_RUN_POST_CHECKOUT to make
sure it is specific enough?

> @@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
>  {
>  	unsigned detach_head = flags & RESET_HEAD_DETACH;
>  	unsigned reset_hard = flags & RESET_HEAD_HARD;
> +	unsigned run_hook = flags & RESET_HEAD_RUN_HOOK;
>  	struct object_id head_oid;
>  	struct tree_desc desc[2] = { { NULL }, { NULL } };
>  	struct lock_file lock = LOCK_INIT;
> @@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
>  			ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
>  					 UPDATE_REFS_MSG_ON_ERR);
>  	}
> +	if (run_hook)
> +		run_hook_le(NULL, "post-checkout",
> +			    oid_to_hex(orig ? orig : &null_oid),
> +			    oid_to_hex(oid), "1", NULL);
>  

This function is never about checking out paths from tree-ish/index
and is always about checking out a branch, so hardcoded "1" is
justified here.  Good.

> @@ -1465,7 +1471,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  					    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
>  					    options.switch_to);
>  				if (reset_head(&oid, "checkout",
> -					       options.head_name, 0,
> +					       options.head_name,
> +					       RESET_HEAD_RUN_HOOK,
>  					       NULL, buf.buf) < 0) {
>  					ret = !!error(_("could not switch to "
>  							"%s"),
> @@ -1539,7 +1546,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	strbuf_addf(&msg, "%s: checkout %s",
>  		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
>  	if (reset_head(&options.onto->object.oid, "checkout", NULL,
> -		       RESET_HEAD_DETACH, NULL, msg.buf))
> +		       RESET_HEAD_DETACH | RESET_HEAD_RUN_HOOK, NULL, msg.buf))
>  		die(_("Could not detach HEAD"));
>  	strbuf_release(&msg);

So... among 6 callers of reset_head(), these two whose *action is
"checkout" gets the RUN_HOOK bit.  Makes sense.

> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 9f9a5163c5..5b4e582caa 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -13,6 +13,8 @@ test_expect_success setup '
>  	EOF
>  	test_commit one &&
>  	test_commit two &&
> +	test_commit rebase-on-me &&
> +	git reset --hard HEAD^ &&
>  	test_commit three three
>  '
>  
> @@ -51,6 +53,24 @@ test_expect_success 'post-checkout receives the right args when not switching br
>  	rm -f .git/post-checkout.args
>  '
>  
> +test_expect_success 'post-checkout is triggered on rebase' '
> +	git checkout -b rebase-test master &&
> +	rm -f .git/post-checkout.args &&

Read the title of this whole test script file; it should verify what
is in the file before removing it.

> +	git rebase rebase-on-me &&
> +	read old new flag < .git/post-checkout.args &&

No SP between "<" and ".git/post-checkout.args".

> +	test $old != $new && test $flag = 1 &&
> +	rm -f .git/post-checkout.args
> +'

Regarding the clean-up of this file, see my review on the previous
one.


> +test_expect_success 'post-checkout is triggered on rebase with fast-forward' '

The same comment as above applies to this.


Thanks.

Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout

[Orgad Shaneh]

On Sat, Dec 29, 2018 at 12:53 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> orgads@gmail.com writes:
>
> > From: Orgad Shaneh <orgads@gmail.com>
>
> > Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout
>
> There is no explanation here?
>
> Is this a regression fix (i.e. scripted version of "rebase" used to
> run the hook)?  Or a new feature (i.e. no earlier version of
> "rebase" run the hook but you think it ought to run it)?

Added an explanation.

[snip]
>
> > +#define RESET_HEAD_RUN_HOOK (1<<2)
>
> Would it be plausible that the only possible hook that can be run by
> reset_head() function will always be post-checkout and nothing else,
> I wonder?  Shouldn't this bit be called *_RUN_POST_CHECKOUT to make
> sure it is specific enough?

Done

[snip]
>
> > +test_expect_success 'post-checkout is triggered on rebase' '
> > +     git checkout -b rebase-test master &&
> > +     rm -f .git/post-checkout.args &&
>
> Read the title of this whole test script file; it should verify what
> is in the file before removing it.

Why? Other tests already do it. This test is meant for rebase, not
plain checkout.

>
> > +     git rebase rebase-on-me &&
> > +     read old new flag < .git/post-checkout.args &&
>
> No SP between "<" and ".git/post-checkout.args".

Done

> > +     test $old != $new && test $flag = 1 &&
> > +     rm -f .git/post-checkout.args
> > +'
>
> Regarding the clean-up of this file, see my review on the previous
> one.

Done

> > +test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
>
> The same comment as above applies to this.

Done

Thanks,
- Orgad

[PATCH 1/2] t5403: Refactor

[orgads]

From: Orgad Shaneh <orgads@gmail.com>

* Replace multiple clones and commits by test_commits.
* Replace 3 invocations of awk by read.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 55 ++++++++++++-----------------------
 1 file changed, 18 insertions(+), 37 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index fc898c9eac..868d6f7272 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -7,67 +7,48 @@ test_description='Test the post-checkout hook.'
 . ./test-lib.sh
 
 test_expect_success setup '
-	echo Data for commit0. >a &&
-	echo Data for commit0. >b &&
-	git update-index --add a &&
-	git update-index --add b &&
-	tree0=$(git write-tree) &&
-	commit0=$(echo setup | git commit-tree $tree0) &&
-	git update-ref refs/heads/master $commit0 &&
-	git clone ./. clone1 &&
-	git clone ./. clone2 &&
-	GIT_DIR=clone2/.git git branch new2 &&
-	echo Data for commit1. >clone2/b &&
-	GIT_DIR=clone2/.git git add clone2/b &&
-	GIT_DIR=clone2/.git git commit -m new2
+	test_commit one &&
+	test_commit two &&
+	test_commit three three &&
+	mv .git/hooks-disabled .git/hooks
 '
 
-for clone in 1 2; do
-    cat >clone${clone}/.git/hooks/post-checkout <<'EOF'
+cat >.git/hooks/post-checkout <<'EOF'
 #!/bin/sh
-echo $@ > $GIT_DIR/post-checkout.args
+echo $@ > .git/post-checkout.args
 EOF
-    chmod u+x clone${clone}/.git/hooks/post-checkout
-done
+chmod u+x .git/hooks/post-checkout
 
 test_expect_success 'post-checkout runs as expected ' '
-	GIT_DIR=clone1/.git git checkout master &&
-	test -e clone1/.git/post-checkout.args
+	git checkout master &&
+	test -e .git/post-checkout.args
 '
 
 test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
-	old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+	read old new flag < .git/post-checkout.args &&
 	test $old = $new && test $flag = 1
 '
 
 test_expect_success 'post-checkout runs as expected ' '
-	GIT_DIR=clone1/.git git checkout master &&
-	test -e clone1/.git/post-checkout.args
+	git checkout master &&
+	test -e .git/post-checkout.args
 '
 
 test_expect_success 'post-checkout args are correct with git checkout -b ' '
-	GIT_DIR=clone1/.git git checkout -b new1 &&
-	old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+	git checkout -b new1 &&
+	read old new flag < .git/post-checkout.args &&
 	test $old = $new && test $flag = 1
 '
 
 test_expect_success 'post-checkout receives the right args with HEAD changed ' '
-	GIT_DIR=clone2/.git git checkout new2 &&
-	old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+	git checkout two &&
+	read old new flag < .git/post-checkout.args &&
 	test $old != $new && test $flag = 1
 '
 
 test_expect_success 'post-checkout receives the right args when not switching branches ' '
-	GIT_DIR=clone2/.git git checkout master b &&
-	old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+	git checkout master -- three &&
+	read old new flag < .git/post-checkout.args &&
 	test $old = $new && test $flag = 0
 '
 
-- 
2.20.1

[PATCH 2/2] Rebase: Run post-checkout hook on checkout

[orgads]

From: Orgad Shaneh <orgads@gmail.com>

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 builtin/rebase.c              |  8 +++++++-
 t/t5403-post-checkout-hook.sh | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index b5c99ec10c..78a09dcda2 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
 
 #define RESET_HEAD_DETACH (1<<0)
 #define RESET_HEAD_HARD (1<<1)
+#define RESET_HEAD_RUN_HOOK (1<<2)
 
 static int reset_head(struct object_id *oid, const char *action,
 		      const char *switch_to_branch, unsigned flags,
@@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
 {
 	unsigned detach_head = flags & RESET_HEAD_DETACH;
 	unsigned reset_hard = flags & RESET_HEAD_HARD;
+	unsigned run_hook = flags & RESET_HEAD_RUN_HOOK;
 	struct object_id head_oid;
 	struct tree_desc desc[2] = { { NULL }, { NULL } };
 	struct lock_file lock = LOCK_INIT;
@@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
 			ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
 					 UPDATE_REFS_MSG_ON_ERR);
 	}
+	if (run_hook)
+		run_hook_le(NULL, "post-checkout",
+			    oid_to_hex(orig ? orig : &null_oid),
+			    oid_to_hex(oid), "1", NULL);
 
 leave_reset_head:
 	strbuf_release(&msg);
@@ -1539,7 +1545,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	strbuf_addf(&msg, "%s: checkout %s",
 		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
 	if (reset_head(&options.onto->object.oid, "checkout", NULL,
-		       RESET_HEAD_DETACH, NULL, msg.buf))
+		       RESET_HEAD_DETACH | RESET_HEAD_RUN_HOOK, NULL, msg.buf))
 		die(_("Could not detach HEAD"));
 	strbuf_release(&msg);
 
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 868d6f7272..ed4cc6e945 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -9,6 +9,8 @@ test_description='Test the post-checkout hook.'
 test_expect_success setup '
 	test_commit one &&
 	test_commit two &&
+	test_commit rebase-on-me &&
+	git reset --hard HEAD^ &&
 	test_commit three three &&
 	mv .git/hooks-disabled .git/hooks
 '
@@ -52,6 +54,22 @@ test_expect_success 'post-checkout receives the right args when not switching br
 	test $old = $new && test $flag = 0
 '
 
+test_expect_success 'post-checkout is triggered on rebase' '
+	git checkout -b rebase-test master &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag < .git/post-checkout.args &&
+	test $old != $new && test $flag = 1
+'
+
+test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
+	git checkout -b ff-rebase-test rebase-on-me^ &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag < .git/post-checkout.args &&
+	test $old != $new && test $flag = 1
+'
+
 if test "$(git config --bool core.filemode)" = true; then
 mkdir -p templates/hooks
 cat >templates/hooks/post-checkout <<'EOF'
-- 
2.20.1

[PATCH 1/2] t5403: Refactor

[orgads]

From: Orgad Shaneh <orgads@gmail.com>

* Replace multiple clones and commits by test_commits.
* Replace 3 invocations of awk by read.

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 55 ++++++++++++-----------------------
 1 file changed, 18 insertions(+), 37 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index fc898c9eac..7e941537f9 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -7,67 +7,48 @@ test_description='Test the post-checkout hook.'
 . ./test-lib.sh
 
 test_expect_success setup '
-	echo Data for commit0. >a &&
-	echo Data for commit0. >b &&
-	git update-index --add a &&
-	git update-index --add b &&
-	tree0=$(git write-tree) &&
-	commit0=$(echo setup | git commit-tree $tree0) &&
-	git update-ref refs/heads/master $commit0 &&
-	git clone ./. clone1 &&
-	git clone ./. clone2 &&
-	GIT_DIR=clone2/.git git branch new2 &&
-	echo Data for commit1. >clone2/b &&
-	GIT_DIR=clone2/.git git add clone2/b &&
-	GIT_DIR=clone2/.git git commit -m new2
+	test_commit one &&
+    test_commit two &&
+    test_commit three three &&
+    mv .git/hooks-disabled .git/hooks
 '
 
-for clone in 1 2; do
-    cat >clone${clone}/.git/hooks/post-checkout <<'EOF'
+cat >.git/hooks/post-checkout <<'EOF'
 #!/bin/sh
-echo $@ > $GIT_DIR/post-checkout.args
+echo $@ > .git/post-checkout.args
 EOF
-    chmod u+x clone${clone}/.git/hooks/post-checkout
-done
+chmod u+x .git/hooks/post-checkout
 
 test_expect_success 'post-checkout runs as expected ' '
-	GIT_DIR=clone1/.git git checkout master &&
-	test -e clone1/.git/post-checkout.args
+	git checkout master &&
+	test -e .git/post-checkout.args
 '
 
 test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
-	old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+	read old new flag < .git/post-checkout.args &&
 	test $old = $new && test $flag = 1
 '
 
 test_expect_success 'post-checkout runs as expected ' '
-	GIT_DIR=clone1/.git git checkout master &&
-	test -e clone1/.git/post-checkout.args
+	git checkout master &&
+	test -e .git/post-checkout.args
 '
 
 test_expect_success 'post-checkout args are correct with git checkout -b ' '
-	GIT_DIR=clone1/.git git checkout -b new1 &&
-	old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+	git checkout -b new1 &&
+    read old new flag < .git/post-checkout.args &&
 	test $old = $new && test $flag = 1
 '
 
 test_expect_success 'post-checkout receives the right args with HEAD changed ' '
-	GIT_DIR=clone2/.git git checkout new2 &&
-	old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+	git checkout two &&
+    read old new flag < .git/post-checkout.args &&
 	test $old != $new && test $flag = 1
 '
 
 test_expect_success 'post-checkout receives the right args when not switching branches ' '
-	GIT_DIR=clone2/.git git checkout master b &&
-	old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
-	new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
-	flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+	git checkout master -- three &&
+    read old new flag < .git/post-checkout.args &&
 	test $old = $new && test $flag = 0
 '
 
-- 
2.20.1

[PATCH 2/2] Rebase: Run post-checkout hook on checkout

[orgads]

From: Orgad Shaneh <orgads@gmail.com>

Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
 builtin/rebase.c              |  8 +++++++-
 t/t5403-post-checkout-hook.sh | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index b5c99ec10c..78a09dcda2 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
 
 #define RESET_HEAD_DETACH (1<<0)
 #define RESET_HEAD_HARD (1<<1)
+#define RESET_HEAD_RUN_HOOK (1<<2)
 
 static int reset_head(struct object_id *oid, const char *action,
 		      const char *switch_to_branch, unsigned flags,
@@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
 {
 	unsigned detach_head = flags & RESET_HEAD_DETACH;
 	unsigned reset_hard = flags & RESET_HEAD_HARD;
+	unsigned run_hook = flags & RESET_HEAD_RUN_HOOK;
 	struct object_id head_oid;
 	struct tree_desc desc[2] = { { NULL }, { NULL } };
 	struct lock_file lock = LOCK_INIT;
@@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
 			ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
 					 UPDATE_REFS_MSG_ON_ERR);
 	}
+	if (run_hook)
+		run_hook_le(NULL, "post-checkout",
+			    oid_to_hex(orig ? orig : &null_oid),
+			    oid_to_hex(oid), "1", NULL);
 
 leave_reset_head:
 	strbuf_release(&msg);
@@ -1539,7 +1545,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	strbuf_addf(&msg, "%s: checkout %s",
 		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
 	if (reset_head(&options.onto->object.oid, "checkout", NULL,
-		       RESET_HEAD_DETACH, NULL, msg.buf))
+		       RESET_HEAD_DETACH | RESET_HEAD_RUN_HOOK, NULL, msg.buf))
 		die(_("Could not detach HEAD"));
 	strbuf_release(&msg);
 
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 7e941537f9..de9c7fb871 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -9,6 +9,8 @@ test_description='Test the post-checkout hook.'
 test_expect_success setup '
 	test_commit one &&
     test_commit two &&
+    test_commit rebase-on-me &&
+    git reset --hard HEAD^ &&
     test_commit three three &&
     mv .git/hooks-disabled .git/hooks
 '
@@ -52,6 +54,22 @@ test_expect_success 'post-checkout receives the right args when not switching br
 	test $old = $new && test $flag = 0
 '
 
+test_expect_success 'post-checkout is triggered on rebase' '
+	git checkout -b rebase-test master &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag < .git/post-checkout.args &&
+	test $old != $new && test $flag = 1
+'
+
+test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
+	git checkout -b ff-rebase-test rebase-on-me^ &&
+	rm -f .git/post-checkout.args &&
+	git rebase rebase-on-me &&
+	read old new flag < .git/post-checkout.args &&
+	test $old != $new && test $flag = 1
+'
+
 if test "$(git config --bool core.filemode)" = true; then
 mkdir -p templates/hooks
 cat >templates/hooks/post-checkout <<'EOF'
-- 
2.20.1

Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout

[Johannes Schindelin]

Hi Orgad,

On Thu, 20 Dec 2018, orgads@gmail.com wrote:

> From: Orgad Shaneh <orgads@gmail.com>
> 
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>

Feel free to steal the PR description I added to your PR at
https://github.com/git-for-windows/git/pull/1992

> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index b5c99ec10c..78a09dcda2 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
>  
>  #define RESET_HEAD_DETACH (1<<0)
>  #define RESET_HEAD_HARD (1<<1)
> +#define RESET_HEAD_RUN_HOOK (1<<2)
>  
>  static int reset_head(struct object_id *oid, const char *action,
>  		      const char *switch_to_branch, unsigned flags,
> @@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
>  {
>  	unsigned detach_head = flags & RESET_HEAD_DETACH;
>  	unsigned reset_hard = flags & RESET_HEAD_HARD;
> +	unsigned run_hook = flags & RESET_HEAD_RUN_HOOK;
>  	struct object_id head_oid;
>  	struct tree_desc desc[2] = { { NULL }, { NULL } };
>  	struct lock_file lock = LOCK_INIT;
> @@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
>  			ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
>  					 UPDATE_REFS_MSG_ON_ERR);
>  	}
> +	if (run_hook)
> +		run_hook_le(NULL, "post-checkout",
> +			    oid_to_hex(orig ? orig : &null_oid),
> +			    oid_to_hex(oid), "1", NULL);

IIRC there was one `git checkout` in the scripted version that ran the
hook, and one did not. The latter did not run it because it did not
actually change the HEAD, but just switched branches.

We could imitate that here by extending the `if (run_hook)` to `if
(run_hook && !oideq(&head_oid, oid))`, I think. Do you agree?

The rest of the patch makes sense to me (and I merged the above-mentioned
PR already).

Thank you!
Johannes

>  
>  leave_reset_head:
>  	strbuf_release(&msg);
> @@ -1539,7 +1545,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>  	strbuf_addf(&msg, "%s: checkout %s",
>  		    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
>  	if (reset_head(&options.onto->object.oid, "checkout", NULL,
> -		       RESET_HEAD_DETACH, NULL, msg.buf))
> +		       RESET_HEAD_DETACH | RESET_HEAD_RUN_HOOK, NULL, msg.buf))
>  		die(_("Could not detach HEAD"));
>  	strbuf_release(&msg);
>  
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 7e941537f9..de9c7fb871 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> @@ -9,6 +9,8 @@ test_description='Test the post-checkout hook.'
>  test_expect_success setup '
>  	test_commit one &&
>      test_commit two &&
> +    test_commit rebase-on-me &&
> +    git reset --hard HEAD^ &&
>      test_commit three three &&
>      mv .git/hooks-disabled .git/hooks
>  '
> @@ -52,6 +54,22 @@ test_expect_success 'post-checkout receives the right args when not switching br
>  	test $old = $new && test $flag = 0
>  '
>  
> +test_expect_success 'post-checkout is triggered on rebase' '
> +	git checkout -b rebase-test master &&
> +	rm -f .git/post-checkout.args &&
> +	git rebase rebase-on-me &&
> +	read old new flag < .git/post-checkout.args &&
> +	test $old != $new && test $flag = 1
> +'
> +
> +test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
> +	git checkout -b ff-rebase-test rebase-on-me^ &&
> +	rm -f .git/post-checkout.args &&
> +	git rebase rebase-on-me &&
> +	read old new flag < .git/post-checkout.args &&
> +	test $old != $new && test $flag = 1
> +'
> +
>  if test "$(git config --bool core.filemode)" = true; then
>  mkdir -p templates/hooks
>  cat >templates/hooks/post-checkout <<'EOF'
> -- 
> 2.20.1
> 
> 

Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout

[Orgad Shaneh]

Hi Johannes,

On Fri, Dec 21, 2018 at 6:12 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Orgad,
>
> On Thu, 20 Dec 2018, orgads@gmail.com wrote:
>
> > From: Orgad Shaneh <orgads@gmail.com>
> >
> > Signed-off-by: Orgad Shaneh <orgads@gmail.com>
>
> Feel free to steal the PR description I added to your PR at
> https://github.com/git-for-windows/git/pull/1992
>
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index b5c99ec10c..78a09dcda2 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
> >
> >  #define RESET_HEAD_DETACH (1<<0)
> >  #define RESET_HEAD_HARD (1<<1)
> > +#define RESET_HEAD_RUN_HOOK (1<<2)
> >
> >  static int reset_head(struct object_id *oid, const char *action,
> >                     const char *switch_to_branch, unsigned flags,
> > @@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
> >  {
> >       unsigned detach_head = flags & RESET_HEAD_DETACH;
> >       unsigned reset_hard = flags & RESET_HEAD_HARD;
> > +     unsigned run_hook = flags & RESET_HEAD_RUN_HOOK;
> >       struct object_id head_oid;
> >       struct tree_desc desc[2] = { { NULL }, { NULL } };
> >       struct lock_file lock = LOCK_INIT;
> > @@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
> >                       ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
> >                                        UPDATE_REFS_MSG_ON_ERR);
> >       }
> > +     if (run_hook)
> > +             run_hook_le(NULL, "post-checkout",
> > +                         oid_to_hex(orig ? orig : &null_oid),
> > +                         oid_to_hex(oid), "1", NULL);
>
> IIRC there was one `git checkout` in the scripted version that ran the
> hook, and one did not. The latter did not run it because it did not
> actually change the HEAD, but just switched branches.
>
> We could imitate that here by extending the `if (run_hook)` to `if
> (run_hook && !oideq(&head_oid, oid))`, I think. Do you agree?

Not exactly. That's what I thought, but it looks like I was wrong.
Returning to the branch is done in sequencer.c. The 2 calls to reset_head
happen on fast-forward case.

On fast-forward there is a call to reset_head with "checkout",
followed by a call
with "Fast-forwarded". I'm not sure what exactly it does, but on my test it sent
a null oid.

I did however miss the `switch-to` case, which I now added too.

- Orgad