From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [V2 PATCH 1/1] services: Add agetty service. Date: Fri, 17 Feb 2017 08:48:28 +0100 Message-ID: <87tw7t8dgz.fsf@elephly.net> References: <8b9a83141665a7a86aa3d3c9ba6363c1ba2e93cd.1487117562.git.leo@famulari.name> <20170215002417.GA19546@jasmine> <20170216070650.GA21674@jasmine> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:36874) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cedHT-0001YO-V8 for guix-devel@gnu.org; Fri, 17 Feb 2017 02:48:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cedHO-0005Mq-TV for guix-devel@gnu.org; Fri, 17 Feb 2017 02:48:40 -0500 Received: from sender-of-o51.zoho.com ([135.84.80.216]:21018) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cedHO-0005Lt-Lv for guix-devel@gnu.org; Fri, 17 Feb 2017 02:48:34 -0500 In-reply-to: <20170216070650.GA21674@jasmine> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Leo Famulari Cc: guix-devel@gnu.org Leo Famulari writes: > On Tue, Feb 14, 2017 at 07:24:17PM -0500, Leo Famulari wrote: >> This 'extra' is a time-saving kludge. I'll add fields for all of >> agetty's configuration options once I'm satisfied that the service works >> on GuixSD. > > Here's a patch that exposes (almost all) of agetty's command-line > options. > > Is this the right way? Or would we rather wrap only the most > commonly-used options, and leave an "escape hatch" as in the first > version of the patch? If so, which options should we expose in Scheme? That’s hard to say. I like to *always* keep an escape hatch, which is useful in case we are too slow to adapt our code to changes introduced by new versions. In my opinion this version of the patch is better because it exposes more values; it would be even better if it added a general purpose escape hatch. > I'll wait for feedback before writing the documentation. Okay. (Can we generate documentation for the individual fields somehow?) Some naive comments follow. > From 215ad705a933fda1170a5883277cd9a68db693e0 Mon Sep 17 00:00:00 2001 > From: Leo Famulari > Date: Tue, 14 Feb 2017 11:28:04 -0500 > Subject: [PATCH] services: Add agetty service. > > * gnu/services/base.scm (): New record type. > (agetty-shepherd-service, agetty-service): New procedures. > (agetty-service-type): New variable. > --- […] > +(define-record-type* > + agetty-configuration make-agetty-configuration > + agetty-configuration? > + (agetty agetty-configuration-agetty ; > + (default util-linux)) > + (tty agetty-configuration-tty) ;string > + (term agetty-term ;string > + (default #f)) > + (baud-rate agetty-baud-rate ;string > + (default #f)) > + (auto-login agetty-auto-login ;string > + (default #f)) > + (login-program agetty-login-program ;gexp > + (default (file-append shadow "/bin/login"))) > + (login-pause? agetty-login-pause? ;Boolean > + (default #f)) > + (eight-bits? agetty-eight-bits? ;Boolean > + (default #f)) > + (no-reset? agetty-no-reset? ;Boolean > + (default #f)) > + (remote? agetty-remote? ;Boolean > + (default #f)) > + (flow-control? agetty-flow-control? ;Boolean > + (default #f)) > + (host agetty-host ;string > + (default #f)) > + (no-issue? agetty-no-issue? ;Boolean > + (default #f)) > + (init-string agetty-init-string ;string > + (default #f)) > + (no-clear? agetty-no-clear? ;Boolean > + (default #f)) > + (local-line agetty-local-line ;always | never | auto > + (default #f)) > + (extract-baud? agetty-extract-baud? ;Boolean > + (default #f)) > + (skip-login? agetty-skip-login? ;Boolean > + (default #f)) > + (no-newline? agetty-no-newline? ;Boolean > + (default #f)) > + (login-options agetty-login-options ;string > + (default #f)) > + (chroot agetty-chroot ;string > + (default #f)) > + (hangup? agetty-hangup? ;Boolean > + (default #f)) > + (timeout agetty-timeout ;integer > + (default #f)) > + (detect-case? agetty-detect-case? ;Boolean > + (default #f)) > + (wait-cr? agetty-wait-cr? ;Boolean > + (default #f)) > + (no-hints? agetty-no-hints? ;Boolean > + (default #f)) > + (no-hostname? agetty-no hostname? ;Boolean > + (default #f)) > + (long-hostname? agetty-long-hostname? ;Boolean > + (default #f)) > + (erase-chars agetty-erase-chars ;string > + (default #f)) > + (kill-chars agetty-kill-chars ;string > + (default #f)) > + (chdir agetty-chdir ;string > + (default #f)) > + (delay agetty-delay ;integer > + (default #f)) > + (nice agetty-nice ;integer > + (default #f)) > +;;; XXX Unimplemented for now! > +;;; (issue-file agetty-issue-file ;plain-file > +;;; (default #f)) > + ) > + > +(define agetty-shepherd-service > + (match-lambda > + (($ agetty tty term baud-rate auto-login > + login-program login-pause? eight-bits? no-reset? remote? flow-control? > + host no-issue? init-string no-clear? local-line extract-baud? > + skip-login? no-newline? login-options chroot hangup? timeout > + detect-case? wait-cr? no-hints? no-hostname? long-hostname? erase-chars > + kill-chars chdir delay nice) > + (list > + (shepherd-service > + (documentation "Run agetty on a tty.") > + (provision (list (symbol-append 'term- (string->symbol tty)))) > + > + ;; Same comment as for mingetty-shepherd-service. > + (requirement '(user-processes host-name udev)) > + > + (start #~(make-forkexec-constructor > + (list #$ (file-append util-linux "/sbin/agetty") > + #$@(if eight-bits? > + #~("--8bits") > + #~()) > + #$@(if no-reset? > + #~("--noreset") > + #~()) > + #$@(if remote? > + #~("--remote") > + #~()) Looking at all the syntactic noise makes me long for some prettier abstraction here, but I can’t think of anything that would make this considerably prettier. Oh well… ¯\_(ツ)_/¯ > + ;; This doesn't work as expected. According to > + ;; agetty(8), if this option is not passed, then the > + ;; default is 'auto'. However, in my tests, when that > + ;; option is selected, agetty never presents the login > + ;; prompt, and the term-ttyS0 service respawns every > + ;; few seconds. Maybe add a FIXME here, so we can more easily find it. It might also be worth opening a bug report after merging this, so that we can track it. > + #$@(if no-newline? > + #~("--nonewline") > + #~()) I’m sure this will trip me up in the future. “nonewline” vs “no-newline” – I prefer your choice, but the double negative is tough for me… > + #$@(if no-hostname? > + #~("--nohostname") > + #~()) > + #$@(if long-hostname? > + #~("--long-hostname") > + #~()) Should we try to prevent nonsensical combinations of options? Would it just be more confusing to merge “no-hostname?” and “long-hostname?” as “print-hostname 'none 'long 'short”? Everything else looks good to me! -- Ricardo GPG: BCA6 89B6 3655 3801 C3C6 2150 197A 5888 235F ACAC https://elephly.net