[pacman-dev] [PATCH] RFC: Remove "duplicate" download error messages

[Andrew Gregory]

On 03/08/21 at 02:12pm, Allan McRae wrote:
> This is a very incomplete and hacky patch...   But it does do this:
> 
> allan at mando ~/arch/code/pacman (patchqueue)
> > pacman -Syy
> :: Synchronizing package databases...
>  core                  131.1 KiB  47.0 KiB/s 00:03 [######################] 100%
>  extra                1645.7 KiB   490 KiB/s 00:03 [######################] 100%
>  community               5.4 MiB   161 KiB/s 00:34 [######################] 100%
> error: failed retrieving file 'extra.db' from allanmcrae.com : The requested URL returned error: 404
> error: failed retrieving file 'community.db' from allanmcrae.com : The requested URL returned error: 404
> error: failed retrieving file 'core.db' from allanmcrae.com : The requested URL returned error: 404
> 
> allan at mando ~/arch/code/pacman (patchqueue)
> > sudo ./build/pacman -Syy
> [sudo] password for allan:
> :: Synchronizing package databases...
>  core                  131.1 KiB  51.2 KiB/s 00:03 [######################] 100%
>  extra                1645.7 KiB   494 KiB/s 00:03 [######################] 100%
>  community               5.4 MiB   172 KiB/s 00:32 [######################] 100%
> error: failed retrieving file(s) from allanmcrae.com : The requested URL returned error: 404
> 
> There are a few things to note.
> 1) we lose the file names for the failed downloads.  And we lose
>    information of how many downloads failed.
> 2) I only changed the messages for download failures so far. So (e.g.)
>    having a file exceed its expected download size will still report
>    the file name.
> 3) debug output still gives file names
> 4) note the "error" in the messages above, yet pacman was still
>    successful as I have an actual mirror configured. I guess this (and
>    others?) should be changed to warning. We get an overall error
>    message if there is complete failure anyway.
> 5) I have been super lazy for this PoC, switching
>    alpm_list_remove_dupes() to use string comparisons, rather than
>    pointer comparisons.  This will need fixed.
> 
> 
> Things to discuss:
> - do we need to report the number of failures from a server.
>   Currently 1 and 200 failures are treated the same.  This involves
>   doing more than a dedup of the error list.
> - converting errors to warnings.  They are clearly not errors until
>   they are...  Is the final line enough:
> error: failed to synchronize all databases (failed to retrieve some files)

My main problem with this approach is that now alpm is spewing a bunch
of identical errors to front-ends, so now unless a front-end dedups
them the same way, we're actually making things worse.  We could dedup
in alpm, but then I don't think there's really an obvious place to
actually send the message.  Sending it at the end makes it impossible
for the user to infer that they might have a bad mirror and should
cancel the download and switch.  Sending it on the first occurrence
does the same because they don't know if it's just one file or
a recurring problem.

What about actually removing a bad mirror instead of deduplicating the
resulting messages?  Maybe start using the next mirror for subsequent
payloads after n failures or immediately for things like an
unresolvable host?

If we do make the errors more generic, I think turning them into
warnings and relying on the final "failed to retrieve some files" to
indicate actual failure is fine.