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

  • Done
  • quality assurance status badge
Details
4 participants
  • Maxim Cournoyer
  • Mathieu Othacehe
  • Ricardo Wurmus
  • Simon Tournier
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 20 Dec 2022 14:58
(address . guix-patches@gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221220135810.28175-1-maxim.cournoyer@gmail.com
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
M
M
Maxim Cournoyer wrote on 20 Dec 2022 15:13
[PATCH 1/2] teams: Add a "get-maintainer" command.
(address . 60218@debbugs.gnu.org)
20221220141330.30372-1-maxim.cournoyer@gmail.com
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(-)

Toggle diff (64 lines)
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)))))
+
(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
M
M
Maxim Cournoyer wrote on 20 Dec 2022 15:13
[PATCH 2/2] .patman: New configuration file.
(address . 60218@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20221220141330.30372-2-maxim.cournoyer@gmail.com
* .patman: New file.

---

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

Toggle diff (17 lines)
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
R
R
Ricardo Wurmus wrote on 25 Dec 2022 00:15
Re: [PATCH 1/2] teams: Add a "get-maintainer" command.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87tu1kbd43.fsf@elephly.net
Hi Maxim,

Toggle quote (6 lines)
> 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.

Toggle quote (8 lines)
> @@ -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.

Toggle quote (6 lines)
> + (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
M
M
Maxim Cournoyer wrote on 27 Dec 2022 04:19
(name . Ricardo Wurmus)(address . rekado@elephly.net)
87cz858re8.fsf@gmail.com
Hi Ricardo,

Ricardo Wurmus <rekado@elephly.net> writes:

Toggle quote (22 lines)
> 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.

Toggle quote (12 lines)
>> + (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.

Toggle quote (3 lines)
> 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
M
M
Mathieu Othacehe wrote on 27 Dec 2022 11:00
Re: bug#60218: [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87pmc5maj2.fsf_-_@gnu.org
Hey Maxim,

Toggle quote (7 lines)
> 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
M
M
Maxim Cournoyer wrote on 27 Dec 2022 16:32
[PATCH v2 1/3] teams: Add a "get-maintainer" command.
(address . 60218@debbugs.gnu.org)
20221227153249.16793-1-maxim.cournoyer@gmail.com
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(-)

Toggle diff (62 lines)
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)))))
+
(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
M
M
Maxim Cournoyer wrote on 27 Dec 2022 16:32
[PATCH v2 2/3] teams: Allow a patch-file argument to cc-members.
(address . 60218@debbugs.gnu.org)
20221227153249.16793-2-maxim.cournoyer@gmail.com
* 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(-)

Toggle diff (62 lines)
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)))
+
(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
M
M
Maxim Cournoyer wrote on 27 Dec 2022 16:32
[PATCH v2 3/3] .patman: New configuration file.
(address . 60218@debbugs.gnu.org)
20221227153249.16793-3-maxim.cournoyer@gmail.com
* .patman: New file.

---

(no changes since v1)

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

Toggle diff (17 lines)
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
M
M
Maxim Cournoyer wrote on 27 Dec 2022 16:35
Re: bug#60218: [PATCH 0/2] New teams.scm 'get-maintainer' command (for integration with patman)
(name . Mathieu Othacehe)(address . othacehe@gnu.org)
8735907tcf.fsf@gmail.com
Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

Toggle quote (14 lines)
> 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
M
M
Mathieu Othacehe wrote on 28 Dec 2022 18:22
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87v8lvwiid.fsf_-_@gnu.org
Hey,

Toggle quote (5 lines)
> 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
M
M
Maxim Cournoyer wrote on 28 Dec 2022 21:42
(name . Mathieu Othacehe)(address . othacehe@gnu.org)
87a6375kh0.fsf@gmail.com
Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

Toggle quote (14 lines)
> 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).

Toggle quote (2 lines)
> 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
Closed
S
S
Simon Tournier wrote on 6 Jan 2023 18:26
Re: [bug#60218] [PATCH v2 3/3] .patman: New configuration file.
87h6x3h8wh.fsf@gmail.com
Hi Maxim,

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

Toggle quote (12 lines)
> +++ 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?


Cheers,
simon
Z
Z
zimoun wrote on 11 Jan 2023 16:10
86bkn55cq7.fsf@gmail.com
Hi Maxim,

On Fri, 06 Jan 2023 at 18:26, Simon Tournier <zimon.toutoune@gmail.com> wrote:
Toggle quote (17 lines)
> 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 :-)

Toggle quote (3 lines)
> 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. :-)


Cheers,
simon
?
Your comment

This issue is archived.

To comment on this conversation send an email to 60218@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 60218
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch