[PATCH] Fix contrib/hooks/post-receive-email for new branch with no new commits

[PATCH] Fix contrib/hooks/post-receive-email for new branch with no new commits

[Pat Notz]

In the show_new_revisions function, the original code:

   git rev-parse --not --branches | grep -v $(git rev-parse $refname) |

isn't quite right since one can create a new branch and push it without
any new commits.  In that case, two refs will have the same sha1 but
both would get filtered by the 'grep'.  In the end, we'll show ALL the
history which is not what we want.  Instead, we should list the branches
by name and remove the branch being updated and THEN pass that list
through rev-parse.

Signed-off-by: Pat Notz <pknotz@sandia.gov>
---
 contrib/hooks/post-receive-email |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 28a3c0e..116f89c 100644
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -615,7 +615,9 @@ show_new_revisions()
 		revspec=$oldrev..$newrev
 	fi
 
-	git rev-parse --not --branches | grep -v $(git rev-parse $refname) |
+	this_branch=$(echo $refname | sed 's@refs/heads/@@')
+	other_branches=$(git branch | sed 's/\*//g' | grep -v $this_branch)
+	git rev-parse --not $other_branches |
 	if [ -z "$custom_showrev" ]
 	then
 		git rev-list --pretty --stdin $revspec
-- 
1.6.1.2

Re: [PATCH] Fix contrib/hooks/post-receive-email for new branch with no new commits

[Jakub Narebski]

"Pat Notz" <pknotz@sandia.gov> writes:

> In the show_new_revisions function, the original code:
> 
>    git rev-parse --not --branches | grep -v $(git rev-parse $refname) |
> 
> isn't quite right since one can create a new branch and push it without
> any new commits.  In that case, two refs will have the same sha1 but
> both would get filtered by the 'grep'.  In the end, we'll show ALL the
> history which is not what we want.  Instead, we should list the branches
> by name and remove the branch being updated and THEN pass that list
> through rev-parse.

Good idea, bad execution.

> 
> Signed-off-by: Pat Notz <pknotz@sandia.gov>
> ---
>  contrib/hooks/post-receive-email |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
> index 28a3c0e..116f89c 100644
> --- a/contrib/hooks/post-receive-email
> +++ b/contrib/hooks/post-receive-email
> @@ -615,7 +615,9 @@ show_new_revisions()
>  		revspec=$oldrev..$newrev
>  	fi
>  
> -	git rev-parse --not --branches | grep -v $(git rev-parse $refname) |
> +	this_branch=$(echo $refname | sed 's@refs/heads/@@')
> +	other_branches=$(git branch | sed 's/\*//g' | grep -v $this_branch)

git-branch is porcelain, git-branch is porcelain, git-branch is porcelain,
git-branch is porcelain, git-branch is porcelain, git-branch is porcelain,
git-branch is porcelain, git-branch is porcelain, ...

Don't use sed if shell will suffice...

Either:

+	this_branch=$refname
+	other_branches=$(git for-each-ref --format='%(refname)' refs/heads/ |
+               grep -v $this_branch)

or

+	this_branch=${refname#refs/heads/}
...

> +	git rev-parse --not $other_branches |
>  	if [ -z "$custom_showrev" ]
>  	then
>  		git rev-list --pretty --stdin $revspec
> -- 
> 1.6.1.2
> 
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] Fix contrib/hooks/post-receive-email for new branch with no new commits

[Johannes Schindelin]

Hi,

On Tue, 10 Feb 2009, Jakub Narebski wrote:

> "Pat Notz" <pknotz@sandia.gov> writes:
> 
> > In the show_new_revisions function, the original code:
> > 
> >    git rev-parse --not --branches | grep -v $(git rev-parse $refname) |
> > 
> > isn't quite right since one can create a new branch and push it without
> > any new commits.  In that case, two refs will have the same sha1 but
> > both would get filtered by the 'grep'.  In the end, we'll show ALL the
> > history which is not what we want.  Instead, we should list the branches
> > by name and remove the branch being updated and THEN pass that list
> > through rev-parse.
> 
> Good idea, bad execution.

And I thought that I hold the patent for grumpy comments on this list :-)

As for your suggestions, I think they are valid.  We try to keep the 
interface of certain commands (so called "plumbing") stable, for script 
consumption.  "git for-each-ref" is such a command.

However, "git branch" is meant for human consumption, and a pretty recent 
patch wants to change the interface to make it even friendlier -- but 
breaking scripts' assumption in the process, should they use "git branch".

Ciao,
Dscho

Re: [PATCH] Fix contrib/hooks/post-receive-email for new branch with no new commits

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> +	this_branch=$refname
> +	other_branches=$(git for-each-ref --format='%(refname)' refs/heads/ |
> +               grep -v $this_branch)

This is still not quite right.  grep -F -v "$this_branch" perhaps?

[PATCH] Fix contrib/hooks/post-receive-email for new duplicate branch

[Pat Notz]

In the show_new_revisions function, the original code:

  git rev-parse --not --branches | grep -v $(git rev-parse $refname) |

isn't quite right since one can create a new branch and push it
without any new commits.  In that case, two refs will have the same
sha1 but both would get filtered by the 'grep'.  In the end, we'll
show ALL the history which is not what we want.  Instead, we should
list the branches by name and remove the branch being updated and THEN
pass that list through rev-parse.

Revised as suggested by Jakub Narebski and Junio C Hamano to use
git-for-each-ref instead of git-branch.  (Thanks!)

Signed-off-by: Pat Notz <pknotz@sandia.gov>
---
 contrib/hooks/post-receive-email |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 28a3c0e..60cbab6 100644
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -615,7 +615,9 @@ show_new_revisions()
 		revspec=$oldrev..$newrev
 	fi
 
-	git rev-parse --not --branches | grep -v $(git rev-parse $refname) |
+	other_branches=$(git for-each-ref --format='%(refname)' refs/heads/ |
+	    grep -F -v $refname)
+	git rev-parse --not $other_branches |
 	if [ -z "$custom_showrev" ]
 	then
 		git rev-list --pretty --stdin $revspec
-- 
1.6.1.2