patman: Make dry-run output match real functionality

PATCH New Peter Tyser 2 participants 5 messages Jan 26, 2015 – Feb 15, 2015

Share & tools

Peter Tyser

author

2015-01-26 17:42 UTC #0 lore raw

PATCH 1/1 patman: Make dry-run output match real functionality

When run with the --dry-run argument patman prints out information showing what it would do. This information currently doesn't line up with what patman/git send-email really do. Some basic examples:
- If an email address is addressed via "Series-cc" and "Patch-cc" patman shows that email address would be CC-ed two times.
- If an email address is addressed via "Series-to" and "Patch-cc" patman shows that email address would be sent TO and CC-ed.
- If an email address is addressed from a combination of tag aliases, get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows that the email address would be CC-ed multiple times.

Patman currently does try to send duplicate emails like the --dry-run output shows, but "git send-email" intelligently removes duplicate addresses so this patch shouldn't change the non-dry-run functionality.

Change patman's output and email addressing to line up with the "git send-email" logic. This trims down patman's dry-run output and prevents confusion about what patman will do when emails are actually sent.

Signed-off-by Peter Tyser <[email protected]>

---

 tools/patman/gitutil.py |  3 ++-
 tools/patman/series.py  | 21 ++++++++-------------
 2 files changed, 10 insertions(+), 14 deletions(-)

tools/patman/gitutil.py +2 -1

index cc5a55a..c593070 100644
@@ -392,7 +392,8 @@ def EmailPatches(series, cover_fname, args, dry_run, raise_on_error, cc_fname,
                    "Or do something like this\n"
                    "git config sendemail.to u-boot at lists.denx.de")
             return
-    cc = BuildEmailList(series.get('cc'), '--cc', alias, raise_on_error)
+    cc = BuildEmailList(list(set(series.get('cc')) - set(series.get('to'))),
+                        '--cc', alias, raise_on_error)
     if self_only:
         to = BuildEmailList([os.getenv('USER')], '--to', alias, raise_on_error)
         cc = []

tools/patman/series.py +8 ~~-13~~

index b67f870..60ebc76 100644
@@ -94,6 +94,9 @@ class Series(dict):
             cmd: The git command we would have run
             process_tags: Process tags as if they were aliases
         """
+        to_set = set(gitutil.BuildEmailList(self.to));
+        cc_set = set(gitutil.BuildEmailList(self.cc));
+
         col = terminal.Color()
         print 'Dry run, so not doing much. But I would do this:'
         print

@@ -106,24 +109,16 @@ class Series(dict):
             commit = self.commits[upto]
             print col.Color(col.GREEN, '   %s' % args[upto])
             cc_list = list(self._generated_cc[commit.patch])
-
-            # Skip items in To list
-            if 'to' in self:
-                try:
-                    map(cc_list.remove, gitutil.BuildEmailList(self.to))
-                except ValueError:
-                    pass
-
-            for email in cc_list:
+            for email in set(cc_list) - to_set - cc_set:
                 if email == None:
                     email = col.Color(col.YELLOW, "<alias '%s' not found>"
                             % tag)
                 if email:
                     print '      Cc: ',email
         print
-        for item in gitutil.BuildEmailList(self.get('to', '<none>')):
+        for item in to_set:
             print 'To:\t ', item
-        for item in gitutil.BuildEmailList(self.cc):
+        for item in cc_set - to_set:
             print 'Cc:\t ', item
         print 'Version: ', self.get('version')
         print 'Prefix:\t ', self.get('prefix')

@@ -131,7 +126,7 @@ class Series(dict):
             print 'Cover: %d lines' % len(self.cover)
             cover_cc = gitutil.BuildEmailList(self.get('cover_cc', ''))
             all_ccs = itertools.chain(cover_cc, *self._generated_cc.values())
-            for email in set(all_ccs):
+            for email in set(all_ccs) - to_set - cc_set:
                     print '      Cc: ',email
         if cmd:
             print 'Git command: %s' % cmd

@@ -230,7 +225,7 @@ class Series(dict):
 	    if add_maintainers:
                 list += get_maintainer.GetMaintainer(commit.patch)
             all_ccs += list
-            print >>fd, commit.patch, ', '.join(list)
+            print >>fd, commit.patch, ', '.join(set(list))
             self._generated_cc[commit.patch] = list
 
         if cover_fname:

Signature · 1 line

1.9.1

Simon Glass

2015-01-27 05:21 UTC #1 lore raw

PATCH 1/1 patman: Make dry-run output match real functionality

Hi Peter,

Quoted 11 lines · Peter Tyser

When run with the --dry-run argument patman prints out information showing what it would do. This information currently doesn't line up with what patman/git send-email really do. Some basic examples:
- If an email address is addressed via "Series-cc" and "Patch-cc" patman shows that email address would be CC-ed two times.
- If an email address is addressed via "Series-to" and "Patch-cc" patman shows that email address would be sent TO and CC-ed.
- If an email address is addressed from a combination of tag aliases, get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows that the email address would be CC-ed multiple times.

Quoted 8 lines

Patman currently does try to send duplicate emails like the --dry-run output shows, but "git send-email" intelligently removes duplicate addresses so this patch shouldn't change the non-dry-run functionality.
Change patman's output and email addressing to line up with the "git send-email" logic. This trims down patman's dry-run output and prevents confusion about what patman will do when emails are actually sent.

Thanks for the patch, it's good to match up with git send-email.

Are the rules that git send-email follows documented or obtained by trial and error?

Regards,
Simon

Peter Tyser

author

2015-01-27 15:40 UTC #2 lore raw

PATCH 1/1 patman: Make dry-run output match real functionality

Quoted 23 lines · Simon Glass

Hi Peter,
Quoted 11 lines · Peter Tyser
When run with the --dry-run argument patman prints out information showing what it would do. This information currently doesn't line up with what patman/git send-email really do. Some basic examples:
- If an email address is addressed via "Series-cc" and "Patch-cc" patman shows that email address would be CC-ed two times.
- If an email address is addressed via "Series-to" and "Patch-cc" patman shows that email address would be sent TO and CC-ed.
- If an email address is addressed from a combination of tag aliases, get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows that the email address would be CC-ed multiple times.
Quoted 8 lines
Patman currently does try to send duplicate emails like the --dry-run output shows, but "git send-email" intelligently removes duplicate addresses so this patch shouldn't change the non-dry-run functionality.
Change patman's output and email addressing to line up with the "git send-email" logic. This trims down patman's dry-run output and prevents confusion about what patman will do when emails are actually sent.

Quoted 4 lines

Thanks for the patch, it's good to match up with git send-email.
Are the rules that git send-email follows documented or obtained by trial and error?

Trial and error initially. The git source code lined up with what I saw (see the send_message function in git-send-email.perl). I didn't see the policy documented officially, but what git does makes sense to me:
- remove any duplicate addresses in the to: field
- remove all the to: addresses from the cc: addresses
- remove any duplicate cc: addresses

This makes sure each email is only sent to an address one time, with the to: field taking precedence over the cc: field.

For a recent NAND patch series it looked like I was going to send Scott 2-4 emails per patch which is why I looked into it.

Regards,
Peter

Simon Glass

2015-01-28 05:08 UTC #3 lore raw

PATCH 1/1 patman: Make dry-run output match real functionality

Quoted 38 lines · Peter Tyser

Quoted 23 lines · Simon Glass
Hi Peter,
Quoted 11 lines · Peter Tyser
When run with the --dry-run argument patman prints out information showing what it would do. This information currently doesn't line up with what patman/git send-email really do. Some basic examples:
- If an email address is addressed via "Series-cc" and "Patch-cc" patman shows that email address would be CC-ed two times.
- If an email address is addressed via "Series-to" and "Patch-cc" patman shows that email address would be sent TO and CC-ed.
- If an email address is addressed from a combination of tag aliases, get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows that the email address would be CC-ed multiple times.
Quoted 8 lines
Patman currently does try to send duplicate emails like the --dry-run output shows, but "git send-email" intelligently removes duplicate addresses so this patch shouldn't change the non-dry-run functionality.
Change patman's output and email addressing to line up with the "git send-email" logic. This trims down patman's dry-run output and prevents confusion about what patman will do when emails are actually sent.
Quoted 4 lines
Thanks for the patch, it's good to match up with git send-email.
Are the rules that git send-email follows documented or obtained by trial and error?
Trial and error initially. The git source code lined up with what I saw (see the send_message function in git-send-email.perl). I didn't see the policy documented officially, but what git does makes sense to me:
- remove any duplicate addresses in the to: field
- remove all the to: addresses from the cc: addresses
- remove any duplicate cc: addresses

Quoted 6 lines

This makes sure each email is only sent to an address one time, with the to: field taking precedence over the cc: field.
For a recent NAND patch series it looked like I was going to send Scott 2-4 emails per patch which is why I looked into it.

Thanks.

Acked-by Simon Glass <[email protected]>

Tested-by Simon Glass <[email protected]>

Simon Glass

Applied

2015-02-15 21:35 UTC #4 lore raw

PATCH 1/1 patman: Make dry-run output match real functionality

Quoted 43 lines · Simon Glass

Quoted 42 lines · Peter Tyser
Quoted 23 lines · Simon Glass
Hi Peter,
Quoted 11 lines · Peter Tyser
When run with the --dry-run argument patman prints out information showing what it would do. This information currently doesn't line up with what patman/git send-email really do. Some basic examples:
- If an email address is addressed via "Series-cc" and "Patch-cc" patman shows that email address would be CC-ed two times.
- If an email address is addressed via "Series-to" and "Patch-cc" patman shows that email address would be sent TO and CC-ed.
- If an email address is addressed from a combination of tag aliases, get_maintainer.pl output, "Series-cc", "Patch-cc", etc patman shows that the email address would be CC-ed multiple times.
Quoted 8 lines
Patman currently does try to send duplicate emails like the --dry-run output shows, but "git send-email" intelligently removes duplicate addresses so this patch shouldn't change the non-dry-run functionality.
Change patman's output and email addressing to line up with the "git send-email" logic. This trims down patman's dry-run output and prevents confusion about what patman will do when emails are actually sent.
Quoted 4 lines
Thanks for the patch, it's good to match up with git send-email.
Are the rules that git send-email follows documented or obtained by trial and error?
Trial and error initially. The git source code lined up with what I saw (see the send_message function in git-send-email.perl). I didn't see the policy documented officially, but what git does makes sense to me:
- remove any duplicate addresses in the to: field
- remove all the to: addresses from the cc: addresses
- remove any duplicate cc: addresses
This makes sure each email is only sent to an address one time, with the to: field taking precedence over the cc: field.

Quoted 7 lines

Quoted 2 lines
For a recent NAND patch series it looked like I was going to send Scott 2-4 emails per patch which is why I looked into it.
Thanks.
Acked-by Simon Glass <[email protected]>
Tested-by Simon Glass <[email protected]>

Applied to x86/patman and now in mainline.