[pacman-dev] [PATCH 1/5] pacman-key: keyring management tool

[Allan McRae]

On 16/09/10 13:29, Denis A. Altoé Falqueto wrote:
> On Wed, Aug 4, 2010 at 10:17 AM, Allan McRae<allan at archlinux.org>  wrote:
>> On 28/07/10 13:50, Denis A. Altoé Falqueto wrote:
>>>
>>> The script pacman-key will manage pacman's keyring. It imports, exports,
>>> fetches from keyservers, helps in the process of trusting and updates
>>> the trust database.
>>>
>>> Signed-off-by: Denis A. Altoé Falqueto<denisfalqueto at gmail.com>
>>
>> Hi Denis,
>>
>> I think it would be good for us to focus on getting this onto the gpg branch
>> and then move onto the other patches.  I do not think this requires massive
>> changes to be ready.
>
> Hi.
>
> Sorry for the delay again. Time is so short lately... It took way
> longer than I would like. But here I am again. I'll answer only the
> things that I would like to discuss further. The other points were
> implemented as advised by you.
>
>>> +prepare_homedir() {
>>> +       if [[ ! -d "${PACMAN_KEYRING_DIR}" ]] ; then
>>> +               mkdir -p "${PACMAN_KEYRING_DIR}"
>>> +               touch "${PACMAN_KEYRING_DIR}/secring.gpg"
>>> +               touch "${PACMAN_KEYRING_DIR}/pubring.gpg"
>>> +               chmod 700 "${PACMAN_KEYRING_DIR}"
>>> +               chmod 600 "${PACMAN_KEYRING_DIR}"/{sec,pub}ring.gpg
>>
>> We should just use:
>> install -dm700 ${PACMAN_KEYRING_DIR}
>> to create the directory with the right permissions.
>>
>> And should those files actually be part of the pacman package and so
>> guaranteed to be present.
>
> Yes, I believe the best place is pacman package. I removed the
> function, so we need to make sure the PKGBUILD for pacman creates the
> proper files and directory.
>

I will adjust the Makefile to ensure these directories get made.

>>> +update_trustdb() {
>>> +       ${GPG_PACMAN} --batch --check-trustdb
>>
>> Should we be using --update-trustdb?
>
>  From gpg's man page:
>
> "The processing is identical to that of --update-trustdb but it skips
> keys with a not yet defined "ownertrust"."
>
> I'm not sure what is the best option. update-trustdb may ask the user
> what is the ownertrust value for the keys gpg can't compute with the
> web of trust. check-trustdb ignores those keys. According to the man
> page, none of them are necessary. The values are computed correctly
> when importing keys. Should we drop that option? It would be still
> accessible through --adv command, if someone really needs it.

I think this is fine at the moment.

>>> +# Read GPGDIR from $CONFIG.
>>> +# The pattern is: any spaces or tabs, GPGDir, any spaces or tabs, equal
>>> sign
>>> +# and the rest of the line. The string is splitted after the first
>>> occurrence of =
>>> +GPGDIR=$(cat ${CONFIG} | awk '/^(\t| )*GPGDir(\t| )*=.*/ { print
>>> substr($0,index($0, "=")+1) }')
>>
>> cat a file to awk it...  yuck.  Also, we do not use awk anywhere else, so
>> grep followed by a bash substitution to remove "*=" from the start may be
>> better.
>
> I see. I googled about grep and tabs and some links showed that grep
> couldn't accept correctly the \t character, but I found now that the
> class [:blank:] is what we really need: spaces and tabs. And grep
> accepts it. I'll change the script.
>

Looks better to me.


Overall, I think this patch looks good now.  I am pulling the new 
version of this patch onto my gpg branch.  We can always make more 
changes before it gets merged to master.

Allan