Re: [gentoo-dev] [PATCH 1/2] sec-keys.eclass: new eclass

[Eli Schwartz <eschwartz@gentoo.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2526 bytes --]

On 11/28/24 5:35 AM, Ulrich Müller wrote:
>>>>>> On Wed, 27 Nov 2024, Eli Schwartz wrote:
> 
>> --- /dev/null
>> +++ b/eclass/sec-keys.eclass
>> @@ -0,0 +1,150 @@
>> +# Copyright 2024 Gentoo Authors
>> +# Distributed under the terms of the GNU General Public License v2
>> +
>> +# @ECLASS: sec-keys.eclass
>> +# @MAINTAINER:
>> +# Eli Schwartz <eschwartz@gentoo.org>
>> +# @AUTHOR:
>> +# Eli Schwartz <eschwartz@gentoo.org>
>> +# @SUPPORTED_EAPIS: 8
>> +# @BLURB: Provides a uniform way of handling ebuilds which package PGP key material
>> +# @DESCRIPTION:
>> +# This eclass provides a streamlined approach to finding suitable source material
>> +# for OpenPGP keys used by the verify-sig eclass. Its primary purpose is to permit
>> +# developers to easily and securely package new sec-keys/* packages. The eclass
>> +# removes the risk of developers accidentally packaging malformed key material, or
>> +# neglecting to notice when PGP identities have changed.
>> +#
>> +# To use the eclass, define SEC_KEYS_VALIDPGPKEYS to contain the fingerprint of
>> +# the key and the short name of the key's owner.
> 
> Please wrap these comment lines to a line length of 70-ish characters
> for readability.
> 
> Also, there should be two spaces after every full stop (except when it's
> followed by a newline), so groff can recognise the sentence end in the
> generated man page.


I usually do 80-ish for readability! Okay, I can do 70 too. :) Thanks
for the tip about the spaces, I don't usually write groff by hand.
Surprising that groff cannot handle this automatically, though.


>> +_sec_keys_set_globals() {
>> +	if [[ ${SEC_KEYS_VALIDPGPKEYS[*]} ]]; then
> 
> Why is the if needed? If the array is empty, the following for loop
> won't execute.


Not sure, perhaps an artifact of a previous revision that had different
handling. Let's remove it.


>> +	printf '%s\n' "${imported_keys[@]}" | sort > imported_keys.list || die
>> +	printf '%s\n' "${SEC_KEYS_VALIDPGPKEYS[@]%%:*}" | sort > allowed_keys.list || die
> 
> Maybe create these files in ${T} instead?


I'm not sure this is an important distinction. It's the main thing the
package works on. I could put GNUPGHOME in ${T} as well, if you like? :)

But keeping it in ${WORKDIR} makes it more straightforward for people to
look at manually when a failed build happens. And that's important when
dealing with the primary logic of a package (there's no source code to
compile here).



-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]