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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

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
> 
>