GNU bug report logs - #63378
A single X-Debbugs-CC header must be used

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 63378 in the body.
You can then email your comments to 63378 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: bug-guix <bug-guix <at> gnu.org>
Subject: A single X-Debbugs-CC header must be used
Date: Mon, 08 May 2023 13:17:40 -0400

Hi,

After some tests, it appears that a single X-Debbugs-CC header must be
used, otherwise the last one is the one that prevails.  This matches my
reading of the 'process' script of the GNU Debbugs instance [0], and
thus must conform to the same email rules outlined in RFC5322 for the To
or CC fields (only one such header must be used); multiple values can be
separated by a comma. [1]

--8<---------------cut here---------------start------------->8---
my %header;

for my $hdr (@headerlines) {
    $hdr = decode_rfc1522($hdr);
    $_ = $hdr;
    s/\n\s/ /g;
    &finish if m/^x-loop: (\S+)$/i && $1 eq "$gMaintainerEmail";
    my $ins = !m/^subject:/i && !m/^reply-to:/i && !m/^return-path:/i
           && !m/^From / && !m/^X-Debbugs-/i && !m/^cc:/i && !m/^to:/i;
    $fwd .= $hdr."\n" if $ins;
    # print DEBUG ">$_<\n";
    if (s/^(\S+):\s*//) {
	my $v = lc $1;
	print DEBUG ">$v=$_<\n";
        ## There may be multiple To: or Cc: headers (see bug#5996).
        if ( ($v eq 'to' || $v eq 'cc') &&
	     defined $header{$v} && length($header{$v}) ) {
            $header{$v} = $header{$v} . ', ' . $_ if length($_);
        } else {
            $header{$v} = $_;
        }
    } else {
	print DEBUG "!>$_<\n";
    }
}
$header{'message-id'} = '' if not defined $header{'message-id'};
--8<---------------cut here---------------end--------------->8---

Only 'to' or 'cc' multiple headers are coalesced into one; otherwise the
$header specific key (for a given header) is overridden to the last
value encountered at line '$header{$v} = $_;', IIUC.

Our teams.scm script should be adjusted to produce a single X-Debbugs-CC
header with comma-separated values.

[0]  https://gitlab.com/npostavs/debbugs/-/blob/gnu-reconstruction/scripts/process#L171
[1]  https://datatracker.ietf.org/doc/rfc5322/, 3.6 Field Definitions

-- 
Thanks,
Maxim

Message #8 received at 63378 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 63378 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH] teams: Fix script to produce a single X-Debbugs-Cc header.
Date: Mon,  8 May 2023 14:29:42 -0400

Fixes <https://issues.guix.gnu.org/63378>.

* etc/teams.scm.in (cc): Adjust format pattern.
(team->members, member->string): New procedures.
(list-members): Refactor in terms of the above procedures.
(main): Adjust the output of the 'cc-members-header-cmd' and
'cc-mentors-header-cmd' actions.
---
 etc/teams.scm.in | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index dec175f630..50bfbca22e 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -605,23 +605,27 @@ (define (cc . teams)
   "Return arguments for `git send-email' to notify the members of the given
 TEAMS when a patch is received by Debbugs."
   (format #true
-          "~{--add-header=\"X-Debbugs-Cc: ~a\"~^ ~}"
+          "--add-header=\"X-Debbugs-Cc: ~{~a~^,~}\""
           (map person-email
                (delete-duplicates (append-map team-members teams) equal?))))
 
+(define (team->members team)
+  "Return the list of members in TEAM."
+  (sort (team-members team)
+        (lambda (m1 m2)
+          (string<? (person-name m1) (person-name m2)))))
+
+(define (member->string member)
+  "Return the 'email <name>' string representation of MEMBER."
+  (format #false "~a <~a>" (person-name member) (person-email member)))
+
 (define* (list-members team #:optional port (prefix ""))
   "Print the members of the given TEAM."
   (define port* (or port (current-output-port)))
   (for-each
    (lambda (member)
-     (format port*
-             "~a~a <~a>~%"
-             prefix
-             (person-name member)
-             (person-email member)))
-   (sort
-    (team-members team)
-    (lambda (m1 m2) (string<? (person-name m1) (person-name m2))))))
+     (format port* "~a~a~%" prefix (member->string member)))
+   (team->members team)))
 
 (define (list-teams)
   "Print all teams, their scope and their members."
@@ -715,13 +719,14 @@ (define (main . args)
      (apply cc (find-team-by-scope
                 (diff-revisions rev-start rev-end))))
     (("cc-members-header-cmd" patch-file)
-     (for-each (lambda (team-name)
-                 (list-members (find-team team-name) (current-output-port)
-                               "X-Debbugs-Cc: "))
-               (patch->teams patch-file)))
+     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
+             (append-map (compose (cut map member->string <>)
+                                  team->members
+                                  find-team)
+                         (patch->teams patch-file))))
     (("cc-mentors-header-cmd" patch-file)
-     (list-members (find-team "mentors") (current-output-port)
-                   "X-Debbugs-Cc: "))
+     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
+             (map member->string (team->members (find-team "mentors")))))
     (("get-maintainer" patch-file)
      (apply main "list-members" (patch->teams patch-file)))
     (("list-teams" . args)

base-commit: 4228d3c358669f5d15f01d3ba466f3356ecb6546
-- 
2.39.2

Message #11 received at 63378 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 63378 <at> debbugs.gnu.org
Cc: rekado <at> elephly.net, arunisaac <at> systemreboot.net
Subject: Re: bug#63378: A single X-Debbugs-CC header must be used
Date: Tue, 09 May 2023 09:33:07 -0400

Hello,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> Fixes <https://issues.guix.gnu.org/63378>.
>
> * etc/teams.scm.in (cc): Adjust format pattern.
> (team->members, member->string): New procedures.
> (list-members): Refactor in terms of the above procedures.
> (main): Adjust the output of the 'cc-members-header-cmd' and
> 'cc-mentors-header-cmd' actions.

Paul from Debian has posted a merge request to the upstream Debbugs [0]
that would make it possible to use multiple X-Debbugs-CC.

Now, I don't think the GNU Debbugs instance is kept up-to-date with
Debbugs upstream, having its own set of changes, so I think the patch
here should still be applied in the meantime.

[0]  https://salsa.debian.org/debbugs-team/debbugs/-/merge_requests/18/diffs

-- 
Thanks,
Maxim

Message #14 received at 63378 <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 63378 <at> debbugs.gnu.org
Cc: rekado <at> elephly.net
Subject: Re: bug#63378: A single X-Debbugs-CC header must be used
Date: Wed, 10 May 2023 00:43:00 +0100

> Now, I don't think the GNU Debbugs instance is kept up-to-date with
> Debbugs upstream, having its own set of changes, so I think the patch
> here should still be applied in the meantime.

I agree. And, even otherwise, it is nice for X-Debbugs-Cc to mirror the
Cc header and use a comma-separated list. So, I'm all in favour of this
patch. Specific comments about the patch itself follow in a separate
email.

I will work on a similar patch for `mumi send-email' and send it
sometime later this week. As discussed earlier, I will make `mumi
send-email' invoke `git config sendemail.headerCmd' to find out about
etc/teams.scm.

Message #17 received at 63378 <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 63378 <at> debbugs.gnu.org
Cc: rekado <at> elephly.net
Subject: Re: bug#63378: [PATCH] teams: Fix script to produce a single
 X-Debbugs-Cc header.
Date: Wed, 10 May 2023 00:55:45 +0100

Hi Maxim,

When a patch relates to no teams, I get the empty header output
"X-Debbugs-Cc: ". For such patches, it would be nicer to not add any
header instead of adding an empty header.

> +(define (team->members team)
> +  "Return the list of members in TEAM."
> +  (sort (team-members team)
> +        (lambda (m1 m2)
> +          (string<? (person-name m1) (person-name m2)))))
> +
> +(define (member->string member)
> +  "Return the 'email <name>' string representation of MEMBER."
> +  (format #false "~a <~a>" (person-name member) (person-email
> member)))

When person name contains a comma, it should be escaped by surrounding
in double quotes. Else, the comma would interfere with the
comma-separated list that we are constructing for
X-Debbugs-Cc. Admittedly, none of our current team members have commas
in their person names. But, some people like to have a name like
"LastName, FirstName". We should protect against that edge case in the
interest of futureproofing.

> +     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
> +             (append-map (compose (cut map member->string <>)
> +                                  team->members
> +                                  find-team)
> +                         (patch->teams patch-file))))

A very nitpicky suggestion: Maybe, separate multiple persons by ", "
instead of just ",". ", " looks slightly better cosmetically. Likewise
in other places where this occurs.

Thanks for all the hard work! :-)

Regards,
Arun

Message #20 received at 63378 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Arun Isaac <arunisaac <at> systemreboot.net>
Cc: rekado <at> elephly.net, 63378 <at> debbugs.gnu.org
Subject: Re: bug#63378: A single X-Debbugs-CC header must be used
Date: Tue, 09 May 2023 21:15:21 -0400

Hi Arun,

Arun Isaac <arunisaac <at> systemreboot.net> writes:

>> Now, I don't think the GNU Debbugs instance is kept up-to-date with
>> Debbugs upstream, having its own set of changes, so I think the patch
>> here should still be applied in the meantime.
>
> I agree. And, even otherwise, it is nice for X-Debbugs-Cc to mirror the
> Cc header and use a comma-separated list. So, I'm all in favour of this
> patch. Specific comments about the patch itself follow in a separate
> email.
>
> I will work on a similar patch for `mumi send-email' and send it
> sometime later this week. As discussed earlier, I will make `mumi
> send-email' invoke `git config sendemail.headerCmd' to find out about
> etc/teams.scm.

Sounds good, and thanks for your efforts pushing toward better teams
tooling!  I'll now address your comments.

-- 
Thanks,
Maxim

Message #23 received at 63378 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 63378 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v2] teams: Fix script to produce a single X-Debbugs-Cc header.
Date: Tue,  9 May 2023 22:55:02 -0400

Fixes <https://issues.guix.gnu.org/63378>.

* etc/teams.scm.in (cc): Adjust format pattern.
(sort-members, member->string): New procedures.
(list-members): Refactor in terms of the above procedures.
(main): Adjust the output of the 'cc-members-header-cmd' and
'cc-mentors-header-cmd' actions.
---
 etc/teams.scm.in | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index d21f1ff67b..d81ce984f6 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -605,24 +605,28 @@ (define (find-team-by-scope files)
 (define (cc . teams)
   "Return arguments for `git send-email' to notify the members of the given
 TEAMS when a patch is received by Debbugs."
-  (format #true
-          "~{--add-header=\"X-Debbugs-Cc: ~a\"~^ ~}"
-          (map person-email
-               (delete-duplicates (append-map team-members teams) equal?))))
+  (let ((members (append-map team-members teams)))
+    (unless (null? members)
+      (format #true "--add-header=\"X-Debbugs-Cc: ~{~a~^, ~}\""
+              (map person-email (sort-members members))))))
+
+(define (sort-members members)
+  "Deduplicate and sort MEMBERS alphabetically by their name."
+  (sort (delete-duplicates members equal?)
+        (lambda (m1 m2)
+          (string<? (person-name m1) (person-name m2)))))
+
+(define (member->string member)
+  "Return the 'email <name>' string representation of MEMBER."
+  (format #false "\"~a\" <~a>" (person-name member) (person-email member)))
 
 (define* (list-members team #:optional port (prefix ""))
   "Print the members of the given TEAM."
   (define port* (or port (current-output-port)))
   (for-each
    (lambda (member)
-     (format port*
-             "~a~a <~a>~%"
-             prefix
-             (person-name member)
-             (person-email member)))
-   (sort
-    (team-members team)
-    (lambda (m1 m2) (string<? (person-name m1) (person-name m2))))))
+     (format port* "~a~a~%" prefix (member->string member)))
+   (sort-members (team-members team))))
 
 (define (list-teams)
   "Print all teams, their scope and their members."
@@ -716,13 +720,15 @@ (define (main . args)
      (apply cc (find-team-by-scope
                 (diff-revisions rev-start rev-end))))
     (("cc-members-header-cmd" patch-file)
-     (for-each (lambda (team-name)
-                 (list-members (find-team team-name) (current-output-port)
-                               "X-Debbugs-Cc: "))
-               (patch->teams patch-file)))
+     (let* ((teams (map find-team (patch->teams patch-file)))
+            (members (sort-members (append-map team-members teams))))
+       (unless (null? members)
+         (format #true "X-Debbugs-Cc: ~{~a~^, ~}"
+                 (map member->string members)))))
     (("cc-mentors-header-cmd" patch-file)
-     (list-members (find-team "mentors") (current-output-port)
-                   "X-Debbugs-Cc: "))
+     (format #true "X-Debbugs-Cc: ~{~a~^, ~}"
+             (map member->string
+                  (sort-members (team-members (find-team "mentors"))))))
     (("get-maintainer" patch-file)
      (apply main "list-members" (patch->teams patch-file)))
     (("list-teams" . args)

base-commit: 8f92dfd9ae7ac491ab7fb4b425799a8c909708a8
-- 
2.39.2

Message #26 received at 63378 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Arun Isaac <arunisaac <at> systemreboot.net>
Cc: rekado <at> elephly.net, 63378 <at> debbugs.gnu.org
Subject: Re: bug#63378: [PATCH] teams: Fix script to produce a single
 X-Debbugs-Cc header.
Date: Wed, 10 May 2023 12:15:15 -0400

Hi Arun,

Arun Isaac <arunisaac <at> systemreboot.net> writes:

> Hi Maxim,
>
> When a patch relates to no teams, I get the empty header output
> "X-Debbugs-Cc: ". For such patches, it would be nicer to not add any
> header instead of adding an empty header.

Good catch!  Fixed.

>> +(define (team->members team)
>> +  "Return the list of members in TEAM."
>> +  (sort (team-members team)
>> +        (lambda (m1 m2)
>> +          (string<? (person-name m1) (person-name m2)))))
>> +
>> +(define (member->string member)
>> +  "Return the 'email <name>' string representation of MEMBER."
>> +  (format #false "~a <~a>" (person-name member) (person-email
>> member)))
>
> When person name contains a comma, it should be escaped by surrounding
> in double quotes. Else, the comma would interfere with the
> comma-separated list that we are constructing for
> X-Debbugs-Cc. Admittedly, none of our current team members have commas
> in their person names. But, some people like to have a name like
> "LastName, FirstName". We should protect against that edge case in the
> interest of futureproofing.

OK!  I opted for simplicity and double-quoted all the names.

>> +     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
>> +             (append-map (compose (cut map member->string <>)
>> +                                  team->members
>> +                                  find-team)
>> +                         (patch->teams patch-file))))
>
> A very nitpicky suggestion: Maybe, separate multiple persons by ", "
> instead of just ",". ", " looks slightly better cosmetically. Likewise
> in other places where this occurs.

Good idea; I've learnt this was valid email syntax just today :-).

Below is the diff of my rework:

--8<---------------cut here---------------start------------->8---
1 file changed, 18 insertions(+), 17 deletions(-)
etc/teams.scm.in | 35 ++++++++++++++++++-----------------

modified   etc/teams.scm.in
@@ -605,20 +605,20 @@ (define (find-team-by-scope files)
 (define (cc . teams)
   "Return arguments for `git send-email' to notify the members of the given
 TEAMS when a patch is received by Debbugs."
-  (format #true
-          "--add-header=\"X-Debbugs-Cc: ~{~a~^,~}\""
-          (map person-email
-               (delete-duplicates (append-map team-members teams) equal?))))
-
-(define (team->members team)
-  "Return the list of members in TEAM."
-  (sort (team-members team)
+  (let ((members (append-map team-members teams)))
+    (unless (null? members)
+      (format #true "--add-header=\"X-Debbugs-Cc: ~{~a~^, ~}\""
+              (map person-email (sort-members members))))))
+
+(define (sort-members members)
+  "Deduplicate and sort MEMBERS alphabetically by their name."
+  (sort (delete-duplicates members equal?)
         (lambda (m1 m2)
           (string<? (person-name m1) (person-name m2)))))
 
 (define (member->string member)
   "Return the 'email <name>' string representation of MEMBER."
-  (format #false "~a <~a>" (person-name member) (person-email member)))
+  (format #false "\"~a\" <~a>" (person-name member) (person-email member)))
 
 (define* (list-members team #:optional port (prefix ""))
   "Print the members of the given TEAM."
@@ -626,7 +626,7 @@ (define* (list-members team #:optional port (prefix ""))
   (for-each
    (lambda (member)
      (format port* "~a~a~%" prefix (member->string member)))
-   (team->members team)))
+   (sort-members (team-members team))))
 
 (define (list-teams)
   "Print all teams, their scope and their members."
@@ -720,14 +720,15 @@ (define (main . args)
      (apply cc (find-team-by-scope
                 (diff-revisions rev-start rev-end))))
     (("cc-members-header-cmd" patch-file)
-     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
-             (append-map (compose (cut map member->string <>)
-                                  team->members
-                                  find-team)
-                         (patch->teams patch-file))))
+     (let* ((teams (map find-team (patch->teams patch-file)))
+            (members (sort-members (append-map team-members teams))))
+       (unless (null? members)
+         (format #true "X-Debbugs-Cc: ~{~a~^, ~}"
+                 (map member->string members)))))
     (("cc-mentors-header-cmd" patch-file)
-     (format #true "X-Debbugs-Cc: ~{~a~^,~}"
-             (map member->string (team->members (find-team "mentors")))))
+     (format #true "X-Debbugs-Cc: ~{~a~^, ~}"
+             (map member->string
+                  (sort-members (team-members (find-team "mentors"))))))
     (("get-maintainer" patch-file)
      (apply main "list-members" (patch->teams patch-file)))
     (("list-teams" . args)
--8<---------------cut here---------------end--------------->8---

A proper v2 patch has been sent.

-- 
Thanks,
Maxim

Message #29 received at 63378 <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: rekado <at> elephly.net, 63378 <at> debbugs.gnu.org
Subject: Re: bug#63378: [PATCH] teams: Fix script to produce a single
 X-Debbugs-Cc header.
Date: Thu, 11 May 2023 12:13:27 +0100

Hi Maxim,

Thank you for the updated patch! :-) It LGTM. Please push.

> OK!  I opted for simplicity and double-quoted all the names.

Fair enough. Though a check is only one condition away! ;-)

(if (string-contains? (person-name member) ",")
    (string-append "\"" (person-name member) "\"")
    (person-name member))

Regards,
Arun

Message #34 received at 63378-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Arun Isaac <arunisaac <at> systemreboot.net>
Cc: rekado <at> elephly.net, 63378-done <at> debbugs.gnu.org
Subject: Re: bug#63378: [PATCH] teams: Fix script to produce a single
 X-Debbugs-Cc header.
Date: Thu, 11 May 2023 09:18:46 -0400

Hi Arun,

Arun Isaac <arunisaac <at> systemreboot.net> writes:

> Hi Maxim,
>
> Thank you for the updated patch! :-) It LGTM. Please push.
>
>> OK!  I opted for simplicity and double-quoted all the names.
>
> Fair enough. Though a check is only one condition away! ;-)
>
> (if (string-contains? (person-name member) ",")
>     (string-append "\"" (person-name member) "\"")
>     (person-name member))

It's string-contains without ?, apparently.  Let's try this (and save a
few bytes per submission :-)):

--8<---------------cut here---------------start------------->8---
modified   etc/teams.scm.in
@@ -618,7 +618,11 @@ (define (sort-members members)

 (define (member->string member)
   "Return the 'email <name>' string representation of MEMBER."
-  (format #false "\"~a\" <~a>" (person-name member) (person-email member)))
+  (let* ((name (person-name member))
+         (quoted-name/maybe (if (string-contains name ",")
+                                (string-append "\"" name "\"")
+                                name)))
+    (format #false "~a <~a>" quoted-name/maybe (person-email member))))

 (define* (list-members team #:optional port (prefix ""))
   "Print the members of the given TEAM."
--8<---------------cut here---------------end--------------->8---

The change is now installed; thanks for the review!

-- 
Thanks,
Maxim

Message #37 received at 63378-done <at> debbugs.gnu.org (full text, mbox):

From: Arun Isaac <arunisaac <at> systemreboot.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: rekado <at> elephly.net, 63378-done <at> debbugs.gnu.org
Subject: Re: bug#63378: [PATCH] teams: Fix script to produce a single
 X-Debbugs-Cc header.
Date: Fri, 12 May 2023 00:59:00 +0100

> It's string-contains without ?, apparently.

Ah, yes. That one always trips me up.

> The change is now installed; thanks for the review!

Thank you! :-)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.