aptcc: Only report errors if there are errors

[julian-klode]

Previously aptcc would report some warnings as errors. This way,
it will drop any warning if there is no error. This matches the
behavior of python-apt.

As a bonus, we also log the entire message as a warning to the
journal.

This obsoletes #295

[dantti]

LGTM +1

[davidmhewitt]

Comment copied from #295 for visibility:

My reasoning for adding an extra parameter was that show_errors is used in a considerable number of places and was unsure whether any of the other places were relying on certain warnings being thrown as errors, so I was making sure the behavior was the same when that method is used elsewhere.

I'm happy that your approach fixes some of what I was trying to fix, but the lines to search for the "does not have a Release file" error and ignore it were relevant to my fix too. This is an error that you commonly get out of the backend with misconfigured repositories (IMO dpkg should report it as a warning instead of an error).

However, these error lines will still be counted in your errorCount variable and will still end up throwing the GPG failure and get stuck in an infinite loop which is what I was trying to fix here. Is there a reason you ommited that part from your new patch?

[dantti]

How would your string matching work if the string is localized?
Tho I think I set ENG language to be able to parse stuff but I'm not sure.

[davidmhewitt]

It looks like the "C" locale is forced so localization shouldn't be an issue.
https://github.com/hughsie/PackageKit/blob/master/backends/aptcc/apt-intf.cpp#L2417

However, there doesn't appear to be a way of getting constants from the backend that would allow cleaner checking for errors. I'm simply trying to fix an issue that causes PackageKit to go into an infinite loop and there is already code in that file that checks strings. If this can be improved in some way, I'm happy to do so, but this PR that now obsoletes my PR doesn't solve the issue I was originally trying to fix.

[julian-klode]

I think we probably should not report PK_ERROR_ENUM_GPG_FAILURE at all, just some generic download failure, since we don't know it's a GPG error.

[julian-klode]

I should add error categories to apt, and the ability to differentiate between different kinds of non-error error items (warnings vs notices).

[julian-klode]

And did that - we now always emit PK_ERROR_ENUM_CANNOT_FETCH_SOURCES

[julian-klode]

I disagree about ignoring "does not have a Release file" - that's a very important error, and should be shown as such. Even the other 404 errors should not be filtered out, that's non-sense.

If a repository does not work like that, the user needs to know.

[julian-klode]

FWIW, PK_ERROR_ENUM_GPG_FAILURE behavior is fundamentally wrong in my perspective. Retrying without verification opens up attack vectors everywhere, such as decompressors and stuff. If users want to configure untrusted repositories, they can allow them in their sources.list files, but circumventing essential security functions like that is a bad idea.

[julian-klode]

Not sure what not failing the last-time-update refers too. In general, apt has three levels of success for updates:

• Failure
• Partial success
• Success

It's always possible that updates fail with an error, but this should not prevent any calculations of new updates or prevent installs or something. The error should be shown, but if any of the repos was successfully updated, it should be considered successful otherwise.

[Conan-Kudo]

@julian-klode friendly ping?

[julian-klode]

Rebased

[hughsie]

If it's good enough for @dantti then it's good enough for me :)

[dantti]

hmm actually it seems errorCount should be initialized.

[julian-klode]

@dantti Fixed (and rebased again)