Merge ~chad.smith/ubuntu/+source/sbuild-launchpad-chroot:focal/fix-multiline-apt-sources into ~motu/ubuntu/+source/sbuild-launchpad-chroot:master

[Chad Smith]

Add support for schroot config option apt.mirror.ubuntu and additional command line parameter --apt-mirror to allow overrides of schroot apt mirror.

LP: #1872163

[Scott Moser (smoser)]

I don't think this produces output or behavior consisent with the old "oneline" behavior.

I put a little test program together to excercise it and see.

After line 91, its basically copy-paste from your branch.

My feeling is that
 a.) its an unintended change that the launchpad chroot has this expanded sources.list
 b.) For the purpose of 'sbuild-launchpad-chroot', it probably actually makes sense to just declare the content of /etc/apt/sources.list (as my change suggested). Possibly you'd want to allow the mirror to be configured.

here is some example output:
$ /tmp/testme.sh updates main oneline | grep "^deb"
deb http://archive.ubuntu.com/ubuntu/ focal main
deb http://archive.ubuntu.com/ubuntu/ focal-security main
deb http://archive.ubuntu.com/ubuntu/ focal-updates main

$ /tmp/testme.sh updates main full | grep "^deb"
deb http://archive.ubuntu.com/ubuntu/ focal main
deb http://archive.ubuntu.com/ubuntu/ focal main
deb http://archive.ubuntu.com/ubuntu/ focal main
deb http://archive.ubuntu.com/ubuntu/ focal-security main
deb http://archive.ubuntu.com/ubuntu/ focal-updates main
deb http://archive.ubuntu.com/ubuntu/ focal-security main
deb http://archive.ubuntu.com/ubuntu/ focal-updates main
deb http://archive.ubuntu.com/ubuntu/ focal-security main
deb http://archive.ubuntu.com/ubuntu/ focal-updates main

and then for 'release', you get:
$ /tmp/testme.sh release main oneline | grep "^deb"
deb http://archive.ubuntu.com/ubuntu/ focal main

$ /tmp/testme.sh release main full | grep "^deb"
deb http://archive.ubuntu.com/ubuntu/ focal main
deb http://archive.ubuntu.com/ubuntu/ focal-updates main
deb http://archive.ubuntu.com/ubuntu/ focal main
deb http://archive.ubuntu.com/ubuntu/ focal-updates main
deb http://archive.ubuntu.com/ubuntu/ focal main
deb http://archive.ubuntu.com/ubuntu/ focal-updates main
deb http://archive.ubuntu.com/ubuntu/ focal-backports main
deb http://security.ubuntu.com/ubuntu/ focal-security main
deb http://security.ubuntu.com/ubuntu/ focal-security main
deb http://security.ubuntu.com/ubuntu/ focal-security main

review: Needs Fixing

[Scott Moser (smoser)]

oh yeah, the test program: https://paste.ubuntu.com/p/bSpKnqbJYw/

[Chad Smith (chad.smith)]

Thanks Scott, I'll adopt your approach here as it seems more correct and avoids the duplication of multi-line config. I'll circle back with livecd.rootfs folks to make sure truncating the multi-line apt config is acceptable.

[Chad Smith (chad.smith)]

Tested this locally and sbuilds work for me with this and represent a simpler sources.list file.

[Scott Moser (smoser)]

One thing I can imagine wanting to do is point my sbuild at a local mirror.
That could be accomplished by allowing the user to set the mirror used.

I'm not entirely sure how this all works, but it looks like values from
 /etc/schroot/chroot.d/<name>
are populated into environment variables.

$ grep -ri launchpad.series /etc/schroot/chroot.d/ 2>/dev/null
/etc/schroot/chroot.d/focal-amd64:launchpad.series=focal
/etc/schroot/chroot.d/bionic-amd64:launchpad.series=bionic

Thats how I found 'LAUNCHPAD_SERIES'. It might be nice to read UBUNTU_APT_MIRROR:

mirror=${UBUNTU_APT_MIRROR:-http://archive.ubuntu.com/ubuntu}
echo "deb $mirror ${LAUNCHPAD_SERIES} main restricted universe multiverse" > "$sources"

Then the user could set ubuntu.apt.mirror (untested).

Possibly 'apt.mirror.ubuntu' or something might be better named. (there is a 'apt.enable' currently in my /etc/schroot/chroot.d/bionic-amd64 .

other than that, this looks fine to me.

c5e347d... by Chad Smith on 2020-05-05

Add --apt-mirror parameter to override default apt mirror in schroot

Add support in /etc/schroot/setup.d/90apt-sources to allow setting
a new apt mirror when creating a launchpad schroot via a custom
schroot config option apt.mirror.ubuntu.

This option can be provided via and new CLI parameter:
  sbuild-launchad-chroot create --apt-mirror http://somemirror.com/ubuntu

If unset, the default mirror http://archive.ubuntu.com/ubuntu/ is used.

[Chad Smith (chad.smith)]

Sorry for the delay on this Scott.

Confirmed from man schroot.conf and added apt.mirror.ubuntu to my schroot config file via

sudo /usr/bin/sbuild-launchpad-chroot create -a amd64 -s focal -n bbsw-focal-amd64 -m http://fakemirror.com/ubuntu

"""
  Customisation
       In addition to the configuration keys listed above, it is possible to
       add custom keys. These keys will be used to add additional environment
       variables to the setup script environment when setup scripts are run.
       The only restriction is that the key name consists only of alphanumeric
       characters and hyphens, begins with an alphabet character and contains
       at least one period. That is to say, that it matches the extended reg‐
       ular expression “^([a-z][a-z0-9]*\.)+[a-z][a-z0-9-]*$”.
"""

[Scott Moser (smoser)]

I'm going to approve, assuming you can fix the one typo there.

It'd be great if we could get someone else to vet this. I really don't understand how this can be broken.

review: Approve

[Scott Moser (smoser)]

Well.. given my attempt at "can this really be broken"
 https://irclogs.ubuntu.com/2020/05/06/%23ubuntu-devel.html#t13:58

If you update debian/changelog appropriately then I'll merge this.

One other nit too... its "more correct" I think to have a default mirror of:
 http://archive.ubuntu.com/ubuntu

As apt always appends a '/' . So with the trailing / on the mirror, you get requests for something like this:

  http://archive.ubuntu.com/ubuntu//dists/focal/Contents-amd64.gz

So in summary, 3 requests:
 a.) series not seris
 b.) replace default mirror values with 'http://archive.ubuntu.com/ubuntu'
 c.) add debian/changelog

and then i'll sponsor upload.

review: Needs Fixing

[Paride Legovini (paride)]

If I understand the proposed patch correctly, it will make the tool always use http://archive.ubuntu.com/ubuntu/ by default. This is a step backwards on non-amd64 archs, where http://ports.ubuntu.com/ubuntu-ports/ should be used, and got properly setup on pre-Focal releases.

[Scott Moser (smoser)]

@Paride,
good catch.
Thank you.

yeah, we'd want to check the arch i guess .

LAUNCHPAD_ARCH (launchpad.arch) could be consulted.

maybe 'apt.archive.ubuntu.x86' and 'apt.archive.ubuntu.ports' or something?
and correctly pick the right one?

49c1df3... by Chad Smith on 2020-05-06

90apt-sources: default to existing apt mirror if no apt.mirror.ubuntu

bddcabc... by Chad Smith on 2020-05-06

update changelog, but version to 0.17

[Chad Smith (chad.smith)]

Thanks for the review gents. Pushed changes to source the default apt mirror url from the existing chroot image /etc/apt/sources.list if --apt-mirror is not provided.
This should default to http://ports.ubuntu.com/ubuntu-ports/ or http://archive.ubuntu.com/ubuntu/ or whatever URL is in the multi-line /etc/apt/sources.list file in the image.

This way we don't have to statically map a given architecture to a known (possibly stale) APT URL.

[Scott Moser (smoser)]

OK...

So I took a more wholistic (sp?) look at this script/program, and the result is at

https://code.launchpad.net/~smoser/ubuntu/+source/sbuild-launchpad-chroot/+git/sbuild-launchpad-chroot/+merge/383600

Unmerged commits

bddcabc... by Chad Smith on 2020-05-06

update changelog, but version to 0.17

49c1df3... by Chad Smith on 2020-05-06

90apt-sources: default to existing apt mirror if no apt.mirror.ubuntu

c5e347d... by Chad Smith on 2020-05-05

Add --apt-mirror parameter to override default apt mirror in schroot

Add support in /etc/schroot/setup.d/90apt-sources to allow setting
a new apt mirror when creating a launchpad schroot via a custom
schroot config option apt.mirror.ubuntu.

This option can be provided via and new CLI parameter:
  sbuild-launchad-chroot create --apt-mirror http://somemirror.com/ubuntu

If unset, the default mirror http://archive.ubuntu.com/ubuntu/ is used.

2dc45dd... by Chad Smith on 2020-04-22

90apt-sources: emit one-line /etc/apt/sources.list for schroot create

Ubuntu Focal started shipping full multi-line /etc/apt/sources.list
which contains empty lines in livecd.ubuntu-base.rootfs.tar.gz.

The setup.d/90apt-sources expects a single apt source line which
it duplicates when adding more suites and pockets.

Continue to emit a one-liner sources.list in chroots to continue to
use simple suite and pocket configuration based on schroot name.

LP: #1872163

Authored-by: Scott Moser <email address hidden>

diff --git a/bin/sbuild-launchpad-chroot b/bin/sbuild-launchpad-chroot
index bb89251..e450e80 100755
--- a/bin/sbuild-launchpad-chroot
+++ b/bin/sbuild-launchpad-chroot
@@ -136,6 +136,8 @@ def create_chroot(args):
     config.set(args.name, "launchpad.arch", args.architecture)
     config.set(args.name, "launchpad.url", "")
     config.set(args.name, "apt.enable", "true")
+    if args.apt_mirror:
+        config.set(args.name, "apt.mirror.ubuntu", args.apt_mirror)
     config.set(args.name, "aliases", ",".join(aliases))
     if personality:
         config.set(args.name, "personality", personality)
@@ -350,6 +352,10 @@ if __name__ == '__main__':
     parser_create.add_argument("-s", "--series", required=True)
     parser_create.add_argument("-a", "--architecture", required=True)
     parser_create.add_argument(
+        "-m", "--apt-mirror",
+        help=("Alternate apt mirror to use in chroot. "
+              "Default=http://archive.ubuntu.com/ubuntu/"))
+    parser_create.add_argument(
         "-b", "--backing-store", default="auto",
         choices=("auto", "btrfs", "directory", "overlay", "overlayfs"))
 
diff --git a/debian/changelog b/debian/changelog
index 9693db0..bcd8597 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+sbuild-launchpad-chroot (0.17) focal; urgency=medium
+
+  * add optional --apt-mirror when creating a new schroot
+  * 90apt-sources: emit a single line to apt-sources.list (LP: #1873507)
+
+ -- Chad Smith <[e-mail]>  Wed, 06 May 2020 14:59:20 -0600
+
 sbuild-launchpad-chroot (0.16) focal; urgency=medium
 
   * Migrate to python3.
diff --git a/etc/schroot/setup.d/90apt-sources b/etc/schroot/setup.d/90apt-sources
index ced4a4e..d1fec1b 100755
--- a/etc/schroot/setup.d/90apt-sources
+++ b/etc/schroot/setup.d/90apt-sources
@@ -112,6 +112,21 @@ if [ $STAGE = "setup-start" ] || [ $STAGE = "setup-recover" ]; then
         APT_COMPONENTS="$comps"
     fi
 
+    # LAUNCHPAD_SERIES and APT_MIRROR_UBUNTU come from schroot config file
+    # launchpad.series and apt.ubuntu.mirror config options.
+    mirror=${APT_MIRROR_UBUNTU:-http://archive.ubuntu.com/ubuntu/}
+    if [ -n "$APT_MIRROR_UBUNTU" ] &&
+        mirror=${APT_MIRROR_UBUNTU}
+    else
+        # No apt.ubuntu.mirror override specified, get the APT URL from $soures
+        mirror=`grep "^deb .*"  $sources -m 1 | cut -d" " -f2`
+    fi
+    if [ -n "$LAUNCHPAD_SERIES" ] &&
+        [ -n "$APT_COMPONENTS" -o -n "$APT_POCKETS" ]; then
+        echo "Setting one-line sources.list (LP: #1873507)"
+        echo "deb ${mirror} ${LAUNCHPAD_SERIES} main restricted universe multiverse" > "$sources"
+    fi
+
     if [ -n "$APT_POCKETS" ]; then
         echo "setting apt pockets to 'release $APT_POCKETS' in sources.list"
         awk -v pockets="$APT_POCKETS" '