can't substitute etc/teams.scm command as doc suggests

  • Open
  • quality assurance status badge
Details
4 participants
  • Liliana Marie Prikler
  • Ludovic Courtès
  • Maxim Cournoyer
  • Simon Tournier
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 27 Oct 2022 05:50
(name . bug-guix)(address . bug-guix@gnu.org)
87r0yuq615.fsf@gmail.com
Hi,

Today, I tried;

Toggle snippet (11 lines)
$ git send-email --to=guix-patches@gnu.org \
$(./etc/teams.scm cc-members origin/master HEAD) 0000-cover-letter.patch
fatal: ambiguous argument 'some.email@redacted.com"': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
format-patch -o /tmp/pFSRbRNNoU --add-header="X-Debbugs-Cc: redacted@gmail.com" --add-header="X-Debbugs-Cc: redacted@email" [...]: command returned error: 128

$ ./etc/teams.scm cc-members origin/master HEAD
--add-header="X-Debbugs-Cc: redacted@email" --add-header="X-Debbugs-Cc: redacted@email" ...

You can see the command fails; this is because when using Bash command
substitution $(), the quotes in the result are not interpreted and are
thus part of the value (literals), which then gets split on white space.

As a quick hacky fix, I tried removing the space and double quotes
like:

modified etc/teams.scm.in
@@ -514,7 +514,7 @@ (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?))))

and sent a patch with that command:

git send-email --to=guix-patches@gnu.org \
$(./etc/teams.scm cc-members origin/master HEAD 0000-cover-letter.patch

It created https://issues.guix.gnu.org/58812with it, but I don't see
any of the X-Debbugs-Cc headers. Mmmh.

--
Thanks,
Maxim
L
L
Liliana Marie Prikler wrote on 27 Oct 2022 08:08
05f449076bd7fd65f3cd301cbed4101490b3b2db.camel@gmail.com
Am Mittwoch, dem 26.10.2022 um 23:50 -0400 schrieb Maxim Cournoyer:
Toggle quote (49 lines)
> Hi,
>
> Today, I tried;
>
> --8<---------------cut here---------------start------------->8---
> $ git send-email --to=guix-patches@gnu.org \
>   $(./etc/teams.scm cc-members origin/master HEAD) 0000-cover-
> letter.patch
> fatal: ambiguous argument 'some.email@redacted.com"': unknown
> revision or path not in the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git <command> [<revision>...] -- [<file>...]'
> format-patch -o /tmp/pFSRbRNNoU --add-header="X-Debbugs-Cc:
> redacted@gmail.com" --add-header="X-Debbugs-Cc: redacted@email"
> [...]: command returned error: 128
>
> $ ./etc/teams.scm cc-members origin/master HEAD
> --add-header="X-Debbugs-Cc: redacted@email" --add-header="X-Debbugs-
> Cc: redacted@email" ...
> --8<---------------cut here---------------end--------------->8---
>
> You can see the command fails; this is because when using Bash
> command substitution $(), the quotes in the result are not
> interpreted and are thus part of the value (literals), which then
> gets split on white space.
>
> As a quick hacky fix,  I tried removing the space and double quotes
> like:
>
> modified   etc/teams.scm.in
> @@ -514,7 +514,7 @@ (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?))))
>
> and sent a patch with that command:
>
> git send-email --to=guix-patches@gnu.org \
>   $(./etc/teams.scm cc-members origin/master HEAD 0000-cover-
> letter.patch
>
> It created https://issues.guix.gnu.org/58812 with it, but I don't see
> any of the X-Debbugs-Cc headers.  Mmmh.
Note that the existing etc/teams also assumes there are no funny
characters in the quote. So it's susceptible to good ol' bobby tables.

Could we, instead of outputting a command, make it so that we can pass
an already formatted patch and etc/teams rewrites it?

Cheers
M
M
Maxim Cournoyer wrote on 27 Oct 2022 14:40
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)(address . 58813@debbugs.gnu.org)
87lep1qw1s.fsf@gmail.com
Hi,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (56 lines)
> Am Mittwoch, dem 26.10.2022 um 23:50 -0400 schrieb Maxim Cournoyer:
>> Hi,
>>
>> Today, I tried;
>>
>> --8<---------------cut here---------------start------------->8---
>> $ git send-email --to=guix-patches@gnu.org \
>>   $(./etc/teams.scm cc-members origin/master HEAD) 0000-cover-
>> letter.patch
>> fatal: ambiguous argument 'some.email@redacted.com"': unknown
>> revision or path not in the working tree.
>> Use '--' to separate paths from revisions, like this:
>> 'git <command> [<revision>...] -- [<file>...]'
>> format-patch -o /tmp/pFSRbRNNoU --add-header="X-Debbugs-Cc:
>> redacted@gmail.com" --add-header="X-Debbugs-Cc: redacted@email"
>> [...]: command returned error: 128
>>
>> $ ./etc/teams.scm cc-members origin/master HEAD
>> --add-header="X-Debbugs-Cc: redacted@email" --add-header="X-Debbugs-
>> Cc: redacted@email" ...
>> --8<---------------cut here---------------end--------------->8---
>>
>> You can see the command fails; this is because when using Bash
>> command substitution $(), the quotes in the result are not
>> interpreted and are thus part of the value (literals), which then
>> gets split on white space.
>>
>> As a quick hacky fix,  I tried removing the space and double quotes
>> like:
>>
>> modified   etc/teams.scm.in
>> @@ -514,7 +514,7 @@ (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?))))
>>
>> and sent a patch with that command:
>>
>> git send-email --to=guix-patches@gnu.org \
>>   $(./etc/teams.scm cc-members origin/master HEAD 0000-cover-
>> letter.patch
>>
>> It created https://issues.guix.gnu.org/58812 with it, but I don't see
>> any of the X-Debbugs-Cc headers.  Mmmh.
> Note that the existing etc/teams also assumes there are no funny
> characters in the quote. So it's susceptible to good ol' bobby tables.
>
> Could we, instead of outputting a command, make it so that we can pass
> an already formatted patch and etc/teams rewrites it?

The solution suggested to me in #bash would be to turn etc/teams.scm
into a git wrapper, that could invoke git with all the arguments at once
(allowing people to pass arguments themselves). I'm told the git
completion could be hooked to such script so that users can still enjoy
git tab-completion when using etc/teams.scm, although I haven't
researched how that'd all work.

A similar idea proposed by selckin on #git would be to have our
etc/teams.scm script receive the complete git command, and append the
args to it itself (and invoke it), like so:

./etc/teams.scm cc mentors -- git send-email --to XXX@debbugs.gnu.org *.patch

--
Thanks,
Maxim
L
L
Liliana Marie Prikler wrote on 27 Oct 2022 18:27
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 58813@debbugs.gnu.org)
1af5b004bf08bdb954dd7f924b6aa1dbb563c075.camel@gmail.com
Am Donnerstag, dem 27.10.2022 um 08:40 -0400 schrieb Maxim Cournoyer:
Toggle quote (80 lines)
> Hi,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
> > Am Mittwoch, dem 26.10.2022 um 23:50 -0400 schrieb Maxim Cournoyer:
> > > Hi,
> > >
> > > Today, I tried;
> > >
> > > --8<---------------cut here---------------start------------->8---
> > > $ git send-email --to=guix-patches@gnu.org \
> > >   $(./etc/teams.scm cc-members origin/master HEAD) 0000-cover-
> > > letter.patch
> > > fatal: ambiguous argument 'some.email@redacted.com"': unknown
> > > revision or path not in the working tree.
> > > Use '--' to separate paths from revisions, like this:
> > > 'git <command> [<revision>...] -- [<file>...]'
> > > format-patch -o /tmp/pFSRbRNNoU --add-header="X-Debbugs-Cc:
> > > redacted@gmail.com" --add-header="X-Debbugs-Cc: redacted@email"
> > > [...]: command returned error: 128
> > >
> > > $ ./etc/teams.scm cc-members origin/master HEAD
> > > --add-header="X-Debbugs-Cc: redacted@email" --add-header="X-
> > > Debbugs-
> > > Cc: redacted@email" ...
> > > --8<---------------cut here---------------end--------------->8---
> > >
> > > You can see the command fails; this is because when using Bash
> > > command substitution $(), the quotes in the result are not
> > > interpreted and are thus part of the value (literals), which then
> > > gets split on white space.
> > >
> > > As a quick hacky fix,  I tried removing the space and double
> > > quotes
> > > like:
> > >
> > > modified   etc/teams.scm.in
> > > @@ -514,7 +514,7 @@ (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?))))
> > >
> > > and sent a patch with that command:
> > >
> > > git send-email --to=guix-patches@gnu.org \
> > >   $(./etc/teams.scm cc-members origin/master HEAD 0000-cover-
> > > letter.patch
> > >
> > > It created https://issues.guix.gnu.org/58812 with it, but I don't
> > > see
> > > any of the X-Debbugs-Cc headers.  Mmmh.
> > Note that the existing etc/teams also assumes there are no funny
> > characters in the quote.  So it's susceptible to good ol' bobby
> > tables.
> >
> > Could we, instead of outputting a command, make it so that we can
> > pass
> > an already formatted patch and etc/teams rewrites it?
>
> The solution suggested to me in #bash would be to turn etc/teams.scm
> into a git wrapper, that could invoke git with all the arguments at
> once (allowing people to pass arguments themselves).  I'm told the
> git completion could be hooked to such script so that users can still
> enjoy git tab-completion when using etc/teams.scm, although I haven't
> researched how that'd all work.
>
> A similar idea proposed by selckin on #git would be to have our
> etc/teams.scm script receive the complete git command, and append the
> args to it itself (and invoke it), like so:
>
> ./etc/teams.scm cc mentors -- git send-email --to
> XXX@debbugs.gnu.org *.patch
I think we should go with the former and implement a proper "guix send-
email" ;)
M
M
Maxim Cournoyer wrote on 5 Jan 18:03 +0100
Re: Bogus ‘etc/teams.scm’ usage recommendations?
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
87wn60oqvj.fsf@gmail.com
+CC 58813@debbugs.gnu.org

Hi,

Simon Tournier <zimon.toutoune@gmail.com> writes:

Toggle quote (23 lines)
> Hi Ludo,
>
> On Tue, 03 Jan 2023 at 23:29, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> The manual recommends this (info "(guix) Teams"):
>>
>> git send-email --to ISSUE_NUMBER@debbugs.gnu.org $(./etc/teams.scm cc mentors) *.patch
>>
>> where:
>>
>> --8<---------------cut here---------------start------------->8---
>> λ ./etc/teams.scm cc mentors
>> --add-header="X-Debbugs-Cc: rg@raghavgururajan.name"
>> --add-header="X-Debbugs-Cc: zimon.toutoune@gmail.com" …
>> --8<---------------cut here---------------end--------------->8---
>>
>> I believe this cannot work because the shell will split words on each
>> whitespace; IOW, the double quotes above do not have the desired effect.

> Well, IIUC, this part is tracked by #58813 [1].
>
> 1: <http://issues.guix.gnu.org/issue/58813>

Indeed (CC'd).

I thought about not using whitespace in the generated output, but I'm
not sure if Debbugs or email clients in general would care, plus it's a
dirty fix.

With the recent patman integration merged (though do apply #60576 as a
fixup commit), I'm tempted to remove the mentions of git send-email
$(etc/teams.scm cc-members ...) and replace that by 'Further automation
of git send-email and etc/teams.scm is possible via the patman package'.

What do you think?

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 5 Jan 18:03 +0100
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
87tu14oqna.fsf@gmail.com
+CC 58813@debbugs.gnu.org

Hi,

Simon Tournier <zimon.toutoune@gmail.com> writes:

Toggle quote (23 lines)
> Hi Ludo,
>
> On Tue, 03 Jan 2023 at 23:29, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> The manual recommends this (info "(guix) Teams"):
>>
>> git send-email --to ISSUE_NUMBER@debbugs.gnu.org $(./etc/teams.scm cc mentors) *.patch
>>
>> where:
>>
>> --8<---------------cut here---------------start------------->8---
>> λ ./etc/teams.scm cc mentors
>> --add-header="X-Debbugs-Cc: rg@raghavgururajan.name"
>> --add-header="X-Debbugs-Cc: zimon.toutoune@gmail.com" …
>> --8<---------------cut here---------------end--------------->8---
>>
>> I believe this cannot work because the shell will split words on each
>> whitespace; IOW, the double quotes above do not have the desired effect.

> Well, IIUC, this part is tracked by #58813 [1].
>
> 1: <http://issues.guix.gnu.org/issue/58813>

Indeed (CC'd).

I thought about not using whitespace in the generated output, but I'm
not sure if Debbugs or email clients in general would care, plus it's a
dirty fix.

With the recent patman integration merged (though do apply #60576 as a
fixup commit), I'm tempted to remove the mentions of git send-email
$(etc/teams.scm cc-members ...) and replace that by 'Further automation
of git send-email and etc/teams.scm is possible via the patman package'.

What do you think?

--
Thanks,
Maxim
L
L
Ludovic Courtès wrote on 9 Jan 18:23 +0100
Re: bug#58813: can't substitute etc/teams.scm command as doc suggests
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87v8lfpqpb.fsf_-_@gnu.org
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (31 lines)
> Simon Tournier <zimon.toutoune@gmail.com> writes:
>
>> Hi Ludo,
>>
>> On Tue, 03 Jan 2023 at 23:29, Ludovic Courtès <ludo@gnu.org> wrote:
>>
>>> The manual recommends this (info "(guix) Teams"):
>>>
>>> git send-email --to ISSUE_NUMBER@debbugs.gnu.org $(./etc/teams.scm cc mentors) *.patch
>>>
>>> where:
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> λ ./etc/teams.scm cc mentors
>>> --add-header="X-Debbugs-Cc: rg@raghavgururajan.name"
>>> --add-header="X-Debbugs-Cc: zimon.toutoune@gmail.com" …
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>> I believe this cannot work because the shell will split words on each
>>> whitespace; IOW, the double quotes above do not have the desired effect.
>
>> Well, IIUC, this part is tracked by #58813 [1].
>>
>> 1: <http://issues.guix.gnu.org/issue/58813>
>
> Indeed (CC'd).
>
> I thought about not using whitespace in the generated output, but I'm
> not sure if Debbugs or email clients in general would care, plus it's a
> dirty fix.

Right.

How about just outputting a line like:

X-Debbugs-Cc: maxim@example.org, ludo@example.org

that people would paste in their cover letter?

How do Linux’s scripts work?

Toggle quote (7 lines)
> With the recent patman integration merged (though do apply #60576 as a
> fixup commit), I'm tempted to remove the mentions of git send-email
> $(etc/teams.scm cc-members ...) and replace that by 'Further automation
> of git send-email and etc/teams.scm is possible via the patman package'.
>
> What do you think?

This is the first time I hear about patman. :-)

The “Submitting Patches” section mentions ‘git send-email’; I don’t
think this is about to change, is it?

Thanks,
Ludo’.
M
M
Maxim Cournoyer wrote on 9 Jan 21:52 +0100
(name . Ludovic Courtès)(address . ludo@gnu.org)
87358jmnwj.fsf@gmail.com
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (43 lines)
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Simon Tournier <zimon.toutoune@gmail.com> writes:
>>
>>> Hi Ludo,
>>>
>>> On Tue, 03 Jan 2023 at 23:29, Ludovic Courtès <ludo@gnu.org> wrote:
>>>
>>>> The manual recommends this (info "(guix) Teams"):
>>>>
>>>> git send-email --to ISSUE_NUMBER@debbugs.gnu.org $(./etc/teams.scm cc mentors) *.patch
>>>>
>>>> where:
>>>>
>>>> --8<---------------cut here---------------start------------->8---
>>>> λ ./etc/teams.scm cc mentors
>>>> --add-header="X-Debbugs-Cc: rg@raghavgururajan.name"
>>>> --add-header="X-Debbugs-Cc: zimon.toutoune@gmail.com" …
>>>> --8<---------------cut here---------------end--------------->8---
>>>>
>>>> I believe this cannot work because the shell will split words on each
>>>> whitespace; IOW, the double quotes above do not have the desired effect.
>>
>>> Well, IIUC, this part is tracked by #58813 [1].
>>>
>>> 1: <http://issues.guix.gnu.org/issue/58813>
>>
>> Indeed (CC'd).
>>
>> I thought about not using whitespace in the generated output, but I'm
>> not sure if Debbugs or email clients in general would care, plus it's a
>> dirty fix.
>
> Right.
>
> How about just outputting a line like:
>
> X-Debbugs-Cc: maxim@example.org, ludo@example.org
>
> that people would paste in their cover letter?

Yes, that's better.

Toggle quote (2 lines)
> How do Linux’s scripts work?

I think the scripts just print stuff at the terminal and expect the user
to copy paste. Patman can be used to provide automation on top of that.

Toggle quote (12 lines)
>> With the recent patman integration merged (though do apply #60576 as a
>> fixup commit), I'm tempted to remove the mentions of git send-email
>> $(etc/teams.scm cc-members ...) and replace that by 'Further automation
>> of git send-email and etc/teams.scm is possible via the patman package'.
>>
>> What do you think?
>
> This is the first time I hear about patman. :-)
>
> The “Submitting Patches” section mentions ‘git send-email’; I don’t
> think this is about to change, is it?

It wouldn't change; patman would be hinted at briefly with a reference
to its documentation (info '(u-boot) Patman patch manager' from the
u-boot-documentation package) as a nice way to stay organize with
submissions and automate a few things on top of git send-email.

--
Thanks,
Maxim
S
S
Simon Tournier wrote on 11 Jan 16:20 +0100
865ydd5c9q.fsf@gmail.com
Hi,

On Mon, 09 Jan 2023 at 15:52, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

Toggle quote (5 lines)
> It wouldn't change; patman would be hinted at briefly with a reference
> to its documentation (info '(u-boot) Patman patch manager' from the
> u-boot-documentation package) as a nice way to stay organize with
> submissions and automate a few things on top of git send-email.

Is it possible to read online patman documentation? I am not able to
find one. Moreover, it could ease the adoption if a minimal sample of
such configuration is provided. A minimal out of the box configuration
as starter.

On my side, if I have to do more than just click to read documentation,
then I give up. And then, if I have to parse lengthy documentation,
then it depends on my motivation but it is also possible that I give
up–the well-known RTFM trap. In other words, I am lazy. :-)

Cheers,
simon
M
M
Maxim Cournoyer wrote on 12 Jan 04:00 +0100
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
87fscgh2xm.fsf@gmail.com
Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

Toggle quote (12 lines)
> Hi,
>
> On Mon, 09 Jan 2023 at 15:52, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>> It wouldn't change; patman would be hinted at briefly with a reference
>> to its documentation (info '(u-boot) Patman patch manager' from the
>> u-boot-documentation package) as a nice way to stay organize with
>> submissions and automate a few things on top of git send-email.
>
> Is it possible to read online patman documentation? I am not able to
> find one.

It's developed as part of U-Boot and available at
generated from the same sources as the info manual from
u-boot-documentation referenced above.

Toggle quote (4 lines)
> Moreover, it could ease the adoption if a minimal sample of
> such configuration is provided. A minimal out of the box configuration
> as starter.

No configuration is required other than the .patman file already checked
in Guix.

Toggle quote (5 lines)
> On my side, if I have to do more than just click to read documentation,
> then I give up. And then, if I have to parse lengthy documentation,
> then it depends on my motivation but it is also possible that I give
> up–the well-known RTFM trap. In other words, I am lazy. :-)

The 'Patman patch manager' section is relatively compact; I read it from
the comfort of Emacs ;-). There's also some alternative short text
available as 'patman -H' if you are in a hurry.

--
Thanks,
Maxim
S
S
Simon Tournier wrote on 12 Jan 14:31 +0100
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87fscfyj52.fsf@gmail.com
Hi Maxim,

On mer., 11 janv. 2023 at 22:00, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

Toggle quote (5 lines)
> It's developed as part of U-Boot and available at
> https://u-boot.readthedocs.io/en/latest/develop/patman.html. It's
> generated from the same sources as the info manual from
> u-boot-documentation referenced above.

Oh thanks! I do not know why I did not found it when using my favorite
search engine. :-)

Toggle quote (9 lines)
>> On my side, if I have to do more than just click to read documentation,
>> then I give up. And then, if I have to parse lengthy documentation,
>> then it depends on my motivation but it is also possible that I give
>> up–the well-known RTFM trap. In other words, I am lazy. :-)
>
> The 'Patman patch manager' section is relatively compact; I read it from
> the comfort of Emacs ;-). There's also some alternative short text
> available as 'patman -H' if you are in a hurry.

Well, I still think that examples about how to use it are worth and will
help for adoption. :-)

For instance, this section of the manual [1] describes how to use
git-format-patch and git-send-email with details applied to Guix
examples. It appears to me better than just pointing [2] and [3]. ;-)



Cheers,
simon
?