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