[PATCH] checkout: add 'pre-checkout' hook

[PATCH] checkout: add 'pre-checkout' hook

[Sam Vilain]

Add a simple hook that will run before checkouts.

Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
---
 Documentation/githooks.txt |   20 +++++++++++++++-----
 builtin-checkout.c         |   25 ++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 06e0f31..8dc3fbf 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -143,21 +143,31 @@ pre-rebase
 This hook is called by 'git-rebase' and can be used to prevent a branch
 from getting rebased.
 
+pre-checkout
+-----------
 
-post-checkout
-~~~~~~~~~~~~~
-
-This hook is invoked when a 'git-checkout' is run after having updated the
+This hook is invoked when a 'git-checkout' is run after before updating the
 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-checkout'.
+This hook can prevent the checkout from proceeding by exiting with an
+error code.
 
 It is also run after 'git-clone', unless the --no-checkout (-n) option is
 used. The first parameter given to the hook is the null-ref, the second the
 ref of the new HEAD and the flag is always 1.
 
+This hook can be used to perform any clean-up deemed necessary before
+checking out the new branch/files.
+
+post-checkout
+-----------
+
+This hook is invoked when a 'git-checkout' is run after having updated the
+worktree.  It takes the same arguments as the 'pre-checkout' hook.
+This hook cannot affect the outcome of 'git-checkout'.
+
 This hook can be used to perform repository validity checks, auto-display
 differences from the previous HEAD if different, or set working dir metadata
 properties.
diff --git a/builtin-checkout.c b/builtin-checkout.c
index d050c37..b72a3cb 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -36,6 +36,17 @@ struct checkout_opts {
 	enum branch_track track;
 };
 
+static int pre_checkout_hook(struct commit *old, struct commit *new,
+			      int changed)
+{
+	return run_hook(NULL, "pre-checkout",
+			sha1_to_hex(old ? old->object.sha1 : null_sha1),
+			sha1_to_hex(new ? new->object.sha1 : null_sha1),
+			changed ? "1" : "0", NULL);
+	/* "new" can be NULL when checking out from the index before
+	   a commit exists. */
+}
+
 static int post_checkout_hook(struct commit *old, struct commit *new,
 			      int changed)
 {
@@ -256,6 +267,13 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	if (errs)
 		return 1;
 
+	/* Run the pre-checkout hook */
+	resolve_ref("HEAD", rev, 0, &flag);
+	head = lookup_commit_reference_gently(rev, 1);
+	errs = pre_checkout_hook(head, head, 0);
+	if (errs)
+		return 1;
+
 	/* Now we are committed to check them out */
 	memset(&state, 0, sizeof(state));
 	state.force = 1;
@@ -279,9 +297,6 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	    commit_locked_index(lock_file))
 		die("unable to write new index file");
 
-	resolve_ref("HEAD", rev, 0, &flag);
-	head = lookup_commit_reference_gently(rev, 1);
-
 	errs |= post_checkout_hook(head, head, 0);
 	return errs;
 }
@@ -543,6 +558,10 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 		parse_commit(new->commit);
 	}
 
+	ret = pre_checkout_hook(old.commit, new->commit, 1);
+	if (ret)
+		return ret;
+
 	ret = merge_working_tree(opts, &old, new);
 	if (ret)
 		return ret;
-- 
1.6.3.3

Re: [PATCH] checkout: add 'pre-checkout' hook

[Jeff King]

On Wed, Oct 14, 2009 at 05:45:25PM +1300, Sam Vilain wrote:

> Add a simple hook that will run before checkouts.

What is the use case that makes it useful as a hook, and not simply as
something people can do before running checkout?

I guess you can use it to block a checkout, but only after finding out
_what_ you are going to checkout, but an exact use case escapes me.

> -post-checkout
> -~~~~~~~~~~~~~
> -
> -This hook is invoked when a 'git-checkout' is run after having updated the
> +This hook is invoked when a 'git-checkout' is run after before updating the

Did you mean "before having" here?

>  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-checkout'.
> +This hook can prevent the checkout from proceeding by exiting with an
> +error code.
>  
>  It is also run after 'git-clone', unless the --no-checkout (-n) option is
>  used. The first parameter given to the hook is the null-ref, the second the
>  ref of the new HEAD and the flag is always 1.

Should this "after" in the bottom paragraph perhaps become "during"?

-Peff

Re: [PATCH] checkout: add 'pre-checkout' hook

[Junio C Hamano]

Sam Vilain <sam.vilain@catalyst.net.nz> writes:

> Add a simple hook that will run before checkouts.
>
> Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
> ---
>  Documentation/githooks.txt |   20 +++++++++++++++-----
>  builtin-checkout.c         |   25 ++++++++++++++++++++++---
>  2 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 06e0f31..8dc3fbf 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -143,21 +143,31 @@ pre-rebase
>  This hook is called by 'git-rebase' and can be used to prevent a branch
>  from getting rebased.
>  
> +pre-checkout
> +-----------
>  
> -post-checkout
> -~~~~~~~~~~~~~
> -
> -This hook is invoked when a 'git-checkout' is run after having updated the
> +This hook is invoked when a 'git-checkout' is run after before updating the

"after before"?

>  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-checkout'.
> +This hook can prevent the checkout from proceeding by exiting with an
> +error code.
>  
>  It is also run after 'git-clone', unless the --no-checkout (-n) option is
>  used. The first parameter given to the hook is the null-ref, the second the
>  ref of the new HEAD and the flag is always 1.
>  
> +This hook can be used to perform any clean-up deemed necessary before
> +checking out the new branch/files.
> +
> +post-checkout
> +-----------

This is not about your patch, but the patch text shows that our diff
algorithm seems to have a room for improvement.  I expected to see a
straight insersion of block of text, not touching anything in the original
section on post-checkout hook.

Re: [PATCH] checkout: add 'pre-checkout' hook

[Sam Vilain]

Junio C Hamano wrote:
> Sam Vilain <sam.vilain@catalyst.net.nz> writes:
> 
>> Add a simple hook that will run before checkouts.
>>
>> Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
>> ---
>>  Documentation/githooks.txt |   20 +++++++++++++++-----
>>  builtin-checkout.c         |   25 ++++++++++++++++++++++---
>>  2 files changed, 37 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> index 06e0f31..8dc3fbf 100644
>> --- a/Documentation/githooks.txt
>> +++ b/Documentation/githooks.txt
>> @@ -143,21 +143,31 @@ pre-rebase
>>  This hook is called by 'git-rebase' and can be used to prevent a branch
>>  from getting rebased.
>>  
>> +pre-checkout
>> +-----------
>>  
>> -post-checkout
>> -~~~~~~~~~~~~~
>> -
>> -This hook is invoked when a 'git-checkout' is run after having updated the
>> +This hook is invoked when a 'git-checkout' is run after before updating the
> 
> "after before"?

*ahem* whoops :).  I think I got the heading style wrong too...

> This is not about your patch, but the patch text shows that our diff
> algorithm seems to have a room for improvement.  I expected to see a
> straight insersion of block of text, not touching anything in the original
> section on post-checkout hook.

Correct.  This is because the paragraph explaining when the hook runs
has been moved to the pre-checkout paragraph, which appears before the
post-checkout section.  I just compared the output to 'diff -du' and it
seems to be the same, so I wouldn't worry too much.
-- 
Sam Vilain, Perl Hacker, Catalyst IT (NZ) Ltd.
phone: +64 4 499 2267        PGP ID: 0x66B25843

Re: [PATCH] checkout: add 'pre-checkout' hook

[Jeff King]

On Tue, Oct 13, 2009 at 10:13:39PM -0700, Junio C Hamano wrote:

> >  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-checkout'.
> > +This hook can prevent the checkout from proceeding by exiting with an
> > +error code.
> >  
> >  It is also run after 'git-clone', unless the --no-checkout (-n) option is
> >  used. The first parameter given to the hook is the null-ref, the second the
> >  ref of the new HEAD and the flag is always 1.
> >  
> > +This hook can be used to perform any clean-up deemed necessary before
> > +checking out the new branch/files.
> > +
> > +post-checkout
> > +-----------
> 
> This is not about your patch, but the patch text shows that our diff
> algorithm seems to have a room for improvement.  I expected to see a
> straight insersion of block of text, not touching anything in the original
> section on post-checkout hook.

I think it's right as-is. He changed the title of the section, made a
few tweaks in the text to make it appropriate for "pre-checkout", and
then made a new post-checkout section that says "This is just like
pre-checkout". So most of the lines were left untouched. Short of our
diff understanding the block-formatting of asciidoc, I think it's as
good as we can get.

-Peff

Re: [PATCH] checkout: add 'pre-checkout' hook

[Sam Vilain]

Jeff King wrote:
>> Add a simple hook that will run before checkouts.
> 
> What is the use case that makes it useful as a hook, and not simply as
> something people can do before running checkout?
> 
> I guess you can use it to block a checkout, but only after finding out
> _what_ you are going to checkout, but an exact use case escapes me.

Right.  Yes, this could be explained more.

Actually the use case is submodules.  When switching branches, the user
wants to add a hook to remove submodules.  However post-checkout is too
late, because the index and the .gitmodules file do not record the
submodule locations.

Of course the best explanation is a test case :) I'll look at cooking
one up.
-- 
Sam Vilain, Perl Hacker, Catalyst IT (NZ) Ltd.
phone: +64 4 499 2267        PGP ID: 0x66B25843

Re: [PATCH] checkout: add 'pre-checkout' hook

[Bert Wesarg]

On Wed, Oct 14, 2009 at 06:45, Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 06e0f31..8dc3fbf 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -143,21 +143,31 @@ pre-rebase
>  This hook is called by 'git-rebase' and can be used to prevent a branch
>  from getting rebased.
>
> +pre-checkout
> +-----------
>
> -post-checkout
> -~~~~~~~~~~~~~
> -
Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?

> -This hook is invoked when a 'git-checkout' is run after having updated the
> +This hook is invoked when a 'git-checkout' is run after before updating the
>  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-checkout'.
> +This hook can prevent the checkout from proceeding by exiting with an
> +error code.
>
>  It is also run after 'git-clone', unless the --no-checkout (-n) option is
>  used. The first parameter given to the hook is the null-ref, the second the
>  ref of the new HEAD and the flag is always 1.
>
> +This hook can be used to perform any clean-up deemed necessary before
> +checking out the new branch/files.
> +
> +post-checkout
> +-----------
Ditto.

> +
> +This hook is invoked when a 'git-checkout' is run after having updated the
> +worktree.  It takes the same arguments as the 'pre-checkout' hook.
> +This hook cannot affect the outcome of 'git-checkout'.
> +
>  This hook can be used to perform repository validity checks, auto-display
>  differences from the previous HEAD if different, or set working dir metadata
>  properties.

Bert

Re: [PATCH] checkout: add 'pre-checkout' hook

[Sam Vilain]

Bert Wesarg wrote:
>> +pre-checkout
>> +-----------
>>
>> -post-checkout
>> -~~~~~~~~~~~~~
>> -
> Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?

Possibly the headings changed from the version I originally patched to
the current 'master'.  More likely, I just made a simple mistake :-}
-- 
Sam Vilain, Perl Hacker, Catalyst IT (NZ) Ltd.
phone: +64 4 499 2267        PGP ID: 0x66B25843

Re: [PATCH] checkout: add 'pre-checkout' hook

[Bert Wesarg]

On Wed, Oct 14, 2009 at 08:50, Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
> Bert Wesarg wrote:
>>> +pre-checkout
>>> +-----------
>>>
>>> -post-checkout
>>> -~~~~~~~~~~~~~
>>> -
>> Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?
>
> Possibly the headings changed from the version I originally patched to
> the current 'master'.
That was me one month ago ;-)