GNU bug report logs - #61744
[PATCH] services: base: Deprecate 'pam-limits-service' procedure.

Previous Next

Package: guix-patches;

Reported by: Bruno Victal <mirai <at> makinata.eu>

Date: Fri, 24 Feb 2023 00:13:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 61744 <at> debbugs.gnu.org
Cc: mirai <at> makinata.eu
Subject: [bug#61744] [PATCH v2 1/2] services: base: Deprecate 'pam-limits-service' procedure.
Date: Fri, 10 Mar 2023 18:52:43 +0100
Hi,

thank you for the patches!

The effective change looks fine to me, but I’m confused about why these
are two patches.  The first one introduces this as an example in the
docs:

+(service
+  pam-limits-service-type
+  (plain-file
+    "limits.conf"
+    (string-join
+      (map pam-limits-entry->string
+        (list (pam-limits-entry "@@realtime" 'both 'rtprio 99)
+              (pam-limits-entry "@@realtime" 'both 'memlock 'unlimited)))
+      "\n")))

But the second removes this again in favour of this prettier form:

+(service pam-limits-service-type
+         (list
+          (pam-limits-entry "@@realtime" 'both 'rtprio 99)
+          (pam-limits-entry "@@realtime" 'both 'memlock 'unlimited)))

Which is really close to the original form:

-(pam-limits-service
- (list
-  (pam-limits-entry "@@realtime" 'both 'rtprio 99)
-  (pam-limits-entry "@@realtime" 'both 'memlock 'unlimited)))

Could you merge these two patches to reduce the number of unnecessary
changes?  I don’t think we should change to file-likes as the argument
value for the pam-limits-service-type.

Another thing that confused me:

+            (test-equal "/etc/security/limits.conf content matches"
+              #$(string-join (map pam-limits-entry->string pam-limit-entries)
+                             "\n" 'suffix)
+              (marionette-eval
+               '(call-with-input-file "/etc/security/limits.conf"
+                  get-string-all)
+               marionette))

Why use the gexp with a computed value here instead of using just the
plain text of the expected contents of that file?  Computing
the expected value in a test where the compared value is computed in the
same way feels like begging the question.

Or perhaps I’m misunderstanding something here?

-- 
Ricardo




This bug report was last modified 2 years and 52 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.