[bug#72316] [PATCH 3/3] Add a guile-pam-module service.

[Felix Lechner]

Hi Florian,

On Mon, Jul 29 2024, pelzflorian (Florian Pelz) wrote:

> guile-pam docs reference guile-wtut, which presumably should be
> guile-tut without w.

Thank you for your review!

A baby has been typing extra letters.  The typo was fixed.  You were
credited in the commit message. [1]

> About this doc/guix.texi addition [...] it would be better giving one
> or two functional examples rather than only calling the (format)
> procedure.  This would showcase to the uninitiated what PAM can do and
> how it looks in Guile.

I personally think that it would turn off new readers.

Guix System configures PAM already.  Only people hoping to accomplish
something non-standard will look into Guile-PAM.  Unfortunately, those
readers have little in common.  That's why I illustrated the way
Guile-PAM works with a simple example.

You are now saying we should instead solve a specialist case, but I
believe that's likely to distract the diverse group of readers by
drawing too much attention to what the module does, as opposed to how
Guile-PAM works.

The example was supposed to draw readers to Guile-PAM's Texinfo manual,
which I mentioned nearby.  Should we strike the example instead?

> It is repetitive that foreign-library-path must be set now everywhere
> for non-guile pam modules.

The foreign-library-path only looks repetitive.  It is the absolute path
to each module.  The modules just happen to be in the same place.

Guix traditionally relied on a special feature in Linux-PAM: One can use
absolute paths but, as many long-timer Guixers know, that is likely to
cause stability issues.

Guile-PAM solves that issue for Guix by separating the load path so a
running process won't reload a newer version of the same shared object.
Since the change has a logic to it, I have trouble relating to your
observation that the load paths look repetitive.

Please note that the foreign-library-path isn't actually needed for
modules that ship with Linux-PAM.  The Linux-PAM load path is added by
default near the comment regarding "courtesy for historical usage" in
the patch.  It is being offered only for user customizations of the
operating-system record, however, and may go away.

The right thing is always list the load path for a module.  That is what
the patch does.

> Even though a foreign-library-path is not always needed, would it be
> better to always set it as default even when unneeded

As I hoped to explain above, the load path is always needed.  In my
estimation, is not better to offer a default even though I did so for
the time being in the interest of a smooth transition.

Ultimately, the matter rests with the Guix maintainers.  They will (or
will not) decide if, when, and how to offer Guile-PAM to their users.

Because Guile-PAM is a new and lightly tested package that strives to
become an integral part of every Guix system, the decision will likely
involve a lot more questions than the ones you and I are discussing in
this thread here.

At the same time, Guile-PAM is only 541 lines of code (in Scheme, not
counting the examples) so maybe someone will get around to taking a
look.

> then patch 2/3 “Switch to Guile-PAM.” could be dropped?

No, the patch does other things.  It switches all PAM configurations
from Linux-PAM to Guile-PAM.  The configured system will use Guile-PAM's
stack implementation.

Guile-PAM should be attractive to Guix for several reasons.  One is that
it may simplify Guix's existing PAM machinery, which is complex, because
the same things can be accomplished better with quoted S-expressions (or
G-expressions, depending on the context).

There are also philosophical considerations which I hope will encourage
Guix to adopt Guile-PAM.  The code is short, written in Scheme, and
licensed under the GPL.

> Disclaimer; I do not know PAM.  I may well be wrong.

No worries, please, and thanks again for your review.  Linux-PAM is
arcane and complicated.  I wrote Guile-PAM for you!

Kind regards
Felix

[1] 
https://codeberg.org/lechner/guile-pam/commit/2f0f20a0a44f7672bfd93470c0562d19eb8ec511