[bug#61744] [PATCH v2 1/2] services: base: Deprecate 'pam-limits-service' procedure.

[Ricardo Wurmus]

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