[bug#72316,v2,1/3] Add guile-pam.

Message ID af48747df906f446d97ae6da1156cf8ad17491de.1746104902.git.felix.lechner@lease-up.com
State New
Headers
Series [bug#72316,v2,1/3] Add guile-pam. |

Commit Message

Felix Lechner May 1, 2025, 1:42 p.m. UTC
  Change-Id: I991ca32c8696de0e6751b0f4225bf24151ba22f2
---
 gnu/packages/linux.scm | 71 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)
  

Comments

Maxim Cournoyer May 1, 2025, 2:04 p.m. UTC | #1
Hi Felix,

Felix Lechner <felix.lechner@lease-up.com> writes:

[...]

> +(define-public guile-pam
> +  (let ((commit "5ea70a5d88e7ade27ba9f231acab7d363b6400fb")
> +        (revision "0"))
> +    (package
> +      (name "guile-pam")
> +      (version (git-version "0.0" revision commit))
> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url "https://codeberg.org/lechner/guile-pam")
> +                      (commit commit)))
> +                (file-name (git-file-name name version))
> +                (sha256
> +                 (base32
> +                  "1i034f42wnmnsz76pcniif2ikpbamh0cki3ib2zwmbnvif4656av"))))
> +      (native-inputs (list
> +                      autoconf
> +                      automake
> +                      gnulib
> +                      guile-3.0
> +                      libtool
> +                      linux-pam
> +                      nyacc-2.01
> +                      pkg-config
> +                      texinfo))
> +      (inputs (list
> +               guile-3.0
> +               linux-pam))
> +      (propagated-inputs (list
> +                          nyacc-2.01))
> +      (build-system gnu-build-system)
> +      (arguments
> +       (list
> +        #:make-flags
> +        #~(list (string-append "ENTRY_POINT_DIR=" #$output "/share/entry-points"))
> +        #:phases
> +        #~(modify-phases %standard-phases
> +            (add-after 'unpack 'install-gnulib
> +              ;; per https://lists.gnu.org/archive/html/guile-devel/2012-08/msg00042.html
> +              (lambda* (#:key inputs #:allow-other-keys)
> +                (let ((gnulib-build-aux (dirname
> +                                         (search-input-file inputs
> +                                                            "/src/gnulib/build-aux/config.rpath"))))

It seems more conventional to avoid the leading '/' on the file name
using `search-input-file'.

> +                  (mkdir-p "build-aux")
> +                  (copy-recursively gnulib-build-aux "build-aux"))
> +                (let ((gnulib-m4 (dirname (search-input-file inputs
> +                                                             "/src/gnulib/m4/lib-link.m4"))))
> +                  (mkdir-p "m4")
> +                  (copy-recursively gnulib-m4 "m4"))))
> +            (add-after 'patch-source-shebangs 'fix-paths
> +              (lambda* (#:key inputs #:allow-other-keys)
> +                (for-each (lambda (file)
> +                            (substitute* file
> +                              (("/usr/bin/env -S guile ")
> +                               (string-append (search-input-file inputs "/bin/guile") " \\\n"))))
> +                          '("test/legacy-control-strings"))

Perhaps not prominently mentioned enough, but our coding conventions
(info "(guix) Formatting Code") suggests to keep the maximum line width
<= 80 characters (it's in the linked style.txt document, and also in
.editorconfig, and our .dir-locals.el sets the fill-column to 78, as
some extra hints)

> +                (substitute* "scm/pam.scm"
> +                  (("[.]/wrap/c/[.]libs/conversation.so")
> +                   (string-append #$output "/lib/guile-pam/wrapper/conversation.so"))))))))
> +      (home-page "https://codeberg.org/lechner/guile-pam")
> +      (synopsis "Write your Linux-PAM authentication logic in Guile Scheme")
> +      (description
> +       "Guile-PAM provides a way to rewrite your authentication logic in the
> +Linux PAM (pluggable authentication modules) in Guile Scheme. It should make
> +those modules more transparent to the administrator and more intuitive to
> +use.")

Sentences should be separated by two spaces in every doc/text of the
source, as another convention.

Other than these nitpicks, LGTM!  Could you please send a v3 with my
small suggestions?
  
Maxim Cournoyer May 1, 2025, 2:30 p.m. UTC | #2
Hi,

Felix Lechner <felix.lechner@lease-up.com> writes:

> Change-Id: I991ca32c8696de0e6751b0f4225bf24151ba22f2

I forgot, you need a GNU ChangeLog message for your commit message (that
holds for any commit to be merged into Guix).  You can use Magit 'C' on
the git diffs while editing a commit message to automate some of it.
  
Z572 May 1, 2025, 3:14 p.m. UTC | #3
Felix Lechner via Guix-patches via <guix-patches@gnu.org> writes:

> Change-Id: I991ca32c8696de0e6751b0f4225bf24151ba22f2
> ---
>  gnu/packages/linux.scm | 71 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index b4adf0c2b43..35ae4558043 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -113,6 +113,7 @@ (define-module (gnu packages linux)
>    #:use-module (gnu packages bash)
>    #:use-module (gnu packages bison)
>    #:use-module (gnu packages boost)
> +  #:use-module (gnu packages build-tools)
>    #:use-module (gnu packages calendar)
>    #:use-module (gnu packages check)
>    #:use-module (gnu packages cpio)
> @@ -145,6 +146,7 @@ (define-module (gnu packages linux)
>    #:use-module (gnu packages graphviz)
>    #:use-module (gnu packages gstreamer)
>    #:use-module (gnu packages gtk)
> +  #:use-module (gnu packages guile)
>    #:use-module (gnu packages haskell-apps)
>    #:use-module (gnu packages haskell-xyz)
>    #:use-module (gnu packages image)
> @@ -158,6 +160,7 @@ (define-module (gnu packages linux)
>    #:use-module (gnu packages m4)
>    #:use-module (gnu packages man)
>    #:use-module (gnu packages maths)
> +  #:use-module (gnu packages mes)
>    #:use-module (gnu packages multiprecision)
>    #:use-module (gnu packages ncurses)
>    #:use-module (gnu packages netpbm)
> @@ -2412,6 +2415,74 @@ (define-public vendor-reset-linux-module
>  ;;; Pluggable authentication modules (PAM).
>  ;;;
>  
> +(define-public guile-pam
> +  (let ((commit "5ea70a5d88e7ade27ba9f231acab7d363b6400fb")
> +        (revision "0"))
> +    (package
> +      (name "guile-pam")
> +      (version (git-version "0.0" revision commit))

i think should (git-version "0.0.1" revision commit)

because this commit is after v0.0.1.

> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url "https://codeberg.org/lechner/guile-pam")
> +                      (commit commit)))
> +                (file-name (git-file-name name version))
> +                (sha256
> +                 (base32
> +                  "1i034f42wnmnsz76pcniif2ikpbamh0cki3ib2zwmbnvif4656av"))))
> +      (native-inputs (list
> +                      autoconf
> +                      automake
> +                      gnulib
> +                      guile-3.0
> +                      libtool
> +                      linux-pam

why linux-pam both in native-inputs and inputs, is it required when
cross compiling?

> +                      nyacc-2.01

Please add a comment why we can't just use nyacc.

> +                      pkg-config
> +                      texinfo))
> +      (inputs (list
> +               guile-3.0
> +               linux-pam))
> +      (propagated-inputs (list
> +                          nyacc-2.01))
> +      (build-system gnu-build-system)
> +      (arguments
> +       (list
> +        #:make-flags
> +        #~(list (string-append "ENTRY_POINT_DIR=" #$output "/share/entry-points"))
> +        #:phases
> +        #~(modify-phases %standard-phases
> +            (add-after 'unpack 'install-gnulib
> +              ;; per https://lists.gnu.org/archive/html/guile-devel/2012-08/msg00042.html
> +              (lambda* (#:key inputs #:allow-other-keys)
> +                (let ((gnulib-build-aux (dirname
> +                                         (search-input-file inputs
> +                                                            "/src/gnulib/build-aux/config.rpath"))))
> +                  (mkdir-p "build-aux")
> +                  (copy-recursively gnulib-build-aux "build-aux"))
> +                (let ((gnulib-m4 (dirname (search-input-file inputs
> +                                                             "/src/gnulib/m4/lib-link.m4"))))
> +                  (mkdir-p "m4")
> +                  (copy-recursively gnulib-m4 "m4"))))
> +            (add-after 'patch-source-shebangs 'fix-paths
> +              (lambda* (#:key inputs #:allow-other-keys)
> +                (for-each (lambda (file)
> +                            (substitute* file
> +                              (("/usr/bin/env -S guile ")
> +                               (string-append (search-input-file inputs "/bin/guile") " \\\n"))))
> +                          '("test/legacy-control-strings"))
> +                (substitute* "scm/pam.scm"
> +                  (("[.]/wrap/c/[.]libs/conversation.so")
> +                   (string-append #$output "/lib/guile-pam/wrapper/conversation.so"))))))))
> +      (home-page "https://codeberg.org/lechner/guile-pam")
> +      (synopsis "Write your Linux-PAM authentication logic in Guile Scheme")
> +      (description
> +       "Guile-PAM provides a way to rewrite your authentication logic in the
> +Linux PAM (pluggable authentication modules) in Guile Scheme. It should make
> +those modules more transparent to the administrator and more intuitive to
> +use.")
> +      (license license:gpl3+))))
> +
>  (define-public linux-pam
>    (package
>      (name "linux-pam")
  
Felix Lechner May 11, 2025, 5:16 p.m. UTC | #4
Hi Maxim,

On Thu, May 01 2025, Maxim Cournoyer wrote:

> you need a GNU ChangeLog message

A ChangeLog message that's acceptable to you, together with the other
adjustments you suggested, fall in my book within the responsibilities
of the GNU Guix maintainers.

Z572's comments have merit; I will address them in short order.  Please
just allow me to state that the package definition is useful on its own.

Patch one provides the shared object pam_guile.so that allows users to
deploy PAM modules written in GNU Guile.  People have tried for decades
to bring interpreted languages to PAM.  Since GNU Guile is also the
configuration language for Guix System, I believe there is potentially a
benefit for your users.

For details, please have a look at the upstream documentation. [1]

I am Guile-PAM's upstream maintainer and hope to provide comprehensive
and timely assistance over there.

Kind regards
Felix

[1] https://juix.org/guile-pam/#From-PAM-to-Guile
  
Felix Lechner May 11, 2025, 5:36 p.m. UTC | #5
Hi Z572,

On Thu, May 01 2025, Z572 wrote:

> Felix Lechner via Guix-patches via <guix-patches@gnu.org> writes:
>
> this commit is after v0.0.1.

Nice catch, thank you!  I created the tag v0.0.3 as an alias for commit
5ea70a5d.  Feel free to use either.

> why linux-pam both in native-inputs and inputs, is it required when
> cross compiling?

I cannot remember, and do not understand the function of native-inputs.

> Please add a comment why we can't just use nyacc.

Matt Wesche made changes and bug fixes on my behalf.  Most notable is
the new Cdata interface [1] which replaced Guile-Bytestructures. [2]

[1] https://www.nongnu.org/nyacc/cdata.html
[2] https://www.nongnu.org/nyacc/nyacc-fh-ug.html

Kind regards
Felix
  
Maxim Cournoyer May 12, 2025, 7:38 a.m. UTC | #6
Hi Felix,

Felix Lechner <felix.lechner@lease-up.com> writes:

> Hi Maxim,
>
> On Thu, May 01 2025, Maxim Cournoyer wrote:
>
>> you need a GNU ChangeLog message
>
> A ChangeLog message that's acceptable to you, together with the other
> adjustments you suggested, fall in my book within the responsibilities
> of the GNU Guix maintainers.

I think you meant s/GNU Guix maintainers/committers/.  I'd just say that
it's more efficient if everybody does their part and strive to meet the
standards rather than expect someone to fix it up.  By leaving things
like the GNU ChangeLog for someone else to write, you lower the chances
that the change gets merged quickly, since it becomes more bothersome
for someone else to fill the gaps themselves before pushing.
  
Felix Lechner May 13, 2025, 2:50 a.m. UTC | #7
Hi Maxim,

On Mon, May 12 2025, Maxim Cournoyer wrote:

> I think you meant s/GNU Guix maintainers/committers/.

Well, I meant maintainers in the common sense. [1]

[1] https://en.wikipedia.org/wiki/Software_maintainer

> it's more efficient if everybody does their part

It may be more efficient for you, but it's not more efficient for me.

As an aside, I maintained a project with many contributors (Debian's
Lintian) and didn't nitpick.  I adjusted commits to my liking, credited
authorship, and accepted contributions because they were beneficial to
my project.  That's also how I will treat you when you come to
Guile-PAM.

> By leaving things like the GNU ChangeLog for someone else to write,
> you lower the chances that the change gets merged quickly

Could I see your analysis, please?  From my experience, a proper commit
message does nothing to make acceptance in GNU Guix any faster.

> it becomes more bothersome for someone else to fill the gaps
> themselves before pushing.

Okay, so you find tasks bothersome that I think are part of a
maintainer's role.

I think it would have been more productive than this exchange if you had
simply added the missing space yourself and made other needed changes
that your trained eyes, which know Guix much better than I, saw in
seconds.  How long does it take to type a space and write a one-line
commit message you like?

Kind regards,
Felix
  

Patch

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index b4adf0c2b43..35ae4558043 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -113,6 +113,7 @@  (define-module (gnu packages linux)
   #:use-module (gnu packages bash)
   #:use-module (gnu packages bison)
   #:use-module (gnu packages boost)
+  #:use-module (gnu packages build-tools)
   #:use-module (gnu packages calendar)
   #:use-module (gnu packages check)
   #:use-module (gnu packages cpio)
@@ -145,6 +146,7 @@  (define-module (gnu packages linux)
   #:use-module (gnu packages graphviz)
   #:use-module (gnu packages gstreamer)
   #:use-module (gnu packages gtk)
+  #:use-module (gnu packages guile)
   #:use-module (gnu packages haskell-apps)
   #:use-module (gnu packages haskell-xyz)
   #:use-module (gnu packages image)
@@ -158,6 +160,7 @@  (define-module (gnu packages linux)
   #:use-module (gnu packages m4)
   #:use-module (gnu packages man)
   #:use-module (gnu packages maths)
+  #:use-module (gnu packages mes)
   #:use-module (gnu packages multiprecision)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages netpbm)
@@ -2412,6 +2415,74 @@  (define-public vendor-reset-linux-module
 ;;; Pluggable authentication modules (PAM).
 ;;;
 
+(define-public guile-pam
+  (let ((commit "5ea70a5d88e7ade27ba9f231acab7d363b6400fb")
+        (revision "0"))
+    (package
+      (name "guile-pam")
+      (version (git-version "0.0" revision commit))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://codeberg.org/lechner/guile-pam")
+                      (commit commit)))
+                (file-name (git-file-name name version))
+                (sha256
+                 (base32
+                  "1i034f42wnmnsz76pcniif2ikpbamh0cki3ib2zwmbnvif4656av"))))
+      (native-inputs (list
+                      autoconf
+                      automake
+                      gnulib
+                      guile-3.0
+                      libtool
+                      linux-pam
+                      nyacc-2.01
+                      pkg-config
+                      texinfo))
+      (inputs (list
+               guile-3.0
+               linux-pam))
+      (propagated-inputs (list
+                          nyacc-2.01))
+      (build-system gnu-build-system)
+      (arguments
+       (list
+        #:make-flags
+        #~(list (string-append "ENTRY_POINT_DIR=" #$output "/share/entry-points"))
+        #:phases
+        #~(modify-phases %standard-phases
+            (add-after 'unpack 'install-gnulib
+              ;; per https://lists.gnu.org/archive/html/guile-devel/2012-08/msg00042.html
+              (lambda* (#:key inputs #:allow-other-keys)
+                (let ((gnulib-build-aux (dirname
+                                         (search-input-file inputs
+                                                            "/src/gnulib/build-aux/config.rpath"))))
+                  (mkdir-p "build-aux")
+                  (copy-recursively gnulib-build-aux "build-aux"))
+                (let ((gnulib-m4 (dirname (search-input-file inputs
+                                                             "/src/gnulib/m4/lib-link.m4"))))
+                  (mkdir-p "m4")
+                  (copy-recursively gnulib-m4 "m4"))))
+            (add-after 'patch-source-shebangs 'fix-paths
+              (lambda* (#:key inputs #:allow-other-keys)
+                (for-each (lambda (file)
+                            (substitute* file
+                              (("/usr/bin/env -S guile ")
+                               (string-append (search-input-file inputs "/bin/guile") " \\\n"))))
+                          '("test/legacy-control-strings"))
+                (substitute* "scm/pam.scm"
+                  (("[.]/wrap/c/[.]libs/conversation.so")
+                   (string-append #$output "/lib/guile-pam/wrapper/conversation.so"))))))))
+      (home-page "https://codeberg.org/lechner/guile-pam")
+      (synopsis "Write your Linux-PAM authentication logic in Guile Scheme")
+      (description
+       "Guile-PAM provides a way to rewrite your authentication logic in the
+Linux PAM (pluggable authentication modules) in Guile Scheme. It should make
+those modules more transparent to the administrator and more intuitive to
+use.")
+      (license license:gpl3+))))
+
 (define-public linux-pam
   (package
     (name "linux-pam")