Re: [PATCH] Corrected return values in post-receive-email.prep_for_email

"Alan Raison" [off-list ref] writes:

---
 contrib/hooks/post-receive-email |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

No sign-off, no description.

This is a regression introduced by 53cad69 (post-receive-email: ensure
sent messages are not empty, 2010-09-10), I think.

This used to "exit 1" before 53cad69 and I agree with the patch that
signalling error with "return 1" is the right thing to do here.

This used to "exit 0" before 53cad69 to cause the program stop before
sending mails.  Again, I agree with the patch that signalling error is the
right thing to do here.

 	fi
 
-	return 1
+	return 0

And this obviously is correct.

Kevin, care to review and Ack?  Alan, care to add a few lines of patch
description and sign-off?

Thanks.

On 12/07/2010 01:34 PM, Junio C Hamano wrote:

"Alan Raison"[off-list ref]  writes:

quoted

---
  contrib/hooks/post-receive-email |    6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

No sign-off, no description.

This is a regression introduced by 53cad69 (post-receive-email: ensure
sent messages are not empty, 2010-09-10), I think.

quoted

diff --git a/contrib/hooks/post-receive-email

b/contrib/hooks/post-receive-email
index 85724bf..020536d 100755

--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email

@@ -150,7 +150,7 @@ prep_for_email()
  			# Anything else (is there anything else?)
  			echo>&2 "*** Unknown type of update to $refname

($rev_type)"
  			echo>&2 "***  - no email generated"
-			return 0
+			return 1

This used to "exit 1" before 53cad69 and I agree with the patch that
signalling error with "return 1" is the right thing to do here.

quoted

  			;;
  	esac

@@ -166,10 +166,10 @@ prep_for_email()
  		esac
  		echo>&2 "*** $config_name is not set so no email will be

sent"
  		echo>&2 "*** for $refname update $oldrev->$newrev"
-		return 0
+		return 1

This used to "exit 0" before 53cad69 to cause the program stop before
sending mails.  Again, I agree with the patch that signalling error is the
right thing to do here.

quoted

  	fi

-	return 1
+	return 0

And this obviously is correct.

Kevin, care to review and Ack?  Alan, care to add a few lines of patch
description and sign-off?

Acked-by: Kevin P. Fleming <redacted>

Yeah, this is clearly my breakage; our internal version of this script 
is so different that it has become hard to backport fixes to the 
upstream version... or I just did a terrible job of it.

Alan, while you are in there fixing this, there is a remaining 'exit 0' 
in prep_for_email (at line 147) that should be 'return 1' instead.

-- 
Kevin P. Fleming
Digium, Inc. | Director of Software Technologies
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
skype: kpfleming | jabber: kfleming@digium.com
Check us out at www.digium.com & www.asterisk.org

From ebe98d1c682f268b39a7eaf3ef529accbf0ac61c Mon Sep 17 00:00:00 2001

From: Alan Raison <redacted>
Date: Mon, 6 Dec 2010 15:49:21 +0000
Subject: [PATCH] Corrected return values in prep_for_email;

Function was returning 0 for failure and 1 for success which was breaking
the logic in the main loop.

Corrected to return 0 for success, 1 for failure.  Function now also returns
in all cases, rather than exiting.
---
 contrib/hooks/post-receive-email |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/hooks/post-receive-email

b/contrib/hooks/post-receive-email
index 85724bf..f99ea95 100755

--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email

@@ -144,13 +144,13 @@ prep_for_email()
 			short_refname=${refname##refs/remotes/}
 			echo >&2 "*** Push-update of tracking branch,

$refname"
 			echo >&2 "***  - no email generated."
-			exit 0
+			return 1
 			;;
 		*)
 			# Anything else (is there anything else?)
 			echo >&2 "*** Unknown type of update to $refname
($rev_type)"
 			echo >&2 "***  - no email generated"
-			return 0
+			return 1
 			;;
 	esac
 

@@ -166,10 +166,10 @@ prep_for_email()
 		esac
 		echo >&2 "*** $config_name is not set so no email will be

sent"
 		echo >&2 "*** for $refname update $oldrev->$newrev"
-		return 0
+		return 1
 	fi
 
-	return 1
+	return 0
 }
 
 #
-- 
1.7.3.1.msysgit.0

On 12/09/2010 07:24 AM, Alan Raison wrote:

 From ebe98d1c682f268b39a7eaf3ef529accbf0ac61c Mon Sep 17 00:00:00 2001
From: Alan Raison<redacted>
Date: Mon, 6 Dec 2010 15:49:21 +0000
Subject: [PATCH] Corrected return values in prep_for_email;

Function was returning 0 for failure and 1 for success which was breaking
the logic in the main loop.

Corrected to return 0 for success, 1 for failure.  Function now also returns
in all cases, rather than exiting.

Your commit message will need a Signed-Off-By line, but...

Acked-By: Kevin P. Fleming <redacted>

-- 
Kevin P. Fleming
Digium, Inc. | Director of Software Technologies
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
skype: kpfleming | jabber: kfleming@digium.com
Check us out at www.digium.com & www.asterisk.org

Function was returning 0 for failure and 1 for success which was breaking
the logic in the main loop.

Corrected to return 0 for success, 1 for failure.  Function now also returns
in all cases, rather than exiting.

Acked-By: Kevin P. Fleming <redacted>
Signed-Off-By: Alan Raison <redacted>
---
 contrib/hooks/post-receive-email |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/hooks/post-receive-email

b/contrib/hooks/post-receive-email
index 85724bf..f99ea95 100755

--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email

@@ -144,13 +144,13 @@ prep_for_email()
 			short_refname=${refname##refs/remotes/}
 			echo >&2 "*** Push-update of tracking branch,

$refname"
 			echo >&2 "***  - no email generated."
-			exit 0
+			return 1
 			;;
 		*)
 			# Anything else (is there anything else?)
 			echo >&2 "*** Unknown type of update to $refname
($rev_type)"
 			echo >&2 "***  - no email generated"
-			return 0
+			return 1
 			;;
 	esac
 

@@ -166,10 +166,10 @@ prep_for_email()
 		esac
 		echo >&2 "*** $config_name is not set so no email will be

sent"
 		echo >&2 "*** for $refname update $oldrev->$newrev"
-		return 0
+		return 1
 	fi
 
-	return 1
+	return 0
 }
 
 #
-- 
1.7.3.1.msysgit.0