[Merge] ~jibel/livecd-rootfs/+git/add_multi_layered_squashfses_support:ubuntu/master into livecd-rootfs:ubuntu/master
Michael Hudson-Doyle
mwhudsonlp at fastmail.fm
Wed Dec 19 21:41:57 UTC 2018
Review: Needs Fixing
Hi,
This is quite a hard branch to review, partly because it's doing a few different things (code motion, adding new machinery and adding things that use the new machinery). And of course the general incomprehensibleness of this stuff and the fact that it's all written in bash...
Sorry about the lack of response so far though.
I think this is /mostly/ OK but it really is a bit sad that it only supports linear layers and so cannot be used for the one existing use of layers we already have. I don't think we should land this until we can use the new machinery for the old use case. In fact, I'm itching to rewrite the live-server code using something like this!
I don't think it would be that hard to lift this restriction though. My proposal: PASSES is a list of layer:baselayers with baselayer defaulting to the previous layer. So live-server could do this:
PASSES='
filesystem
maas-rack
maas-region
installer:filesystem
kernel
'
This will obviously complicate lb_chroot_layered a bit but I think it's reasonably contained.
I have some more slightly specific design-level questions/comments:
1) I presume all chroot hooks run for all layers? So hooks will need to key off $PASS to determine whether to run or not? Is there a way to more declaratively say a hook should only run for a given pass?
2) While lb_chroot_layered seems a full copy of lb_chroot, lb_binary_layered seems a poor cousin. Unless I'm missing things it does not support hooks or includes (or well lots of other things but I don't think we will every use any of those features)
3) I find the way that the overlay directories for each pass ends up in a directory called chroot.$PASS a bit confusing. overlay.$PASS would be better? Also did you consider just leaving the overlayfs for each pass around rather than constructing the list of overlays each time?
A couple more comments inline.
Have you tested this? Can I see what a livefs built using this looks like?
Diff comments:
> diff --git a/live-build/auto/config b/live-build/auto/config
> index 21cba94..61fa399 100755
> --- a/live-build/auto/config
> +++ b/live-build/auto/config
> @@ -77,24 +122,34 @@ add_package ()
> done
> }
>
> -OPTS=
> -COMPONENTS=
> -BINARY_REMOVE_LINUX=:
> -BINARY_IMAGES=none
> -MEMTEST=none
> -SOURCE='--source false'
> -BOOTLOADER=none
> -BOOTAPPEND_LIVE=
> -LIVE_TASK=
> -PREINSTALLED=false
> -PREINSTALL_POOL=
> -PREINSTALL_POOL_SEEDS=
> -PREFIX="livecd.$PROJECT${SUBARCH:+-$SUBARCH}"
> +add_layered_pass() {
Don't add_layered_pass (which seems unused btw) and add_layered_pass_delta need to add things to PASSES?
> + # Add a layer to an existing pass based on seeds matching a regexp
> + # $1 base pass
> + # $2 seeds (regexp)
>
> -CHROOT_HOOKS=
> -BINARY_HOOKS=
> + for seed in $(ls config/germinate-output/|grep -P "$2"); do
> + pass=${1}_${seed}
> + list_packages_from_seed ${seed} >> config/package-lists/livecd-rootfs.list.chroot_$pass
> + done
> +}
>
> -APT_OPTIONS=" --yes -oDebug::pkgDepCache::AutoInstall=yes "
> +add_layered_pass_delta() {
> + # Add a layer to an existing pass based on delta between seeds matching a regexp and a base seed
> + # $1 base pass
> + # $2 base seed
> + # $3 seeds to remove from base seed (regexp). If empty, a no-<base-seed> sublayer is generated.
> +
> + local seed_regexp="$3"
> + if [ -z "${seed_regexp}" ]; then
> + substract_package_lists ${2} "" >> config/package-lists/livecd-rootfs.removal-list.chroot_${1}_no-${2}
> + return
> + fi
> +
> + for seed in $(ls config/germinate-output/|grep -P "$seed_regexp"); do
> + pass=${1}_${seed}
> + substract_package_lists ${2} ${seed} >> config/package-lists/livecd-rootfs.removal-list.chroot_$pass
> + done
> +}
>
> add_chroot_hook ()
> {
> diff --git a/live-build/functions b/live-build/functions
> index 2cbe3e0..33ef79e 100644
> --- a/live-build/functions
> +++ b/live-build/functions
> @@ -501,3 +530,166 @@ snap_preseed() {
> fi
> _snap_preseed $CHROOT_ROOT $SNAP $CHANNEL
> }
> +
> +snap_from_seed() {
> + local base_seed=$1
> + local out=$2
> + local all_snaps
> + local seeds_expanded
> +
> + seeds_expanded=$(inheritance ${base_seed})
> + for seed in ${seeds_expanded}; do
> + echo "snap: considering ${seed}"
> + file=config/germinate-output/${seed}.snaps
> + [ -e "${file}" ] || continue
> + # extract the first column (snap package name) from germinate's output
> + # translate the human-readable "foo (classic)" into a
> + # more machine readable "foo/classic"
> + seed_snaps=$(sed -rn '1,/-----/d;/-----/,$d; s/(.*) \|.*/\1/; s, \(classic\),/classic,; p' "${file}")
> + for snap in ${seed_snaps}; do
> + echo "snap: found ${snap}"
> + all_snaps="${all_snaps:+${all_snaps} }${snap}"
> + done
> + done
> + if [ -n "${all_snaps}" ]; then
> + echo "${all_snaps}" > $out
> + fi
> +}
> +
> +seed_from_task ()
> +{
> + # Retrieve the name of the seed from a task name
> + local task=$1
> + local seed
> + local seedfile
> + local seedfiles
> +
> + seedfile="$(grep -lE "^Task-Key: +${task}\$" config/germinate-output/*seedtext|head -1)"
> + if [ -n "$seedfile" ]; then
> + basename $seedfile .seedtext
> + return
> + fi
> +
> + seedfiles="$(grep -lE "^Task-Per-Derivative: *1\$" config/germinate-output/*seedtext)"
> + if [ -n "$seedfiles" ]; then
> + for seed in $(echo $seedfiles | xargs basename -s .seedtext); do
> + if [ ${PROJECT}-${seed} = $task ]; then
> + echo ${seed}
> + return
> + fi
> + done
> + fi
> +}
> +
> +list_packages_from_seed () {
> + # Store all packages for a given seed, including its seed dependency
> + # $1: Name of the seed to expand to a package list
> +
> + local all_seeds="$(inheritance $1)"
> +
> + for seed in $all_seeds; do
> + head -n-2 config/germinate-output/${seed}.seed|tail -n+3|awk '{print $1}'
> + done|sort -u
> +}
> +
> +substract_package_lists() {
BTW, it's spelt subtract in anything but very archaic English.
> + # Substract a package list from another
> + #
> + # $1 source package list
> + # $2 Package list to substract from source package list
> + local list1=$(mktemp)
> + local list2=$(mktemp)
> +
> + list_packages_from_seed $1 > list1
> + list_packages_from_seed $2 > list2
> + comm -23 list1 list2
> +
> + rm list1
> + rm list2
> +}
> +
> +clean_debian_chroot() {
> + # remove crufty files that shouldn't be left in an image
> + rm -f chroot/var/cache/debconf/*-old chroot/var/lib/dpkg/*-old
> + Chroot chroot apt clean
> +}
> +
> +configure_universe() {
> + if [ -f config/universe-enabled ]; then
> + # This is cargo-culted almost verbatim (with some syntax changes for
> + # preinstalled being slightly different in what it doesn't ask) from
> + # debian-installer's apt-setup:
> +
> + cat > chroot/etc/apt/sources.list << EOF
> +# See http://help.ubuntu.com/community/UpgradeNotes for how to upgrade to
> +# newer versions of the distribution.
> +deb $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION main restricted
> +# deb-src $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION main restricted
> +
> +## Major bug fix updates produced after the final release of the
> +## distribution.
> +deb $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION-updates main restricted
> +# deb-src $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION-updates main restricted
> +
> +## N.B. software from this repository is ENTIRELY UNSUPPORTED by the Ubuntu
> +## team. Also, please note that software in universe WILL NOT receive any
> +## review or updates from the Ubuntu security team.
> +deb $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION universe
> +# deb-src $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION universe
> +deb $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION-updates universe
> +# deb-src $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION-updates universe
> +
> +## N.B. software from this repository is ENTIRELY UNSUPPORTED by the Ubuntu
> +## team, and may not be under a free licence. Please satisfy yourself as to
> +## your rights to use the software. Also, please note that software in
> +## multiverse WILL NOT receive any review or updates from the Ubuntu
> +## security team.
> +deb $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION multiverse
> +# deb-src $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION multiverse
> +deb $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION-updates multiverse
> +# deb-src $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION-updates multiverse
> +
> +## N.B. software from this repository may not have been tested as
> +## extensively as that contained in the main release, although it includes
> +## newer versions of some applications which may provide useful features.
> +## Also, please note that software in backports WILL NOT receive any review
> +## or updates from the Ubuntu security team.
> +deb $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION-backports main restricted universe multiverse
> +# deb-src $LB_PARENT_MIRROR_BINARY $LB_DISTRIBUTION-backports main restricted universe multiverse
> +
> +## Uncomment the following two lines to add software from Canonical's
> +## 'partner' repository.
> +## This software is not part of Ubuntu, but is offered by Canonical and the
> +## respective vendors as a service to Ubuntu users.
> +# deb http://archive.canonical.com/ubuntu $LB_DISTRIBUTION partner
> +# deb-src http://archive.canonical.com/ubuntu $LB_DISTRIBUTION partner
> +
> +deb $LB_PARENT_MIRROR_BINARY_SECURITY $LB_DISTRIBUTION-security main restricted
> +# deb-src $LB_PARENT_MIRROR_BINARY_SECURITY $LB_DISTRIBUTION-security main restricted
> +deb $LB_PARENT_MIRROR_BINARY_SECURITY $LB_DISTRIBUTION-security universe
> +# deb-src $LB_PARENT_MIRROR_BINARY_SECURITY $LB_DISTRIBUTION-security universe
> +deb $LB_PARENT_MIRROR_BINARY_SECURITY $LB_DISTRIBUTION-security multiverse
> +# deb-src $LB_PARENT_MIRROR_BINARY_SECURITY $LB_DISTRIBUTION-security multiverse
> +EOF
> +
> +fi
> +}
> +
> +configure_network_manager() {
> + # If the image pre-installs network-manager, let it manage all devices by
> + # default. Installing NM on an existing system only manages wifi and wwan via
> + # /usr/lib/NetworkManager/conf.d/10-globally-managed-devices.conf. When setting
> + # the global backend to NM, netplan overrides that file.
> + if [ -e chroot/usr/sbin/NetworkManager ]; then
> + echo "===== Enabling all devices in NetworkManager ===="
> + mkdir -p chroot/etc/netplan
> + cat <<EOF > chroot/etc/netplan/01-network-manager-all.yaml
> +# Let NetworkManager manage all devices on this system
> +network:
> + version: 2
> + renderer: NetworkManager
> +EOF
> + else
> + echo "==== NetworkManager not installed ===="
> + fi
> +}
--
https://code.launchpad.net/~jibel/livecd-rootfs/+git/add_multi_layered_squashfses_support/+merge/360878
Your team Ubuntu Core Development Team is subscribed to branch livecd-rootfs:ubuntu/master.
More information about the Ubuntu-reviews
mailing list