pdebuild-cross fails to build things

[Wookey]

Various minor but fatal bugs were found using pdebuild-cross to actually build packages:

1) It didn't fall back to debian/control when debian/xcontrol was not present (it almost never is) (this fix somehow escpaed previous release)
2) It was not respecting arch qualifiers on dependencies so would try to install packages only available for kfreebsd, for example
3) When collecting the list of available apt sources it used any files in /etc/sources.list.d/ even backups and files not ending in .list this tended to break things
4) It would default to 'main contrib non-free' instead of 'main universe', which is wrong on ubuntu
5) It wouldn't install any packages if any unauthenticated repos were present.
6) Build dependencies from build-depends-indep were not installed. I'm not sure how much this matters, but it doesn't do any harm to include those.

All these are fixed in the attached patch. It now uses lsb-release to correctly default to ubuntu or debian system behaviour.

[Wookey (wookey)]

Here is an updated patch which doesn't add an lsb-release dependency and does work properly with latest apt. The upload should be syncronised with multstrap fix: https://bugs.launchpad.net/ubuntu/+source/multistrap/+bug/646901
It also removes vestiges of the now-superceded apt-cross which were just wasting time on every build.

[Steve Langasek (vorlon)]

From the diff:

  Only in emdebian-crush-2.2.4ubuntu2/buildd: A00pbuilder-cross-hook

Is this file meant to be included?

[Steve Langasek (vorlon)]

Likewise,

  Only in emdebian-crush-2.2.4ubuntu2/buildd: A20-apt-cross-hook

[Steve Langasek (vorlon)]

 foreach my $dep (@dependencies)
 {
        $dep =~ s/^ //;
+ my $archlisted=1;
+ # if there is an arch specifier then honour it when making deps list
+ if ($dep =~ /\[(.*)\]/) {
+ $archlisted=0;
+ my @archspec=split(/ /, $1);
+ foreach my $arch (@archspec) {
+ if ($arch eq $hostarch) { $archlisted=1; last }
+ }
+ next unless $archlisted;
+ };
        @bits=split(/ /, $dep);
        push @aptlist, $bits[0];
 }

This is an incomplete fix; it misses the architecture wildcards that are now supported as of Debian Policy 3.9.1 (Section 7.1 of Policy), and these wildcards are starting to be used in the wild. This change is still a net improvement so I don't think we should block on a fully correct fix, but please open a separate bug to track the incomplete handling of arch-specific build-deps.

[Wookey (wookey)]

+++ Steve Langasek [2010-09-28 18:37 -0000]:
> Likewise,
>
> Only in emdebian-crush-2.2.4ubuntu2/buildd: A00pbuilder-cross-hook
> Only in emdebian-crush-2.2.4ubuntu2/buildd: A20-apt-cross-hook

Both those files should be removed. Now that it is clear xapt actually
works, apt-cross is being dropped. Having those hooks around but
apt-cross unusued wasted a lot of time installing apt-cross plus about
15 perls packages on every build.

This is inline with upstream, we're just slightly ahread of the Debian
mainline package at the moment - it's all being merged.

Wookey
--
Principal hats: Linaro, Emdebian, Wookware, Balloonboard, ARM
http://wookware.org/

[Steve Langasek (vorlon)]

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";
+ }
+}

[Wookey (wookey)]

+++ Steve Langasek [2010-09-29 18:03 -0000]:
> 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.

agreed

> And lsb-release is a
> part of the minimal Ubuntu system.

Do you mean the file or the package? The file is part of base-files
and is present in any Ubuntu chroot, the package lsb-release is only there
if something installs it.

> Could you instead check for the
> existence of lsb-release at runtime, and fall back to assuming Debian if
> it's not present?

We could, but my understanding was that the file was due to start
existing in Debian soonish, so that seems even less reliable. If I'm
wrong about that then it would be plausible.

In fact we've decided that a much better way to do this is provide a
simple config file for the things that change between distros, and
that file can change with distro. So now the program itself doesn't
have to guess anything about distro - just read its file.

I could just implement that.

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

I don't really agree that MITM attacks on disposable chroots matter
much, and certainly not that unauthenticated remote repos are a bug.
They are quite common, especially in embedded and cross-tool work
(that's where marcin's tools started a few weeks ago for example) and
it's useful if they work.

xapt is only doing this because the whole new authentication
infrastructure in 0.8 doesn't seem to work right (or at least we are
not using it right in multistrap). xapt already used --force-yes in
the version that's already in the repo (under some circs) so refusing
to upload it on those grounds is not realy convincing.

All that said I do agree that this is hackery and would love to have
it all working right, but I did quite a bit of fiddling and failed to
work out what was actually necessary for this to work, and in the
meantime it seemed a lot better to have a working solution of some
sort than to disappear further down that rathole. At least until I got
some clarification from the apt team.

A related issue is that no-auth is an apt-wide setting and should
arguably be a per-repo thing so that when you have one unauthed repo
with a few extra packages in it you don't have ignore auth on _all_
packages. It's very all-or-nothing currently.

> * 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',
> a...

[Wookey (wookey)]

OK. here is an updated patch following above discussion which tests OK with current multistrap.

It now uses a config file /etc/xapt/xapt.conf to set mirror and default components - this is the only thing that should change between Debian/Ubuntu. Avoids whole lsb-release issue and the switch that neither upstream nor Steve liked.

The apt trusted keyring config options are needed to make xapt install anything, otherwise it just insists nothing can be authenticated. This is exactly the same issue as with multistrap (because xapt is downloading cross-packages into /var/lib/xapt, not the main chroot). So now we have working authentication with new-apt - woo.

Also included is an upstream fix to the translation file stuff so that translations should actually work - they were bust in previous release.

[Wookey (wookey)]

OK, and here's the same thing with a bit more tidying.

[Steve Langasek (vorlon)]

Wookey,

This looks good, but the xapt.conf needed for Ubuntu is missing from the diff (shows up as an "Only in [...]" file) Can you please attach it?

[Wookey (wookey)]

+++ Steve Langasek [2010-10-05 07:51 -0000]:
> Wookey,
>
> This looks good, but the xapt.conf needed for Ubuntu is missing from the
> diff (shows up as an "Only in [...]" file) Can you please attach it?

oops.

Wookey
--
Principal hats: Linaro, Emdebian, Wookware, Balloonboard, ARM
http://wookware.org/

[Launchpad Janitor (janitor)]

This bug was fixed in the package emdebian-crush - 2.2.4ubuntu3

---------------
emdebian-crush (2.2.4ubuntu3) maverick; urgency=low

  [Wookey]
  * xapt: Add config file for Debian/Ubuntu variant defaults
     (mirror and components)
  * xapt: Fix incorrect skipping of source URLs that don't end in .list
  * xapt: use --force-yes for re-installs
  * xapt: use keyring from host apt config when downloading packages
     to cross, so authenticated repos work with apt>= 0.7.26 LP: #645147
  * xapt-hook: Respect arch qualifiers in dependencies so we don't try
     to install deps which don't exist for the host architecture.
  * xapt-hook: Fix failure to fall back to control when xcontrol not present
  * xapt-hook: Use Build-Depends-Indep too in dependency list
  * pdebuild-cross: remove redundant apt-cross hooks

  [ Neil Williams ]
  * pdebuild-cross needs a strict dependency on xapt source version.
 -- Wookey <email address hidden> Mon, 13 Sep 2010 17:59:35 +0100