Comment 7 for bug 645147

Revision history for this message
Steve Langasek (vorlon) wrote :

Sorry, last comments - saving the best for last... :)

  * xapt: Use /etc/lsb-release to identify Debian/Ubuntu and use correct default
     URL and suites, but avoid 9MB lsb-release+python dependency

However, lsb_release is the defined interface for this - parsing the file may give you random otherness in the future. And lsb-release is a part of the minimal Ubuntu system. Could you instead check for the existence of lsb-release at runtime, and fall back to assuming Debian if it's not present? You already have a fallback, you're just checking /etc/lsb-release explicitly instead of

  * xapt: use --force-yes so unauthenticated repositories work

unauthenticated remote repositories are a bug, that should not be worked around by disabling authentication as this opens a trivial MITM attack against the main repos that *do* provide gpg authentication. This needs to be an option to xapt, not a hard-coded setting, and needs to be OFF by default (i.e., authentication enabled).

  * xapt: Fix incorrect skipping of source URLs that don't end in .list

I don't understand the point of this change - the only effective difference appears to be whether we print out the 'skipping' message for backup files, we're still ignoring all files which don't end in '.list', and that appears to be correct?

Finally:

 $progname = basename($0);
 $ourversion = &scripts_version();
 $dir = "/var/lib/xapt/";
-# $mirror = "http://ftp.uk.debian.org/debian/";
-$mirror = "http://archive.ubuntu.com/ubuntu/";
+given($distribution) {
+ when (/^Debian/) {
+ $mirror = "http://ftp.uk.debian.org/debian/";
+ $defaultsuites = "main contrib non-free";
+ }
+ when (/^Ubuntu/) {
+ $mirror = "http://archive.ubuntu.com/ubuntu/";
+ $defaultsuites = "main universe multiverse";
+ }
+ when (undef) {
+ $mirror = "http://ftp.uk.debian.org/debian/";
+ $defaultsuites = "main contrib non-free";
+ }
+}

should this use cdn.debian.net maybe, which is geo-aware? And the default suties list duplicates 'main' here and below; and $distribution should never be undefined because we already fall back to Debian. So maybe this is better written like so?:

-# $mirror = "http://ftp.uk.debian.org/debian/";
-$mirror = "http://archive.ubuntu.com/ubuntu/";
+given($distribution) {
+ when (/^Debian/) {
+ $mirror = "http://cdn.debian.net/debian/";
+ $defaultsuites = "contrib non-free";
+ }
+ when (/^Ubuntu/) {
+ $mirror = "http://archive.ubuntu.com/ubuntu/";
+ $defaultsuites = "universe multiverse";
+ }
+}