Fix Changelog URL
ClosedPublic

Authored by vanini on Feb 19 2018, 1:15 PM.

Details

Summary

Use apt-pkg to get the correct URL to fetch the Changelog.

BUG: 390475

Test Plan

Muon now shows the Changelog in the respective tab.

Diff Detail

Repository
R548 QAPT Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
vanini requested review of this revision.Feb 19 2018, 1:15 PM
vanini created this revision.
sitter requested changes to this revision.Feb 19 2018, 1:37 PM

Looks good. I am not sure this is entirely correct though.

From looking at the ::URI() code it seems this will only work if AlwaysOnline isn't set to false (false is the default value). If it is false, ::URI() will try to give out a URI to the local file system using the somewhat non-standard schemes copy:// or gzip:// which I am willing to bet frontends cannot handle properly. Which makes this a backwards incompatible change. So, I'd say you need to apply a filter for http(s) and return QUrl() if it is none of those.

As an addition, you could add another function changelogUrlWithLocal() which returns all URIs, irregardless of scheme, and maybe even coerces those copy:// and gzip:// schemes to file:// (I do not quite understand why they aren't file to begin with though). The two functions could then get merged in the next ABI break.

This revision now requires changes to proceed.Feb 19 2018, 1:37 PM
vanini updated this revision to Diff 27604.Feb 20 2018, 1:22 PM
  • Filter out URIs which are not http or https
sitter accepted this revision.Feb 20 2018, 1:28 PM

you might want to make the string cost. other than that good to land. πŸ‘

src/package.cpp
573

could be const

This revision is now accepted and ready to land.Feb 20 2018, 1:28 PM

On my system (Ubuntu 17.10 artful) the apt package ships with /etc/apt/apt.conf.d/01-vendor-ubuntu which contains

/etc/apt/apt.conf.d/01-vendor-ubuntu
Acquire::Changelogs::AlwaysOnline "true";
$ apt-config dump |grep AlwaysOnline
Acquire::Changelogs::AlwaysOnline "true";
Acquire::Changelogs::AlwaysOnline::Origin "";
Acquire::Changelogs::AlwaysOnline::Origin::Ubuntu "1";

We could force the setting to true when initializing the library, but that would mean to override user settings without notice for no good reason.

Regarding the addition of the function changelogUrlWithLocal() you proposed, I don't care (given the widespread use of the library... Muon and a couple kubuntu tools). I would rather have apt-pkg handle the download/copy/decompression and give me a local file in /tmp/ to pass over. This way the frontend wouldn't have to figure out whether it is compressed and we behave the same way as apt. Although yes, being able to get just the URL makes sense.

This revision was automatically updated to reflect the committed changes.