[PATCH] Upgrade nyacc to 1.07.0

  • Done
  • quality assurance status badge
Details
3 participants
  • Josselin Poiret
  • Felix Lechner
  • Ludovic Courtès
Owner
unassigned
Submitted by
Felix Lechner
Severity
normal
F
F
Felix Lechner wrote on 2 Jun 2022 14:14
(address . guix-patches@gnu.org)
CAFHYt5559tBrB_yM0z3q-FhE6anAXiu7w9LHqbbT0Jnm5j=sUA@mail.gmail.com
Control: tags -1 + patch

Hi,

For my work in making PAM configurable in Guile, I rely on the foreign
function interface (FFI). The automatic generation of that interface
requires the compile-ffi script in nyacc. Unfortunately, the highest
version we ship suffers from a bug that prevents the script from
running on the pam_client.h header in linux-pam. This patch upgrades
nyacc to the version Matt Wette released after committing the fix.

The changes are further documented in the commit message.

I am new to Guix and already use the new version of linux-pam locally
(via Guix System) but I was unable to test the patch according to the
requirements and recommendations listed for this submission. [1]

This email was sent to -devel in lieu of -mentors, which does not
exist. Any guidance is much appreciated. Thanks for taking a look!

KInd regards
Felix Lechner

Attachment: 0001-Upgrade-nyacc-to-1.07.0.patch
J
J
Josselin Poiret wrote on 4 Jun 2022 18:17
87leucwggu.fsf@jpoiret.xyz
Hello Felix,

Felix Lechner <felix.lechner@gmail.com> writes:

Toggle quote (25 lines)
> Control: tags -1 + patch
>
> Hi,
>
> For my work in making PAM configurable in Guile, I rely on the foreign
> function interface (FFI). The automatic generation of that interface
> requires the compile-ffi script in nyacc. Unfortunately, the highest
> version we ship suffers from a bug that prevents the script from
> running on the pam_client.h header in linux-pam. This patch upgrades
> nyacc to the version Matt Wette released after committing the fix.
>
> The changes are further documented in the commit message.
>
> I am new to Guix and already use the new version of linux-pam locally
> (via Guix System) but I was unable to test the patch according to the
> requirements and recommendations listed for this submission. [1]
>
> This email was sent to -devel in lieu of -mentors, which does not
> exist. Any guidance is much appreciated. Thanks for taking a look!
>
> KInd regards
> Felix Lechner
>
> [1] https://guix.gnu.org/en/manual/devel/en/html_node/Submitting-Patches.html

While the contents of the patch look good, you're missing the standard
ChangeLog format for the commit message, see "(standards)Change Logs".

I would recommend something along

Toggle snippet (5 lines)
[Your message]

* gnu/packages/mes.scm (nyacc): Update to 1.07.0.

Also, I think that inline patches are preferred, even though the manual
says that MIME is fine too. The usual advice is to just use
git-send-email to send patch series, which will take care of everything
itself (you can check out a tutorial at [1]). You could send a v2
patch, or maybe a maintainer will add the necessary format themselves
and commit, whichever comes first :).

In any case, thanks for your contribution!


Best,
--
Josselin Poiret
L
L
Ludovic Courtès wrote on 7 Jun 2022 18:22
Re: bug#55763: [PATCH] Upgrade nyacc to 1.07.0
(name . Felix Lechner)(address . felix.lechner@gmail.com)(address . 55763-done@debbugs.gnu.org)
878rq8a1es.fsf@gnu.org
Hello,

Felix Lechner <felix.lechner@gmail.com> skribis:

Toggle quote (16 lines)
> For my work in making PAM configurable in Guile, I rely on the foreign
> function interface (FFI). The automatic generation of that interface
> requires the compile-ffi script in nyacc. Unfortunately, the highest
> version we ship suffers from a bug that prevents the script from
> running on the pam_client.h header in linux-pam. This patch upgrades
> nyacc to the version Matt Wette released after committing the fix.
>
> The changes are further documented in the commit message.
>
> I am new to Guix and already use the new version of linux-pam locally
> (via Guix System) but I was unable to test the patch according to the
> requirements and recommendations listed for this submission. [1]
>
> This email was sent to -devel in lieu of -mentors, which does not
> exist. Any guidance is much appreciated. Thanks for taking a look!

The patch is perfect; I only adjusted the commit log so that it follows
the ChangeLog convention, as Josselin pointed out:


I also check that dependents, as reported by ‘guix refresh -l nyacc’,
all build fine with the new version.

Thanks,
Ludo’.

PS: guix-mentors now exists!
Closed
?