Subject: Re: [PATCH v2 1/3] Add guile-pam.

From: Maxim Cournoyer <[email-masked]>
Date: Thu, 01 May 2025 23:04:01 +0900

Hi Felix,

Felix Lechner <[email-masked]> 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?

-- 
Thanks,
Maxim