[pacman-dev] [PATCH] Report which package is missing a signature

[Allan McRae]

On 21/07/13 22:33, Ashley Whetter wrote:
> On 21 July 2013 11:09, Allan McRae <allan at archlinux.org> wrote:
> 
>> If any package in a sync transaction is missing a required signature,
>> we give an uninformative error message (which may or may not state that
>> the missing signature is the issue).  Always output the package with
>> the missing signature.
>>
>> Signed-off-by: Allan McRae <allan at archlinux.org>
>> ---
>>
>> There are still output errors here...
>>
>> When there is a single package in a transaction and it has a missing
>> required signature issue we output:
>> error: failed to commit transaction (package missing required signature)
>>
>> If there are multiple packages in a transaction and one is missing a
>> required signature, we could output either:
>> error: failed to commit transaction (package missing required signature)
>> or
>> error: failed to commit transaction (invalid or corrupted package (PGP
>> signature))
>>
>>
> I think "package missing required signature" makes sense for multiple
> packages because it's consistent with a single package transaction (unless
> we were to change that as well). Plus I think it reads more nicely.
> 

Just to clarify, I was reporting that make package still outputs one of
these error messages depending on where the package with the missing
signature comes in the transaction.  It is clear which is the right
message, but I do not intend to fix that.

>> When there is a mixture of missing signatures and corrupt packages, we
>> can out put either error message. (I guess technically both are correct!)
>>
> 
> Neither message is great. "invalid or corrupted package (PGP signature)"
> implies only the signature is incorrect, but really we need to make it
> clear that both things are wrong.
> 

The "invalid or corrupted package (PGP signature)"  (that is already
used) is a whole heap clearer when you are informed that PGP signature
verification failed just above and are offered to delete the corrupt
package.

>>
>> However, with this change we will easily be able to identify all corrupt
>> packages and all packages with missing signatures.
>>
>>
>> And this finished my quest for more informative error output from
>> signature checking...
>>
>>
>>  lib/libalpm/sync.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
>> index f9217bd..c342bbf 100644
>> --- a/lib/libalpm/sync.c
>> +++ b/lib/libalpm/sync.c
>> @@ -1094,6 +1094,10 @@ static int check_validity(alpm_handle_t *handle,
>>         if(errors) {
>>                 for(i = errors; i; i = i->next) {
>>                         struct validity *v = i->data;
>> +                       if(v->error == ALPM_ERR_PKG_MISSING_SIG) {
>> +                               _alpm_log(handle, ALPM_LOG_ERROR,
>> +                                               _("%s: missing required
>> signature\n"), v->pkg->name);
>> +                       }
>>                         if(v->error == ALPM_ERR_PKG_INVALID_SIG) {
>>
> 
> Correct me if I'm wrong but 'if(v->error == ALPM_ERR_PKG_INVALID_SIG) {'
> could become 'else if...' couldn't it.

Sure.

>                                 _alpm_process_siglist(handle, v->pkg->name,
>> v->siglist,
>>                                                 v->level &
>> ALPM_SIG_PACKAGE_OPTIONAL,
>> --
>> 1.8.3.3
>>
>>
>>
> 
> 
>