llvm dev - Jan 2013 - [LLVMdev] [cfe-dev] RFC: put commit messages in *-commits subject lines?

Hi Daniel,

Daniel Dunbar wrote:
> Hi Sebastion,
> 
> I've attached the current configuration file from the server.
> 
> I'm not sure how far you want to go down the "trying to get
realize nice
> summary path" lines, but if svn_mailer somehow supported running an
> external script to process the commit and come up with the path that would
> be ideal for integration on the server.
I think you could modify the sources of svn_mailer to add such a hook.  When I
first investigated this issue, I ended on a bug report from the debian project
that fixed the subject line with a one line patch:
> To get the first line of the log message as a subject line, please apply
this
> one line patch:
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=17;bug=379534
> Note that you can directly patch the installed script: on my machine it is
> located in
/usr/local/lib/python2.7/dist-packages/svnmailer/notifier/_mail.py
For your convenience, here is the patch from that debian bug report:


svnmailer: add log substitution for subject templates

Add first line of commit log as a substitution variable for
*_subject_template, E.G. add %(log)s to use.

Signed-off-by: Peter Korsgaard <jacmet at sunsite.dk>

Index: svnmailer-1.0.8/src/lib/svnmailer/notifier/_mail.py
==================================================================---
svnmailer-1.0.8.orig/src/lib/svnmailer/notifier/_mail.py	2008-05-27
14:56:27.000000000 +0200
+++ svnmailer-1.0.8/src/lib/svnmailer/notifier/_mail.py	2008-05-27
15:01:50.000000000 +0200
@@ -314,6 +314,7 @@
             'part'    : countprefix,
             'files'   : self._getPrefixedFiles(changeset),
             'dirs'    : self._getPrefixedDirectories(changeset),
+            'log'     : self.getLog().split('\n',1)[0],
         }
 
         # We may try twice, first with files/dirs = files



Sebastian
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation

Sebastian Pop wrote:
> Hi Daniel,
> 
> Daniel Dunbar wrote:
> > Hi Sebastion,
> > 
> > I've attached the current configuration file from the server.
> > 
> > I'm not sure how far you want to go down the "trying to get
realize nice
> > summary path" lines, but if svn_mailer somehow supported running
an
> > external script to process the commit and come up with the path that
would
> > be ideal for integration on the server.
> 
> I think you could modify the sources of svn_mailer to add such a hook. 
When I
> first investigated this issue, I ended on a bug report from the debian
project
> that fixed the subject line with a one line patch:
> 
> > To get the first line of the log message as a subject line, please
apply this
> > one line patch:
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=17;bug=379534
> > Note that you can directly patch the installed script: on my machine
it is
> > located in
/usr/local/lib/python2.7/dist-packages/svnmailer/notifier/_mail.py
> 
> For your convenience, here is the patch from that debian bug report:
> 
> 
> svnmailer: add log substitution for subject templates
> 
> Add first line of commit log as a substitution variable for
> *_subject_template, E.G. add %(log)s to use.
> 
> Signed-off-by: Peter Korsgaard <jacmet at sunsite.dk>
> 
> Index: svnmailer-1.0.8/src/lib/svnmailer/notifier/_mail.py
> ==================================================================> ---
svnmailer-1.0.8.orig/src/lib/svnmailer/notifier/_mail.py	2008-05-27
14:56:27.000000000 +0200
> +++ svnmailer-1.0.8/src/lib/svnmailer/notifier/_mail.py	2008-05-27
15:01:50.000000000 +0200
> @@ -314,6 +314,7 @@
>              'part'    : countprefix,
>              'files'   : self._getPrefixedFiles(changeset),
>              'dirs'    : self._getPrefixedDirectories(changeset),
> +            'log'     : self.getLog().split('\n',1)[0],
>          }
>  
>          # We may try twice, first with files/dirs = files
> 
> 
Once you patched the installed version of this file, in
/usr/local/lib/python2.7/dist-packages/svnmailer/notifier/_mail.py
you have two more changes in the config file you sent me: (the first change to
print the context of diff is something that I was missing more than the subject)

--- svn-mailer.conf.orig        2013-01-25 15:39:23.000000000 -0600
+++ svn-mailer.conf     2013-01-25 15:42:02.000000000 -0600
@@ -1,6 +1,6 @@
 [general]
 sendmail_command = /usr/sbin/sendmail
-diff_command = /usr/bin/diff -u -L %(label_from)s -L %(label_to)s %(from)s
%(to)s
+diff_command = /usr/bin/diff -up -L %(label_from)s -L %(label_to)s %(from)s
%(to)s
 generate_diffs = add copy modify propchange
 
 [authors]
@@ -360,6 +360,7 @@
 mail_type = single
 show_applied_charset = nondefault
 custom_header = SVN-Repository %(REPOS)s
+commit_subject_template = %(prefix)s r%(revision)s - %(log)s
 
 [branch]
 for_repos = .*/llvm-project$



Note that I have not addressed the path prefix suggestion from David:
>>> For what it's worth, (I think) Chris' suggestion of
including the
>>> directories was about including them "smart"ly by
removing conceptual
>>> duplicates (lib/foo + include/foo + test/foo) and generally giving
a
>>> brief sense of what a change is touching. If the change you have
adds
>>> the full (repo-relative) path all the directories without any smart
>>> deduplication then I suspect it's going to easily push the log
>>> description off most mail readers (especially mobile) & reduce
the
>>> value of this change.
>>>
>>> I suspect we'll want to just stick with revision + log for now.
>>
>> That's fine.  Alternatively we can provide a list of all the dirs
that we want
>> to see appearing and match them like this:
>>
>> for_paths =
.*(?P<SelectDirs>(lib/Analysis|lib/Transforms/Vectorize|lib/Transforms/Scalar)).*
>>
>> commit_subject_template = %(prefix)s r%(revision)s - [%(SelectDirs)s]
%(log)s
>
>Could we go one step further and map these paths to names (eg: map
>"lib/Analysis", "include/llvm/Analysis",
"test/Analysis" -> "Analysis"
>and unique the results)? Also, I'd prefer to have the dirs after the
>log message I think, though perhaps if we get them short enough it'd
>work.
>
>This solution does seem like it could involve a bit of manual work to
>update the list of blessed dirs all the time. If we could express it
>generically (lib/*, test/*, include/llvm/*) it might be OK, but as it
>stands I'm still not sure it's worth the hassle.
This could be done as Daniel has suggested, running a separate script that
filters and classifies in a smarter way the commits following the paths they are
touching.

Thanks,
Sebastian
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation

On Fri, Jan 25, 2013 at 2:06 PM, Sebastian Pop <spop at codeaurora.org>
wrote:
> Sebastian Pop wrote:
> > Hi Daniel,
> >
> > Daniel Dunbar wrote:
> > > Hi Sebastion,
> > >
> > > I've attached the current configuration file from the server.
> > >
> > > I'm not sure how far you want to go down the "trying to
get realize
> nice
> > > summary path" lines, but if svn_mailer somehow supported
running an
> > > external script to process the commit and come up with the path
that
> would
> > > be ideal for integration on the server.
> >
> > I think you could modify the sources of svn_mailer to add such a hook.
>  When I
> > first investigated this issue, I ended on a bug report from the debian
> project
> > that fixed the subject line with a one line patch:
> >
> > > To get the first line of the log message as a subject line,
please
> apply this
> > > one line patch:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=17;bug=379534
> > > Note that you can directly patch the installed script: on my
machine
> it is
> > > located in
> /usr/local/lib/python2.7/dist-packages/svnmailer/notifier/_mail.py
> >
> > For your convenience, here is the patch from that debian bug report:
> >
> >
> > svnmailer: add log substitution for subject templates
> >
> > Add first line of commit log as a substitution variable for
> > *_subject_template, E.G. add %(log)s to use.
> >
> > Signed-off-by: Peter Korsgaard <jacmet at sunsite.dk>
> >
> > Index: svnmailer-1.0.8/src/lib/svnmailer/notifier/_mail.py
> > ==================================================================>
> --- svnmailer-1.0.8.orig/src/lib/svnmailer/notifier/_mail.py  2008-05-27
> 14:56:27.000000000 +0200
> > +++ svnmailer-1.0.8/src/lib/svnmailer/notifier/_mail.py      
2008-05-27
> 15:01:50.000000000 +0200
> > @@ -314,6 +314,7 @@
> >              'part'    : countprefix,
> >              'files'   : self._getPrefixedFiles(changeset),
> >              'dirs'    :
self._getPrefixedDirectories(changeset),
> > +            'log'     :
self.getLog().split('\n',1)[0],
> >          }
> >
> >          # We may try twice, first with files/dirs = files
> >
> >
>
> Once you patched the installed version of this file, in
> /usr/local/lib/python2.7/dist-packages/svnmailer/notifier/_mail.py
> you have two more changes in the config file you sent me: (the first
> change to
> print the context of diff is something that I was missing more than the
> subject)
>
I'm a bit hesitant to just hack up the installed copy of svnmailer, are we
sure this patch hasn't been dealt with in an upstream version?

>
> --- svn-mailer.conf.orig        2013-01-25 15:39:23.000000000 -0600
> +++ svn-mailer.conf     2013-01-25 15:42:02.000000000 -0600
> @@ -1,6 +1,6 @@
>  [general]
>  sendmail_command = /usr/sbin/sendmail
> -diff_command = /usr/bin/diff -u -L %(label_from)s -L %(label_to)s
> %(from)s %(to)s
> +diff_command = /usr/bin/diff -up -L %(label_from)s -L %(label_to)s
> %(from)s %(to)s
>  generate_diffs = add copy modify propchange
>
I'll go ahead and make this change.

 - Daniel

>
>  [authors]
> @@ -360,6 +360,7 @@
>  mail_type = single
>  show_applied_charset = nondefault
>  custom_header = SVN-Repository %(REPOS)s
> +commit_subject_template = %(prefix)s r%(revision)s - %(log)s
>
>  [branch]
>  for_repos = .*/llvm-project$
>
>
>
> Note that I have not addressed the path prefix suggestion from David:
>
> >>> For what it's worth, (I think) Chris' suggestion of
including the
> >>> directories was about including them "smart"ly by
removing conceptual
> >>> duplicates (lib/foo + include/foo + test/foo) and generally
giving a
> >>> brief sense of what a change is touching. If the change you
have adds
> >>> the full (repo-relative) path all the directories without any
smart
> >>> deduplication then I suspect it's going to easily push the
log
> >>> description off most mail readers (especially mobile) &
reduce the
> >>> value of this change.
> >>>
> >>> I suspect we'll want to just stick with revision + log for
now.
> >>
> >> That's fine.  Alternatively we can provide a list of all the
dirs that
> we want
> >> to see appearing and match them like this:
> >>
> >> for_paths >
.*(?P<SelectDirs>(lib/Analysis|lib/Transforms/Vectorize|lib/Transforms/Scalar)).*
> >>
> >> commit_subject_template = %(prefix)s r%(revision)s -
[%(SelectDirs)s]
> %(log)s
> >
> >Could we go one step further and map these paths to names (eg: map
> >"lib/Analysis", "include/llvm/Analysis",
"test/Analysis" -> "Analysis"
> >and unique the results)? Also, I'd prefer to have the dirs after
the
> >log message I think, though perhaps if we get them short enough
it'd
> >work.
> >
> >This solution does seem like it could involve a bit of manual work to
> >update the list of blessed dirs all the time. If we could express it
> >generically (lib/*, test/*, include/llvm/*) it might be OK, but as it
> >stands I'm still not sure it's worth the hassle.
>
> This could be done as Daniel has suggested, running a separate script that
> filters and classifies in a smarter way the commits following the paths
> they are
> touching.
>
> Thanks,
> Sebastian
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by The Linux Foundation
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130125/d6cf234a/attachment.html>