[PATCH] Add post-merge hook.

[PATCH] Add post-merge hook.

[jjengla]

From: Josh England <jjengla@sandia.gov>

This adds a post-merge hook that will run after `git pull` operations
if enabled.  The hook is passed no arguments and cannot affect the
outcome of a merge.

Signed-off-by: Josh England <jjengla@sandia.gov>
---
 Documentation/hooks.txt |    8 ++++++++
 git-merge.sh            |    6 ++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index c39edc5..841b37f 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -87,6 +87,14 @@ parameter, and is invoked after a commit is made.
 This hook is meant primarily for notification, and cannot affect
 the outcome of `git-commit`.
 
+post-merge
+-----------
+
+This hook is invoked by `git-merge`, which happens when a `git pull`
+is done on a local repository.
+
+The hook takes no parameters, and cannot affect the outcome of `git-merge`.
+
 [[pre-receive]]
 pre-receive
 -----------
diff --git a/git-merge.sh b/git-merge.sh
index 3a01db0..0a77bfb 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -97,6 +97,12 @@ finish () {
 		fi
 		;;
 	esac
+
+	# Run a post-merge hook
+        if test -x "$GIT_DIR"/hooks/post-merge
+        then
+                "$GIT_DIR"/hooks/post-merge
+        fi
 }
 
 merge_name () {
-- 
1.5.3.rc7-dirty

Re: [PATCH] Add post-merge hook.

[Junio C Hamano]

jjengla@sandia.gov writes:

> From: Josh England <jjengla@sandia.gov>
>
> This adds a post-merge hook that will run after `git pull` operations
> if enabled.  The hook is passed no arguments and cannot affect the
> outcome of a merge.
>
> Signed-off-by: Josh England <jjengla@sandia.gov>

Thanks for your patch.

Two questions.

 * Do you want to run the post-merge hook even for a squash
   merge?

 * After resolving a conflicted merge, you would conclude it
   with "git commit"; don't you want to have the same hook apply
   at the commit time, or is that what you expect the user to
   deal with with post-commit hook?


And two requests and a half.

 - We would want a new test in the test suite for this, to make
   sure that later changes by others would not break this new
   feature you would depend upon.

 - You described _what_ the patch and the new feature do in the
   log message and in the documentation.

   You need to also explain _why_ somebody might want to have
   such a hook in his repository.

   . In the documentation, you would want to make the reader
     realize "aha, this is something that would help me" with an
     example.

   . In the log message, you would want to make sure other
     people understand why this new feature was added, what the
     design consideration were at the point of the feature was
     added.

   The latter is important to me personally.  I want to have
   something better than "this change does not break anything
   existing, and it is something somebody wants to get added, so
   while I can not image the exact use scenario why anybody
   would want to use such a hook I'd apply the change" to
   convince myself.

 - We are deep in feature-freeze for 1.5.3; I'd appreciate a
   resend for any patch that is not a bugfix / documentation
   update after 1.5.3 final.

Re: [PATCH] Add post-merge hook.

[Josh England]

Sorry for late response --- *long* weekend.  :)

On Thu, 2007-08-30 at 16:07 -0700, Junio C Hamano wrote:
> > This adds a post-merge hook that will run after `git pull` operations
> > if enabled.  The hook is passed no arguments and cannot affect the
> > outcome of a merge.
> >
> > Signed-off-by: Josh England <jjengla@sandia.gov>
> 
> Thanks for your patch.
> 
> Two questions.
> 
>  * Do you want to run the post-merge hook even for a squash
>    merge?

Yes.  I'd like to run it at any time that the working tree might be
updated.

>  * After resolving a conflicted merge, you would conclude it
>    with "git commit"; don't you want to have the same hook apply
>    at the commit time, or is that what you expect the user to
>    deal with with post-commit hook?

I wouldn't want the post-merge hook running there because (in my case)
it needs to do something slightly different than the post-commit hook,
and would seem counter-intuitive to me. The pre-commit and post-commit
hooks are already there and handle that scenario nicely.

> And two requests and a half.
> 
>  - We would want a new test in the test suite for this, to make
>    sure that later changes by others would not break this new
>    feature you would depend upon.

Can do.  You want me to resubmit the original patch along with tests or
submit the tests as a new patch?

>  - You described _what_ the patch and the new feature do in the
>    log message and in the documentation.
> 
>    You need to also explain _why_ somebody might want to have
>    such a hook in his repository.
> 
>    . In the documentation, you would want to make the reader
>      realize "aha, this is something that would help me" with an
>      example.
> 
>    . In the log message, you would want to make sure other
>      people understand why this new feature was added, what the
>      design consideration were at the point of the feature was
>      added.

I may have to come up with a use-case that is more mainstream than mine.
I'm personally using it to update permissions/ownership in the working
tree based on a git-controlled file (created by pre-commit).  I'll post
the script when its fully fleshed out.  Should I just put "If you're
doing something crazy like me, this hook may be useful."  :)

>  - We are deep in feature-freeze for 1.5.3; I'd appreciate a
>    resend for any patch that is not a bugfix / documentation
>    update after 1.5.3 final.

Sure thing.  It looks like the 1.5.4 cycled just started.  I'll resend
with the additions you requested.

-JE

Re: [PATCH] Add post-merge hook.

[Junio C Hamano]

"Josh England" <jjengla@sandia.gov> writes:

>> Two questions.
>> 
>>  * Do you want to run the post-merge hook even for a squash
>>    merge?
>
> Yes.  I'd like to run it at any time that the working tree might be
> updated.

If that is the case, perhaps your hook may want to get a
parameter to tell it what kind of "git-merge" invocation it was?
Squash merge does not even advance the HEAD and is of a very
different nature from a normal merge.

>>  - We would want a new test in the test suite for this, to make
>>    sure that later changes by others would not break this new
>>    feature you would depend upon.
>
> Can do.  You want me to resubmit the original patch along with tests or
> submit the tests as a new patch?

I'd like a full resend whenever I reject a patch with a
comment.  That way the patch will be easier to review with
context by other people.

Thanks.

Re: [PATCH] Add post-merge hook.

[Josh England]

On Tue, 2007-09-04 at 10:25 -0700, Junio C Hamano wrote:
> "Josh England" <jjengla@sandia.gov> writes:
> 
> >> Two questions.
> >> 
> >>  * Do you want to run the post-merge hook even for a squash
> >>    merge?
> >
> > Yes.  I'd like to run it at any time that the working tree might be
> > updated.
> 
> If that is the case, perhaps your hook may want to get a
> parameter to tell it what kind of "git-merge" invocation it was?
> Squash merge does not even advance the HEAD and is of a very
> different nature from a normal merge.

OK.  Should it just pass in a flag (squash or normal), or are there
other merge types it should need to know about.

-JE

Re: [PATCH] Add post-merge hook.

[Junio C Hamano]

"Josh England" <jjengla@sandia.gov> writes:

> On Tue, 2007-09-04 at 10:25 -0700, Junio C Hamano wrote:
>> "Josh England" <jjengla@sandia.gov> writes:
>> 
>> >> Two questions.
>> >> 
>> >>  * Do you want to run the post-merge hook even for a squash
>> >>    merge?
>> >
>> > Yes.  I'd like to run it at any time that the working tree might be
>> > updated.
>> 
>> If that is the case, perhaps your hook may want to get a
>> parameter to tell it what kind of "git-merge" invocation it was?
>> Squash merge does not even advance the HEAD and is of a very
>> different nature from a normal merge.
>
> OK.  Should it just pass in a flag (squash or normal), or are there
> other merge types it should need to know about.

I suspect you have thought abuot the issues involved longer than
I have ;-), so you should take whatever I say with grain of
salt, but I think you would also want to know fast-forwards and
up-to-dates if the hook wants to be generic, not "for Josh's
workflow only".

Re: [PATCH] Add post-merge hook.

[Josh England]

On Tue, 2007-09-04 at 13:03 -0700, Junio C Hamano wrote:
> >> >>  * Do you want to run the post-merge hook even for a squash
> >> >>    merge?
> >> >
> >> > Yes.  I'd like to run it at any time that the working tree might be
> >> > updated.
> >> 
> >> If that is the case, perhaps your hook may want to get a
> >> parameter to tell it what kind of "git-merge" invocation it was?
> >> Squash merge does not even advance the HEAD and is of a very
> >> different nature from a normal merge.
> >
> > OK.  Should it just pass in a flag (squash or normal), or are there
> > other merge types it should need to know about.
> 
> I suspect you have thought abuot the issues involved longer than
> I have ;-), so you should take whatever I say with grain of
> salt, but I think you would also want to know fast-forwards and
> up-to-dates if the hook wants to be generic, not "for Josh's
> workflow only".

Generic is great, I'm just trying to figure out when/why someone would
need to know the exact type of merge operation used.  The hook should be
generic, yet not require an end user to know any git internals not
explicitly mentioned in the git-merge man page.  I'm thinking that it
will be sufficient to pass a flag indicating whether the working tree
has been modified or not.  The flag can be set for normal merge and
fast-forward merges, and unset for up-to-dates and squash merges.  I
don't really know git internals myself.  Am I missing anything?

-JE

Re: [PATCH] Add post-merge hook.

[Steven Grimm]

Josh England wrote:
> I'm thinking that it
> will be sufficient to pass a flag indicating whether the working tree
> has been modified or not.  The flag can be set for normal merge and
> fast-forward merges, and unset for up-to-dates and squash merges.
>   

Squash merges modify the working tree. In fact, that's *all* they do -- 
they don't commit anything.

-Steve

Re: [PATCH] Add post-merge hook.

[Josh England]

On Tue, 2007-09-04 at 13:52 -0700, Steven Grimm wrote:
> Josh England wrote:
> > I'm thinking that it
> > will be sufficient to pass a flag indicating whether the working tree
> > has been modified or not.  The flag can be set for normal merge and
> > fast-forward merges, and unset for up-to-dates and squash merges.
> >   
> Squash merges modify the working tree. In fact, that's *all* they do -- 
> they don't commit anything.

OK.  Looking at it closer, the post-merge hook should only run when
there has been a real merge (not run for up-to-dates), which seems
alright with me.  With the immediate data at hand I could pass in a flag
indicating a squash merge or not, or could simply pass in the current
HEAD.  I think the first scenario is more appropriate.

-JE