threads / patch / 9712

patchAdd post-merge hook.

Subject: [PATCH] Add post-merge hook.

## tl;dr

9 messages between Aug 30, 2007 and Sep 4, 2007. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

jjengla@sandia.gov· Aug 30, 2007, 22:40 UTC · lore
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(-)
Show changes to 2 files +14 −0

Documentation/hooks.txt, git-merge.sh

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
Junio C Hamano· Aug 30, 2007, 23:07 UTC · re: jjengla@sandia.gov · lore

Re: [PATCH] Add post-merge hook.

jjengla@sandia.gov writes:
Show 7 quoted lines
> 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.
Josh England· Sep 4, 2007, 16:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add post-merge hook.

Sorry for late response --- *long* weekend.  :)
On Thu, 2007-08-30 at 16:07 -0700, Junio C Hamano wrote:
Show 12 quoted lines
> > 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.

Show 5 quoted lines
> 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?

Show 14 quoted lines
>  - 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
Junio C Hamano· Sep 4, 2007, 17:25 UTC · re: Josh England · lore

Re: [PATCH] Add post-merge hook.

"Josh England" <jjengla@sandia.gov> writes:
Show 7 quoted lines
>> 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.

Show 6 quoted lines
>>  - 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.
Josh England· Sep 4, 2007, 19:36 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add post-merge hook.

On Tue, 2007-09-04 at 10:25 -0700, Junio C Hamano wrote:
Show 14 quoted lines
> "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
Junio C Hamano· Sep 4, 2007, 20:03 UTC · re: Josh England · lore

Re: [PATCH] Add post-merge hook.

"Josh England" <jjengla@sandia.gov> writes:
Show 18 quoted lines
> 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".

Josh England· Sep 4, 2007, 20:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add post-merge hook.

On Tue, 2007-09-04 at 13:03 -0700, Junio C Hamano wrote:
Show 19 quoted lines
> >> >>  * 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
Steven Grimm· Sep 4, 2007, 20:52 UTC · re: Josh England · lore

Re: [PATCH] Add post-merge hook.

Josh England wrote:
Show 5 quoted lines
> 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
Josh England· Sep 4, 2007, 21:23 UTC · re: Steven Grimm · lore

Re: [PATCH] Add post-merge hook.

On Tue, 2007-09-04 at 13:52 -0700, Steven Grimm wrote:
Show 8 quoted lines
> 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

← back to recent threads