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

[Allan McRae]

On 22/07/13 13:46, Allan McRae wrote:
> 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.
>>
> 

Ugh...  clarifying the clarification...

> Just to clarify, I was reporting that 

*pacman*

> 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
>>>
>>>
>>>
>>
>>
>>
> 
> 
> 
>