From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Richardson Subject: Re: [Patch] add keychain Date: Sat, 24 Sep 2016 13:24:56 -0400 Message-ID: <87eg49mcrb.fsf@thor.jamestechnotes.com> References: <87vaxo6spj.fsf@thor.jamestechnotes.com> <57E5050B.8030406@crazy-compilers.com> 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]:33753) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bnqhP-0000C5-Lh for guix-devel@gnu.org; Sat, 24 Sep 2016 13:25:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bnqhL-0003t4-Ap for guix-devel@gnu.org; Sat, 24 Sep 2016 13:25:14 -0400 Received: from gandalf.jamestechnotes.com ([2600:3c02::f03c:91ff:fe61:3885]:54890) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bnqhL-0003mR-6S for guix-devel@gnu.org; Sat, 24 Sep 2016 13:25:11 -0400 Received: from cpe-75-190-233-89.nc.res.rr.com ([75.190.233.89] helo=thor.jamestechnotes.com) by gandalf.jamestechnotes.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1bnqhC-0004G1-2i for guix-devel@gnu.org; Sat, 24 Sep 2016 13:25:02 -0400 Received: from localhost ([127.0.0.1] helo=thor.jamestechnotes.com) by thor.jamestechnotes.com with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1bnqh6-0006xB-Cu for guix-devel@gnu.org; Sat, 24 Sep 2016 13:24:56 -0400 In-reply-to: <57E5050B.8030406@crazy-compilers.com> 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: guix-devel@gnu.org Hartmut Goebel writes: Thank you for the feedback! Much appreciated! > Am 22.09.2016 um 14:10 schrieb James Richardson: >> * gnu/packages/keychain.scm: Add new file. > > I suggest putting this into some other file, e.g. crypto.scm or ssh.scm. > Otherwise we have a file for a single package. I wasn't sure of the policy. I will move it to one of these files. > >> * gnu/packages/keychain.scm: (keychain): New variable. > > If you stay with the separate file, this line is not needed. > >> + #:builder (begin >> + (use-modules (guix build utils)) >> + (let ((bin-dir (string-append %output "/bin")) >> + (man1-dir (string-append %output "/share/man/man1")) >> + (tar (string-append >> + (assoc-ref %build-inputs "tar") >> + "/bin/tar")) >> + (bzip2 (assoc-ref %build-inputs "bzip2")) > You may want to have a look at audio.scm:freepats for a bit shorter and > more obvious code. I will take a look. > >> + (mkdir-p bin-dir) >> + (mkdir-p man1-dir) >> + (setenv "PATH" (string-append bzip2 "/bin")) >> + (system* tar "xfv" source) > > Please unpack the source first, the code is more obvious then. >> + (copy-file (string-append ,name "-" >> + ,version "/keychain.1") > If you run this using "with-directory-excursion", the code is more > obvious and simpler. >> + (string-append man1-dir "/keychain.1")) >> + (copy-file (string-append ,name "-" >> + ,version "/keychain") >> + (string-append bin-dir "/keychain")))))) > > You can use install-file and save the make-p above. >> + (native-inputs `(("bzip2" ,bzip2) >> + ("tar" ,tar) I wasn't aware of of install-file. I will make these updates. > > Please warp like this: > > + (native-inputs > + `(("bzip2" ,bzip2) > + ("tar" ,tar) > > >> + ("source" ,source))) > > > No need for listing the source here. > I will work on the wording. >> + (synopsis "Key manager for OpenSSH") > > Not only for *open*ssh, but for other implementations, too. And für > GnuPG-Agent, Maybe even talk abpout the Agent in the synopsis. > > Keychains itself says it is a "agent manager". > >> + (description >> + "Keychain is an OpenSSH key manager, typically run from >> +~/.bash_profile. When keychain is run, it checks for a running ssh-agent, >> +otherwise it starts one. It saves the ssh-agent environment variables to >> +~/.keychain/$HOSTNAME-sh, so that subsequent logins and >> +non-interactive shells such as cron jobs can source the file and make >> +passwordless ssh connections. In addition, when keychain runs, it >> +verifies that the key files specified on the command-line are known to >> +ssh-agent, otherwise it loads them, prompting you for a password >> +if necessary.") > > > The text above is a details workflow description. For me the text form > the keychain.spec-file is more meaningful. Maybe you want to combine them? > > Keychain is a manager for OpenSSH, ssh.com, Sun SSH and GnuPG agents. > It acts as a front-end to the agents, allowing you to easily have one > long-running agent process per system, rather than per login session. > This reduces the number of times you need to enter your passphrase > from once per new login session to once every time your local machine > is rebooted. I'll try to have these changes in the next few days. Thank you for the review. -- James Richardson