[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Patch] add keychain
From: |
James Richardson |
Subject: |
Re: [Patch] add keychain |
Date: |
Sat, 24 Sep 2016 13:24:56 -0400 |
User-agent: |
mu4e 0.9.16; emacs 24.5.1 |
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