threads / patch / 21237

patchcheckout: add 'pre-checkout' hook

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

## tl;dr

9 messages between Oct 14, 2009 and Oct 14, 2009. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Sam Vilain· Oct 14, 2009, 04:45 UTC · lore
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(-)
Show changes to 2 files +37 −8

Documentation/githooks.txt, builtin-checkout.c

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
Jeff King· Oct 14, 2009, 05:13 UTC · re: Sam Vilain · lore

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

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.

Show 5 quoted lines
> -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?
Show 11 quoted lines
>  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
Sam Vilain· Oct 14, 2009, 05:25 UTC · re: Jeff King · lore

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

Jeff King wrote:
Show 7 quoted lines
>> 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
Junio C Hamano· Oct 14, 2009, 05:13 UTC · re: Sam Vilain · lore

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

Sam Vilain <sam.vilain@catalyst.net.nz> writes:
Show 24 quoted lines
> 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"?
Show 17 quoted lines
>  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.

Sam Vilain· Oct 14, 2009, 05:22 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 28 quoted lines
> 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
Jeff King· Oct 14, 2009, 05:25 UTC · re: Junio C Hamano · lore

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

On Tue, Oct 13, 2009 at 10:13:39PM -0700, Junio C Hamano wrote:
Show 22 quoted lines
> >  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
Bert Wesarg· Oct 14, 2009, 06:49 UTC · re: Sam Vilain · lore

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

On Wed, Oct 14, 2009 at 06:45, Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
Show 14 quoted lines
> 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. ---)?
Show 19 quoted lines
> -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.
Show 8 quoted lines
> +
> +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
Sam Vilain· Oct 14, 2009, 06:50 UTC · re: Bert Wesarg · lore

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

Bert Wesarg wrote:
Show 7 quoted lines
>> +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
Bert Wesarg· Oct 14, 2009, 07:04 UTC · re: Sam Vilain · lore

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

On Wed, Oct 14, 2009 at 08:50, Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
Show 11 quoted lines
> 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 ;-)

← back to recent threads