[bug#60218] [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)

[bug#60218] [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)

[Maxim Cournoyer]

Along with the correct '.patman' configuration file for Guix, this makes it
possible to invoke the 'patman' command and have it add all the required
'--cc' directives, as computed by 'etc/teams.scm get-maintainers
<patch-file>'.


Maxim Cournoyer (2):
  teams: Add a "get-maintainer" command.
  .patman: New configuration file.

 .patman          |  9 +++++++++
 etc/teams.scm.in | 21 ++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 .patman


base-commit: f28ca2447c5e2eef1ba6a3a11587380a665b0e26
-- 
2.38.1

[bug#60218] [PATCH 1/2] teams: Add a "get-maintainer" command.

[Maxim Cournoyer]

This can be used as a compatibility mode with the get_maintainer.pl Perl
script included in the Linux (or U-Boot) source tree.

* etc/teams.scm.in (git-patch->commit-id): New procedure.
(main) <get-maintainer>: Register new command.  Document it.
---

 etc/teams.scm.in | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index aa38a3b798..4f02df79d5 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -5,6 +5,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2022 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2022 Mathieu Othacehe <othacehe@gnu.org>
+;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -34,6 +35,7 @@
              (ice-9 format)
              (ice-9 regex)
              (ice-9 match)
+             (ice-9 rdelim)
              (guix ui)
              (git))
 
@@ -608,6 +610,15 @@ (define (diff-revisions rev-start rev-end)
      (const 0))
     files))
 
+(define (git-patch->commit-id file)
+  "Parse the commit ID from the first line of FILE, a patch produced with git."
+  (call-with-input-file file
+    (lambda (port)
+      (let ((m (string-match "^From ([0-9a-f]{40})" (read-line port))))
+        (unless m
+          (error "invalid patch file:" file))
+        (match:substring m 1)))))
+
 \f
 (define (main . args)
   (match args
@@ -616,6 +627,14 @@ (define (main . args)
     (("cc-members" rev-start rev-end)
      (apply cc (find-team-by-scope
                 (diff-revisions rev-start rev-end))))
+    (("get-maintainer" patch-file)
+     (let* ((rev-end (git-patch->commit-id patch-file))
+            (rev-start (string-append rev-end "^")))
+       (apply main "list-members"
+              (map symbol->string
+                   (map team-id
+                        (find-team-by-scope
+                         (diff-revisions rev-start rev-end)))))))
     (("list-teams" . args)
      (list-teams))
     (("list-members" . team-names)
@@ -631,6 +650,7 @@ (define (main . args)
   cc <team-name>            get git send-email flags for cc-ing <team-name>
   cc-members <start> <end>  cc teams related to files changed between revisions
   list-teams                list teams and their members
-  list-members <team-name>  list members belonging to <team-name>~%"))))
+  list-members <team-name>  list members belonging to <team-name>~%
+  get-maintainer <patch>    compatibility mode with Linux get_maintainer.pl"))))
 
 (apply main (cdr (command-line)))

base-commit: f28ca2447c5e2eef1ba6a3a11587380a665b0e26
-- 
2.38.1

[bug#60218] [PATCH 2/2] .patman: New configuration file.

[Maxim Cournoyer]

* .patman: New file.

---

 .patman | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 .patman

diff --git a/.patman b/.patman
new file mode 100644
index 0000000000..4708bb5ea8
--- /dev/null
+++ b/.patman
@@ -0,0 +1,9 @@
+# This config file allows for Patchwork integration with
+# https://patches.guix-patches.cbaines.net/.
+[settings]
+project: guix-patches
+patchwork_url: https://patches.guix-patches.cbaines.net
+add_signoff: False
+# TODO: enable check_patch
+check_patch: False
+get_maintainer_script: etc/teams.scm get-maintainer
-- 
2.38.1

[bug#60218] [PATCH 1/2] teams: Add a "get-maintainer" command.

[Ricardo Wurmus]

Hi Maxim,

> This can be used as a compatibility mode with the get_maintainer.pl Perl
> script included in the Linux (or U-Boot) source tree.
>
> * etc/teams.scm.in (git-patch->commit-id): New procedure.
> (main) <get-maintainer>: Register new command.  Document it.

Interesting.

> @@ -616,6 +627,14 @@ (define (main . args)
>      (("cc-members" rev-start rev-end)
>       (apply cc (find-team-by-scope
>                  (diff-revisions rev-start rev-end))))
> +    (("get-maintainer" patch-file)
> +     (let* ((rev-end (git-patch->commit-id patch-file))
> +            (rev-start (string-append rev-end "^")))

This is to get the changes introduced by this patch-file right?  In a
format that allows you to use “diff-revisions” below, which you need to
run find-team-by-scope.

> +       (apply main "list-members"
> +              (map symbol->string
> +                   (map team-id
> +                        (find-team-by-scope
> +                         (diff-revisions rev-start rev-end)))))))

Here I’d do

    (map (compose symbol->string team-id) …)

instead of mapping twice.

I haven’t used get_maintainer.pl before, but I don’t object to this
change if it’s useful to you.

-- 
Ricardo

[bug#60218] [PATCH 1/2] teams: Add a "get-maintainer" command.

[Maxim Cournoyer]

Hi Ricardo,

Ricardo Wurmus <rekado@elephly.net> writes:

> Hi Maxim,
>
>> This can be used as a compatibility mode with the get_maintainer.pl Perl
>> script included in the Linux (or U-Boot) source tree.
>>
>> * etc/teams.scm.in (git-patch->commit-id): New procedure.
>> (main) <get-maintainer>: Register new command.  Document it.
>
> Interesting.
>
>> @@ -616,6 +627,14 @@ (define (main . args)
>>      (("cc-members" rev-start rev-end)
>>       (apply cc (find-team-by-scope
>>                  (diff-revisions rev-start rev-end))))
>> +    (("get-maintainer" patch-file)
>> +     (let* ((rev-end (git-patch->commit-id patch-file))
>> +            (rev-start (string-append rev-end "^")))
>
> This is to get the changes introduced by this patch-file right?  In a
> format that allows you to use “diff-revisions” below, which you need to
> run find-team-by-scope.

Yes!  The get-maintainer.pl script expects a single patch file rather
than two git refspecs.

>> +       (apply main "list-members"
>> +              (map symbol->string
>> +                   (map team-id
>> +                        (find-team-by-scope
>> +                         (diff-revisions rev-start rev-end)))))))
>
> Here I’d do
>
>     (map (compose symbol->string team-id) …)
>
> instead of mapping twice.

Thanks, that's better.  Adjusted locally.

> I haven’t used get_maintainer.pl before, but I don’t object to this
> change if it’s useful to you.

It's useful in conjunction with patman (which is patch 2/2 of this
series), which can be configured to use a get-maintainer like script to
retrieve the people it should CC based on the patches it can 'git
send-email' for you.  If nobody else has a say, I'll push in about a
week.

Thanks for taking a look!

-- 
Maxim

[bug#60218] [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)

[Mathieu Othacehe]

Hey Maxim,

>    cc <team-name>            get git send-email flags for cc-ing <team-name>
>    cc-members <start> <end>  cc teams related to files changed between revisions
>    list-teams                list teams and their members
> -  list-members <team-name>  list members belonging to <team-name>~%"))))
> +  list-members <team-name>  list members belonging to <team-name>~%
> +  get-maintainer <patch>    compatibility mode with Linux get_maintainer.pl"))))

Maybe it could be interesting to also add this patch mode to the
cc-members command, this way for instance:

cc-members [<start> <end>|<patch> ...]

and also have the get-maintainer command for compatibility with other
tools?

Thanks,

Mathieu

[bug#60218] [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)

[Maxim Cournoyer]

Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hey Maxim,
>
>>    cc <team-name>            get git send-email flags for cc-ing <team-name>
>>    cc-members <start> <end>  cc teams related to files changed between revisions
>>    list-teams                list teams and their members
>> -  list-members <team-name>  list members belonging to <team-name>~%"))))
>> +  list-members <team-name>  list members belonging to <team-name>~%
>> +  get-maintainer <patch>    compatibility mode with Linux get_maintainer.pl"))))
>
> Maybe it could be interesting to also add this patch mode to the
> cc-members command, this way for instance:
>
> cc-members [<start> <end>|<patch> ...]

Implemented in v2!  I guess it could be useful.  Note that it doesn't
parse the patch for the file names touched, instead it assumes the patch
was produced from a commit registered in git and then use the usual code
path (for simplicity).

-- 
Thanks,
Maxim

[bug#60218] [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)

[Mathieu Othacehe]

Hey,

> Implemented in v2!  I guess it could be useful.  Note that it doesn't
> parse the patch for the file names touched, instead it assumes the patch
> was produced from a commit registered in git and then use the usual code
> path (for simplicity).

Nice! I think that it is fair to assume that the commit is registered in
the local git repository.

As a follow-up a documentation update could also be interesting as I
think that the new 'cc-members patch' command is easier to use that the
'cc-member start end' variant.

I had a look to the rest of the patchset it seems fine to me :)

Thanks,

Mathieu

bug#60218: [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)

[Maxim Cournoyer]

Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hey,
>
>> Implemented in v2!  I guess it could be useful.  Note that it doesn't
>> parse the patch for the file names touched, instead it assumes the patch
>> was produced from a commit registered in git and then use the usual code
>> path (for simplicity).
>
> Nice! I think that it is fair to assume that the commit is registered in
> the local git repository.
>
> As a follow-up a documentation update could also be interesting as I
> think that the new 'cc-members patch' command is easier to use that the
> 'cc-member start end' variant.

OK!  I intend to document the use of patman along teams.scm, as I find
it helps automate things and keep submissions organized.  We can
probably briefly mention the tool, and point the interested user to its
full doc (which lives in u-boot-documentation).

> I had a look to the rest of the patchset it seems fine to me :)

OK, great!  I've now pushed the series.  Happy New Year!

-- 
Thanks,
Maxim

[bug#60218] [PATCH v2 1/3] teams: Add a "get-maintainer" command.

[Maxim Cournoyer]

This can be used as a compatibility mode with the get_maintainer.pl Perl
script included in the Linux (or U-Boot) source tree.

* etc/teams.scm.in (git-patch->commit-id): New procedure.
(main) <get-maintainer>: Register new command.  Document it.

---

Changes in v2:
- Move newline character (~%) in usage output to the bottom

 etc/teams.scm.in | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index f42a7f6f28..e50efea786 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -5,6 +5,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2022 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2022 Mathieu Othacehe <othacehe@gnu.org>
+;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -34,6 +35,7 @@
              (ice-9 format)
              (ice-9 regex)
              (ice-9 match)
+             (ice-9 rdelim)
              (guix ui)
              (git))
 
@@ -623,6 +625,15 @@ (define (diff-revisions rev-start rev-end)
      (const 0))
     files))
 
+(define (git-patch->commit-id file)
+  "Parse the commit ID from the first line of FILE, a patch produced with git."
+  (call-with-input-file file
+    (lambda (port)
+      (let ((m (string-match "^From ([0-9a-f]{40})" (read-line port))))
+        (unless m
+          (error "invalid patch file:" file))
+        (match:substring m 1)))))
+
 \f
 (define (main . args)
   (match args
@@ -631,6 +642,12 @@ (define (main . args)
     (("cc-members" rev-start rev-end)
      (apply cc (find-team-by-scope
                 (diff-revisions rev-start rev-end))))
+    (("get-maintainer" patch-file)
+     (let* ((rev-end (git-patch->commit-id patch-file))
+            (rev-start (string-append rev-end "^")))
+       (apply main "list-members"
+              (map (compose symbol->string team-id)
+                   (find-team-by-scope (diff-revisions rev-start rev-end))))))
     (("list-teams" . args)
      (list-teams))
     (("list-members" . team-names)
@@ -646,6 +663,7 @@ (define (main . args)
   cc <team-name>            get git send-email flags for cc-ing <team-name>
   cc-members <start> <end>  cc teams related to files changed between revisions
   list-teams                list teams and their members
-  list-members <team-name>  list members belonging to <team-name>~%"))))
+  list-members <team-name>  list members belonging to <team-name>
+  get-maintainer <patch>    compatibility mode with Linux get_maintainer.pl~%"))))
 
 (apply main (cdr (command-line)))

base-commit: 8f93a1e01a879ae026678dd92c18e2a2a49be540
-- 
2.38.1

[bug#60218] [PATCH v2 2/3] teams: Allow a patch-file argument to cc-members.

[Maxim Cournoyer]

* etc/teams.scm.in (git-patch->revisions): New procedure.
(main) [cc-members]: New match pattern to support patch file argument.
[get-maintainer]: Simplify using the newly introduced procedure from above.
(main): Update usage doc.

---

Changes in v2:
- New: support passing a patch file to the cc-members command

 etc/teams.scm.in | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index e50efea786..96a04aca3d 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -634,20 +634,29 @@ (define (git-patch->commit-id file)
           (error "invalid patch file:" file))
         (match:substring m 1)))))
 
+(define (git-patch->revisions file)
+  "Return the start and end revisions of FILE, a patch file produced with git."
+  (let* ((rev-end (git-patch->commit-id file))
+         (rev-start (string-append rev-end "^")))
+    (list rev-start rev-end)))
+
 \f
 (define (main . args)
   (match args
     (("cc" . team-names)
      (apply cc (map find-team team-names)))
+    (("cc-members" patch-file)
+     (unless (file-exists? patch-file)
+       (error "patch file does not exist:" patch-file))
+     (apply main "cc-members" (git-patch->revisions patch-file)))
     (("cc-members" rev-start rev-end)
      (apply cc (find-team-by-scope
                 (diff-revisions rev-start rev-end))))
     (("get-maintainer" patch-file)
-     (let* ((rev-end (git-patch->commit-id patch-file))
-            (rev-start (string-append rev-end "^")))
-       (apply main "list-members"
-              (map (compose symbol->string team-id)
-                   (find-team-by-scope (diff-revisions rev-start rev-end))))))
+     (apply main "list-members"
+            (map (compose symbol->string team-id)
+                 (find-team-by-scope (apply diff-revisions
+                                            (git-patch->revisions patch-file))))))
     (("list-teams" . args)
      (list-teams))
     (("list-members" . team-names)
@@ -660,10 +669,15 @@ (define (main . args)
              "Usage: etc/teams.scm <command> [<args>]
 
 Commands:
-  cc <team-name>            get git send-email flags for cc-ing <team-name>
-  cc-members <start> <end>  cc teams related to files changed between revisions
-  list-teams                list teams and their members
-  list-members <team-name>  list members belonging to <team-name>
-  get-maintainer <patch>    compatibility mode with Linux get_maintainer.pl~%"))))
+  cc <team-name>
+      get git send-email flags for cc-ing <team-name>
+  cc-members <start> <end> | patch
+      cc teams related to files changed between revisions or in a patch file
+  list-teams
+      list teams and their members
+  list-members <team-name>
+      list members belonging to <team-name>
+  get-maintainer <patch>
+      compatibility mode with Linux get_maintainer.pl~%"))))
 
 (apply main (cdr (command-line)))
-- 
2.38.1

[bug#60218] [PATCH v2 3/3] .patman: New configuration file.

[Maxim Cournoyer]

* .patman: New file.

---

(no changes since v1)

 .patman | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 .patman

diff --git a/.patman b/.patman
new file mode 100644
index 0000000000..4708bb5ea8
--- /dev/null
+++ b/.patman
@@ -0,0 +1,9 @@
+# This config file allows for Patchwork integration with
+# https://patches.guix-patches.cbaines.net/.
+[settings]
+project: guix-patches
+patchwork_url: https://patches.guix-patches.cbaines.net
+add_signoff: False
+# TODO: enable check_patch
+check_patch: False
+get_maintainer_script: etc/teams.scm get-maintainer
-- 
2.38.1

[bug#60218] [PATCH v2 3/3] .patman: New configuration file.

[Simon Tournier]

Hi Maxim,

On Tue, 27 Dec 2022 at 10:32, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> +++ b/.patman
> @@ -0,0 +1,9 @@
> +# This config file allows for Patchwork integration with
> +# https://patches.guix-patches.cbaines.net/.
> +[settings]
> +project: guix-patches
> +patchwork_url: https://patches.guix-patches.cbaines.net
> +add_signoff: False
> +# TODO: enable check_patch
> +check_patch: False
> +get_maintainer_script: etc/teams.scm get-maintainer

Maybe it could be worse to provide an example about how to use this
.patman file.   Maybe under Notifying Teams [1] or Teams [2].  WDYT?

1: https://guix.gnu.org/manual/devel/en/guix.html#Notifying-Teams-1
2: https://guix.gnu.org/manual/devel/en/guix.html#Teams

Cheers,
simon

[bug#60218] [PATCH v2 3/3] .patman: New configuration file.

[zimoun]

Hi Maxim,

On Fri, 06 Jan 2023 at 18:26, Simon Tournier <zimon.toutoune@gmail.com> wrote:
> On Tue, 27 Dec 2022 at 10:32, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>> +++ b/.patman
>> @@ -0,0 +1,9 @@
>> +# This config file allows for Patchwork integration with
>> +# https://patches.guix-patches.cbaines.net/.
>> +[settings]
>> +project: guix-patches
>> +patchwork_url: https://patches.guix-patches.cbaines.net
>> +add_signoff: False
>> +# TODO: enable check_patch
>> +check_patch: False
>> +get_maintainer_script: etc/teams.scm get-maintainer
>
> Maybe it could be worse to provide an example about how to use this
> .patman file.   Maybe under Notifying Teams [1] or Teams [2].  WDYT?

s/worse/worth :-)

> 1: https://guix.gnu.org/manual/devel/en/guix.html#Notifying-Teams-1
> 2: https://guix.gnu.org/manual/devel/en/guix.html#Teams

In #58813 [3], you suggest to reference to patman documentation.  That’s
my suggestion, so let discuss overthere. :-)

3: <http://issues.guix.gnu.org/msgid/87358jmnwj.fsf@gmail.com>

Cheers,
simon