Re: [PATCH v4 08/14] release: Compress installation images and remove live image

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 8841 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> systemd-sysupdate will fail if the OS image does not fit in the
> partitions that the installer created.  Therefor, the partitions need to
> be very large so that there is plenty of room for the OS to grow.
> Furthermore, systemd-sysupdate requires both A and B copies of both the
> root and verity partitions.
>
> mkfs.ext4 is not able to produce images with files large enough to hold
> both the primary and backup copy of the root partition [1].  Reducing
> the sizes of partitions to be little greater than the size of the root
> filesystem image does not help.  The produced file is still too large.
> Therefore, compress the image, which causes it to be small enough that
> mkfs.ext4 can handle it.
>
> This breaks the option to use the installer as a live image.  Therefore,
> remove it.  This option will return once Spectrum switches to the GNOME
> OS installer [2].  However, it is still possible to build a live image
> that is separate from the installer.  Document how to build and use it.
>
> GRUB2 does support compressed loopback images, but these presumably
> buffer the whole image in memory.  Since the entire installer will be
> replaced, making it work is not considered worthwhile.
>
> [1]: https://github.com/tytso/e2fsprogs/issues/254
> [2]: https://spectrum-os.org/lists/archives/spectrum-devel/87wm4dlkhz.fsf@alyssa.is/
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
> Changes since v3:
> - Make the compression level configurable.  The default is 1 so that
>   development builds finish in a reasonable amount of time.  Release
>   builds should use compression level 9.
> Changes since v2:
> - Remove live image test instead of skipping it.
> - Document the change.
> - Document that there is still a live image available, though it is
>   separate from the installer.
> - Document how to build the live image.
>
> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
> ---
>  Documentation/installation/getting-spectrum.adoc | 31 +++++++++++++++++++-----
>  host/initramfs/Makefile                          |  8 ------
>  host/initramfs/etc/probe                         | 20 ---------------
>  lib/config.default.nix                           |  1 +
>  release/checks/integration/meson.build           |  2 +-
>  release/checks/integration/try.c                 | 29 ----------------------
>  release/combined/eosimages.nix                   | 13 +++++-----
>  release/combined/grub.cfg.in                     |  5 ----
>  8 files changed, 34 insertions(+), 75 deletions(-)

Okay, I've left comments, but the only real blocker here is deciding
whether we go ahead with the config option.  I would rather not as
outlined below.

> diff --git a/Documentation/installation/getting-spectrum.adoc b/Documentation/installation/getting-spectrum.adoc
> index 29803aa324b196119a03b22d7f1e2d7730e2c1eb..e7806e0f92793320bf0cdcbdd11dbc4e713275c7 100644
> --- a/Documentation/installation/getting-spectrum.adoc
> +++ b/Documentation/installation/getting-spectrum.adoc
> @@ -42,10 +42,30 @@ still take a very long time.
>  == Installing Spectrum
>  
>  To install Spectrum on a computer, you can use a USB drive as a
> -bootable Spectrum installer device.  When booting a system from the
> -installer device, you will be able to choose whether to try out
> -Spectrum without installing it on your system (as a live image), or to
> -install it to your computer's internal storage.
> +bootable Spectrum installer device.  You will need to choose whether to
> +try out Spectrum without installing it on your system (as a live image),
> +or to install it to your computer's internal storage.
> +
> +=== Building A Live Image
> +
> +First, you need to build the Spectrum image:
> +
> +[source,shell]
> +----
> +git clone https://spectrum-os.org/git/spectrum
> +nix-build spectrum/release/live
> +----
> +
> +If you haven't set up the xref:binary-cache.adoc[binary cache], this
> +will take a very long time.  When it's done, a symbolic link named
> +"result" will appear in the current directory, pointing to the
> +installer image.  Write that image to a USB drive, for example using
> +`dd` (command line) or
> +https://gitlab.com/bztsrc/usbimager[`usbimager`] (graphical, available
> +in Nixpkgs).  Boot your system from the USB drive, and Spectrum should
> +be ready for you to use.
> +

This is duplicating a lot of existing text.  Could we not explain the
building stuff once, and then just say in the Live Image / Installer
sections which paths to build?

> +=== Building The Installer
>  
>  First, you need to build the Spectrum image:
>  
> @@ -62,8 +82,7 @@ installer image.  Write that image to a USB drive, for example using
>  `dd` (command line) or
>  https://gitlab.com/bztsrc/usbimager[`usbimager`] (graphical, available
>  in Nixpkgs).  Boot your system from the USB drive, and you should see
> -a menu allowing you to choose between "Try Spectrum" and "Install
> -Spectrum".
> +a menu allowing you to "Install Spectrum".

This should be updated to demonstrate release/installer rather than
release/combined, since the latter is now pointless and due for removal.

> diff --git a/host/initramfs/Makefile b/host/initramfs/Makefile
> index 27a26b46a8110d35ee02a63b12931d6b9c2742e5..735c12fc207f027db2b605309976a832b09335d6 100644
> --- a/host/initramfs/Makefile
> +++ b/host/initramfs/Makefile
> @@ -43,14 +43,6 @@ build/live.img: ../../scripts/format-uuid.awk ../../scripts/make-gpt.sh ../../sc
>  	    $(ROOT_FS):root:$$1
>  	mv $@.tmp $@
>  
> -build/loop.tar: build/live.img
> -	$(TAR) -cf $@ build/live.img
> -
> -build/loop.img: ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/loop.ext4
> -	bash ../../scripts/make-gpt.sh $@.tmp \
> -	    build/loop.ext4:56a3bbc3-aefa-43d9-a64d-7b3fd59bbc4e
> -	mv $@.tmp $@
> -
>  clean:
>  	rm -rf build
>  .PHONY: clean

Oh, was this completely unused?  I'll look into whether we can just drop
this immediately.

> diff --git a/lib/config.default.nix b/lib/config.default.nix
> index a8422345cc00f9413bb19ec968fd89c82fed801b..b1c4c33f1dca3d503fb8e3ef855d65c49cf5a202 100644
> --- a/lib/config.default.nix
> +++ b/lib/config.default.nix
> @@ -4,4 +4,5 @@
>  {
>    pkgsFun = import ./nixpkgs.default.nix;
>    pkgsArgs = {};
> +  compressionLevel = 1;
>  }

I don't love proliferating config parameters…  Given this is likely
going to be best set to 9 for any non-development image builds, and 1
will mostly only be desirable for people working on the image (and CI, I
suppose, but it should really be testing the settings used on the real
image), those people working on the image could adjust the gzip
parameters when doing so.

At the very least, 9 should be the default.  Upstream-produced images
should use the default settings so they're easy to reproduce (when they
are actually reproducible.)

> diff --git a/release/combined/eosimages.nix b/release/combined/eosimages.nix
> index 0ac4c48374e7098a2b91f61fc07cebb2042ffbdc..9cb35dcecee54c17392b609c493272ec83062e9b 100644
> --- a/release/combined/eosimages.nix
> +++ b/release/combined/eosimages.nix
> @@ -2,7 +2,7 @@
>  # SPDX-FileCopyrightText: 2021-2023 Alyssa Ross <hi@alyssa.is>
>  
>  import ../../lib/call-package.nix (
> -{ callSpectrumPackage, runCommand, e2fsprogs, tar2ext4 }:
> +{ callSpectrumPackage, runCommand, e2fsprogs, tar2ext4, config }:
>  
>  runCommand "eosimages.img" {
>    nativeBuildInputs = [ e2fsprogs tar2ext4 ];
> @@ -12,11 +12,12 @@ runCommand "eosimages.img" {
>    unsafeDiscardReferences = { out = true; };
>    dontFixup = true;
>  } ''
> +  set -euo pipefail
>    mkdir dir
>    cd dir
> -  ln -s $image $imageName
> -  sha256sum $imageName > $imageName.sha256
> -  tar -chf $NIX_BUILD_TOP/eosimages.tar *
> -  tar2ext4 -i $NIX_BUILD_TOP/eosimages.tar -o $out
> -  e2label $out eosimages
> +  ln -s -- "$image" "$imageName"
> +  gzip -${builtins.toString (0 + config.compressionLevel)} < "$image" > "$imageName.gz"
> +  sha256sum -- "$imageName.gz" > "$imageName.gz.sha256"
> +  tar -ch -- "$imageName.gz" "$imageName.gz.sha256" | tar2ext4 -o "$out"
> +  e2label "$out" eosimages
>  '') (_: {})

I have made the same comments the last two times I have reviewed this
code[1][2], and yet the set that applies options already set by stdenv
is still here, and so are the unnecessary quoting changes getting in the
way of seeing the actual changes.  Please make sure comments are
addressed before resubmitting the same thing again.

https://spectrum-os.org/lists/archives/spectrum-devel/87v7jyj5a3.fsf@alyssa.is
https://spectrum-os.org/lists/archives/spectrum-devel/87ikfdapmx.fsf@alyssa.is

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]