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

[Ashley Whetter]

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.


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


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

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