[PATCH v4 0/2] git-multimail: a replacement for post-receive-email

[PATCH v4 0/2] git-multimail: a replacement for post-receive-email

[Michael Haggerty]

This is the fourth iteration submission of git-multimail to Git.  The
earlier submissions have all gotten a lot of good feedback, which has
mostly been implemented.  This submission differs from v3 in the
following ways:

* Renames the directory within the Git project from
  contrib/hooks/git-multimail/ to contrib/hooks/multimail/.  The
  "git-" seems redundant within the Git project [1].  I have no strong
  feelings either way.

* Includes the latest version of the upstream project.  Highlights:

  * A change in how git-multimail is configured via Python.  I wasn't
    happy with the old method; concerns were not separated well
    enough.  I wanted to get this right before more people start
    writing against the internal API.  The new version uses mixin
    classes, which is a technique that can easily be overdone.  But in
    this case I was happy with the way that it permitted different
    aspects of the configuration to be disentangled quite well.  (This
    doesn't change how the script is configured externally via "git
    config"; that has been stable for quite some time already.)

  * Fixes a scalability issue for repos with lots of refs that was
    pointed out by Ævar Bjarmason.

  * Allows an arbitrary program to be substituted in place of
    /usr/sbin/sendmail when using the SendMailer.

  * Improvements suggested by Ramkumar Ramachandra's code review
    (thanks!).

  * Various documentation improvements.

* Adds a new file, README.Git, which explains the relationship between
  the Git and git-multimail projects, and documents the version of the
  upstream project that corresponds to the code being submitted to the
  Git project.

* Adds a notice to contrib/hooks/multimail/post-receive deprecating
  that script and pointing users to git-multimail.

The upstream project also now includes better tests.  Though I am not
including the tests in the code submitted to the Git project,
obviously the code benefits from them.

[1] I also got the feeling that Junio prefers the new directory name,
    though there is a good chance that I read more into one of his
    emails than he intended.

Michael Haggerty (2):
  git-multimail: an improved replacement for post-receive-email
  post-receive-email: deprecate script in favor of git-multimail

 contrib/hooks/multimail/README                     |  486 ++++
 contrib/hooks/multimail/README.Git                 |   15 +
 .../README.migrate-from-post-receive-email         |  146 ++
 contrib/hooks/multimail/git_multimail.py           | 2394 ++++++++++++++++++++
 contrib/hooks/multimail/migrate-mailhook-config    |  270 +++
 contrib/hooks/multimail/post-receive               |   90 +
 contrib/hooks/post-receive-email                   |   17 +-
 7 files changed, 3414 insertions(+), 4 deletions(-)
 create mode 100644 contrib/hooks/multimail/README
 create mode 100644 contrib/hooks/multimail/README.Git
 create mode 100644 contrib/hooks/multimail/README.migrate-from-post-receive-email
 create mode 100755 contrib/hooks/multimail/git_multimail.py
 create mode 100755 contrib/hooks/multimail/migrate-mailhook-config
 create mode 100755 contrib/hooks/multimail/post-receive

-- 
1.8.3.2

[PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

[Michael Haggerty]

Add a notice to the top of post-receive-email explaining that the
script is no longer under active development and pointing the user to
git-multimail.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
If you think this is premature or the wording is too strong; feel free
to change or omit this patch.  I think it's fine, but then I might be
biased...

 contrib/hooks/post-receive-email | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 0e5b72d..23d7783 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -2,10 +2,19 @@
 #
 # Copyright (c) 2007 Andy Parkins
 #
-# An example hook script to mail out commit update information.  This hook
-# sends emails listing new revisions to the repository introduced by the
-# change being reported.  The rule is that (for branch updates) each commit
-# will appear on one email and one email only.
+# An example hook script to mail out commit update information.
+#
+# ***NOTICE***: This script is no longer under active development.  It
+# has been superseded by git-multimail, which is more capable and
+# configurable and is largely backwards-compatible with this script;
+# please see "contrib/hooks/multimail/".  For instructions on how to
+# migrate from post-receive-email to git-multimail, please see
+# "README.migrate-from-post-receive-email" in that directory.
+#
+# This hook sends emails listing new revisions to the repository
+# introduced by the change being reported.  The rule is that (for
+# branch updates) each commit will appear on one email and one email
+# only.
 #
 # This hook is stored in the contrib/hooks directory.  Your distribution
 # will have put this somewhere standard.  You should make this script
-- 
1.8.3.2

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

[Jonathan Nieder]

Michael Haggerty wrote:

> Add a notice to the top of post-receive-email explaining that the
> script is no longer under active development and pointing the user to
> git-multimail.

I think the spirit of this patch is sane.  Some thoughts on wording:

[...]
> --- a/contrib/hooks/post-receive-email
> +++ b/contrib/hooks/post-receive-email
> @@ -2,10 +2,19 @@
>  #
>  # Copyright (c) 2007 Andy Parkins
>  #
> -# An example hook script to mail out commit update information.  This hook
> -# sends emails listing new revisions to the repository introduced by the
> -# change being reported.  The rule is that (for branch updates) each commit
> -# will appear on one email and one email only.
> +# An example hook script to mail out commit update information.
> +#
> +# ***NOTICE***: This script is no longer under active development.  It
> +# has been superseded by git-multimail, which is more capable and
> +# configurable and is largely backwards-compatible with this script;
> +# please see "contrib/hooks/multimail/".  For instructions on how to
> +# migrate from post-receive-email to git-multimail, please see
> +# "README.migrate-from-post-receive-email" in that directory.

I think I'd say something like

(1)
	# An example hook script to mail out commit update information.
	#
	# This script is kept for compatibility, but it is no longer actively
	# maintained.  Consider switching to git-multimail, which is more
	# configurable and largely compatible with this script.  See
	# contrib/hooks/multimail/README.migrate-from-post-receive.
	#
	# This hook sends emails listing new revisions ...

or, if I wanted to emphasize the warning,

(2)
	# An example hook ...
	#
	# Warning: this script is kept for compatibility, but it is no longer
	# actively maintained.  Consider switching to ...

or, if I wanted to avoid seeming to promise that the script will be
around in the future,

(3)
	# An example hook ...
	#
	# Warning: this script is no longer actively maintained.  Consider
	# switching to ...

I prefer (2), which makes it clear to the reader that it is dangerous
to keep using the script (since no one is actively chasing down bugs)
while also making it clear why a potentially buggy script with a good
natural successor is still in contrib for now.  What do you think?

Thanks,
Jonathan

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

[Michael Haggerty]

On 07/15/2013 08:02 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> Add a notice to the top of post-receive-email explaining that the
>> script is no longer under active development and pointing the user to
>> git-multimail.
> 
> I think the spirit of this patch is sane.  Some thoughts on wording:
> 
> [...]
>> --- a/contrib/hooks/post-receive-email
>> +++ b/contrib/hooks/post-receive-email
>> @@ -2,10 +2,19 @@
>>  #
>>  # Copyright (c) 2007 Andy Parkins
>>  #
>> -# An example hook script to mail out commit update information.  This hook
>> -# sends emails listing new revisions to the repository introduced by the
>> -# change being reported.  The rule is that (for branch updates) each commit
>> -# will appear on one email and one email only.
>> +# An example hook script to mail out commit update information.
>> +#
>> +# ***NOTICE***: This script is no longer under active development.  It
>> +# has been superseded by git-multimail, which is more capable and
>> +# configurable and is largely backwards-compatible with this script;
>> +# please see "contrib/hooks/multimail/".  For instructions on how to
>> +# migrate from post-receive-email to git-multimail, please see
>> +# "README.migrate-from-post-receive-email" in that directory.
> 
> I think I'd say something like
> 
> (1)
> 	# An example hook script to mail out commit update information.
> 	#
> 	# This script is kept for compatibility, but it is no longer actively
> 	# maintained.  Consider switching to git-multimail, which is more
> 	# configurable and largely compatible with this script.  See
> 	# contrib/hooks/multimail/README.migrate-from-post-receive.
> 	#
> 	# This hook sends emails listing new revisions ...
> 
> or, if I wanted to emphasize the warning,
> 
> (2)
> 	# An example hook ...
> 	#
> 	# Warning: this script is kept for compatibility, but it is no longer
> 	# actively maintained.  Consider switching to ...
> 
> or, if I wanted to avoid seeming to promise that the script will be
> around in the future,
> 
> (3)
> 	# An example hook ...
> 	#
> 	# Warning: this script is no longer actively maintained.  Consider
> 	# switching to ...
> 
> I prefer (2), which makes it clear to the reader that it is dangerous
> to keep using the script (since no one is actively chasing down bugs)
> while also making it clear why a potentially buggy script with a good
> natural successor is still in contrib for now.  What do you think?

Honestly, as the main author of git-multimail, I am biased and think it
would be better for other people to make the decision about if/how/when
to annotate post-receive-email.  Any of your suggestions are fine with me.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

[Matthieu Moy]

Jonathan Nieder <jrnieder@gmail.com> writes:

> (3)
> 	# An example hook ...
> 	#
> 	# Warning: this script is no longer actively maintained.  Consider
> 	# switching to ...
>
> I prefer (2), which makes it clear to the reader that it is dangerous
> to keep using the script (since no one is actively chasing down bugs)
> while also making it clear why a potentially buggy script with a good
> natural successor is still in contrib for now.  What do you think?

I don't think it is dangerous to keep using the old script. If you look
at its history, it's pretty stable these day. I think it has known bugs
in new revision detections that are fixed by git-multimail, but nothing
really blocking IMHO.

There are two good reasons to use it: 1) you already use it, and you're
too lazy to change (e.g. because it's packaged by Debian and is already
there on your server), and 2) you don't have Python on your server.

I think the notice still deserve the "***NOTICE***" or whatever makes it
visible enough to distinguish it from the traditional licence &
non-warranty header, but I don't think we should kill the old script too
early.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> (3)
>> 	# An example hook ...
>> 	#
>> 	# Warning: this script is no longer actively maintained.  Consider
>> 	# switching to ...
>>
>> I prefer (2), which makes it clear to the reader that it is dangerous
>> to keep using the script (since no one is actively chasing down bugs)
>> while also making it clear why a potentially buggy script with a good
>> natural successor is still in contrib for now.  What do you think?
>
> I don't think it is dangerous to keep using the old script. If you look
> at its history, it's pretty stable these day. I think it has known bugs
> in new revision detections that are fixed by git-multimail, but nothing
> really blocking IMHO.
>
> There are two good reasons to use it: 1) you already use it, and you're
> too lazy to change (e.g. because it's packaged by Debian and is already
> there on your server), and 2) you don't have Python on your server.

Well said.

> I think the notice still deserve the "***NOTICE***" or whatever makes it
> visible enough to distinguish it from the traditional licence &
> non-warranty header, but I don't think we should kill the old script too
> early.

True.  I personally felt that Jonathan's (1) read the most natural
(i.e. showing no strong preference, just let the users decide).

Re: [PATCH v4 2/2] post-receive-email: deprecate script in favor of git-multimail

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> I think the notice still deserve the "***NOTICE***" or whatever makes it
>> visible enough to distinguish it from the traditional licence &
>> non-warranty header, but I don't think we should kill the old script too
>> early.
>
> True.  I personally felt that Jonathan's (1) read the most natural
> (i.e. showing no strong preference, just let the users decide).

We need a decider to push this forward, so let's squash this in to
tone it down and then merge the result to 'next' (and then 'master'
by -rc1).

 contrib/hooks/post-receive-email | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 23d7783..1531150 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -4,8 +4,8 @@
 #
 # An example hook script to mail out commit update information.
 #
-# ***NOTICE***: This script is no longer under active development.  It
-# has been superseded by git-multimail, which is more capable and
+# NOTE: This script is no longer under active development.  There
+# is another script, git-multimail, which is more capable and
 # configurable and is largely backwards-compatible with this script;
 # please see "contrib/hooks/multimail/".  For instructions on how to
 # migrate from post-receive-email to git-multimail, please see

Re: [PATCH v4 1/2] git-multimail: an improved replacement for post-receive-email

[Jonathan Nieder]

Michael Haggerty wrote:

> Add git-multimail, a tool for generating notification emails for
> pushes to a Git repository.  It is largely plug-in compatible with
> post-receive-email, and is proposed to eventually replace that script.
> The advantages of git-multimail relative to post-receive-email are
> described in README.migrate-from-post-receive-email.

Yay!

>  contrib/hooks/multimail/README                     |  486 ++++
>  contrib/hooks/multimail/README.Git                 |   15 +
>  .../README.migrate-from-post-receive-email         |  146 ++
>  contrib/hooks/multimail/git_multimail.py           | 2394 ++++++++++++++++++++
>  contrib/hooks/multimail/migrate-mailhook-config    |  270 +++
>  contrib/hooks/multimail/post-receive               |   90 +
>  6 files changed, 3401 insertions(+)
>  create mode 100644 contrib/hooks/multimail/README
>  create mode 100644 contrib/hooks/multimail/README.Git
>  create mode 100644 contrib/hooks/multimail/README.migrate-from-post-receive-email
>  create mode 100755 contrib/hooks/multimail/git_multimail.py
>  create mode 100755 contrib/hooks/multimail/migrate-mailhook-config
>  create mode 100755 contrib/hooks/multimail/post-receive

I wouldn't be surprised if it is possible to improve this in some
way or other, but where as usual "Reviewed-by" means "If I were
maintainer I would commit and push it out and as a non maintainer
I vouch for its suitability",

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

The patch integrates it into the git tree sanely and further
refinements can safely come on top.  Thanks for your hard work.