Re: [RFC v2] git-multimail: a replacement for post-receive-email

On 02/14/2013 01:55 PM, Matthieu Moy wrote:

Michael Haggerty [off-list ref] writes:

quoted

On 02/13/2013 03:56 PM, Matthieu Moy wrote:

quoted

Installation troubles:

I had an old python installation (Red Hat package, and I'm not root),
that did not include the email.utils package, so I couldn't use my
system's python. I found no indication about python version in README,
so I installed the latest python by hand, just to find out that
git-multimail wasn't compatible with Python 3.x. 2to3 can fix
automatically a number of 3.x compatibility issues, but not all of them
so I gave up and installed Python 2.7.

What version of Python was it that caused problems?

Python 2.4.3, installed with RHEL 5.9.

quoted

I just discovered that the script wouldn't have worked with Python
2.4, where "email.utils" used to be called "email.Utils".

Indeed, "import email.Utils" works with this Python.

quoted

But I pushed a fix to GitHub:

    ddb1796660 Accommodate older versions of Python's email module.

Not sufficient, but I added a pull request that works for me with 2.4.

quoted

quoted

@@ -835,6 +837,17 @@ class ReferenceChange(Change):
                 for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE):
                     yield line
 
+            if adds and self.showlog:
+                yield '\n'
+                yield 'Detailed log of added commits:\n\n'
+                for line in read_lines(
+                        ['git', 'log']
+                        + self.logopts
+                        + ['%s..%s' % (self.old.commit, self.new.commit,)],
+                        keepends=True,
+                        ):
+                    yield line
+
             # The diffstat is shown from the old revision to the new
             # revision.  This is to show the truth of what happened in
             # this change.  There's no point showing the stat from the

Thanks for the patch.  I like the idea, but I think the implementation
is incorrect.  Your code will not only list new commits but will also
list commits that were already in the repository on another branch
(e.g., if an existing feature branch is merged into master, all of the
commits on the feature branch will be listed).  (Or was that your
intention?)

I did not think very carefully about this case, but the behavior of my
code seems sensible (although not uncontroversial): it's just showing
the detailed log for the same commits as the summary at the top of the
email. I have no personnal preferences.

I guess it depends a lot on what logopts are used.  If the user
configures logopts to emit full patches, then the repeated reporting of
the same commits would cause a big increase in the bulk of notification
emails.  But if the logopts are set to just emit a brief summary (e.g.,
author and log message), then a bit of repetition might be acceptable.
But since I wouldn't use this feature, I don't personally have a preference.

quoted

But even worse, it will fail to list commits that were
added at the same time that a branch was created (e.g., if I create a
feature branch with a number of commits on it and then push it for the
first time).

Right.

quoted

Probably the Push object has to negotiate with its constituent
ReferenceChange objects to figure out which one is responsible for
summarizing each of the commits newly added by the push (i.e., the ones
returned by push.get_new_commits(None)).

I updated the pull request with a version that works for new branches,
and takes the list of commits to display from the call to
get_new_commits (which were already there for other purpose). Then, it
essentially calls "git log --no-walk $list_of_sha1s".

This should be better.

I will check it out.

Thanks!

Michael

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