[Merge] ~ubuntustudio-dev/livecd-rootfs:studio-new-installer into livecd-rootfs:ubuntu/master

Steve Langasek mp+456901 at code.launchpad.net
Tue Dec 5 19:07:18 UTC 2023


Review: Needs Fixing



Diff comments:

> diff --git a/live-build/auto/config b/live-build/auto/config
> index 20e280b..e9ab8ea 100755
> --- a/live-build/auto/config
> +++ b/live-build/auto/config
> @@ -846,8 +846,32 @@ case $PROJECT in
>  		;;
>  
>  	ubuntustudio-dvd)
> -		add_task install minimal standard ubuntustudio-desktop ubuntustudio-audio ubuntustudio-graphics ubuntustudio-video ubuntustudio-photography
> +		# By default Ubuntu Studio now ships the new installer.
> +		touch config/universe-enabled
> +		PASSES_TO_LAYERS="true"
>  		KERNEL_FLAVOURS=lowlatency
> +		add_task install minimal standard ubuntustudio-desktop ubuntustudio-audio ubuntustudio-graphics ubuntustudio-video ubuntustudio-photography

The first argument to 'add_task' is the name of the squashfs layer to add this task to.  'install' is not the correct name for a squashfs layer in a new-style multilayer image in the existing vernacular.  (You could use it as the name for one of your layers, but the rest of your code doesn't implement this.)

The names of the layers are heirarchical dot paths; e.g. 'minimal.standard.live' means it is the 'live' layer which is mounted on top of the standard layer which is mounted on top of the minimal layer (overlayfs).

UbuntuStudio does not offer a 'minimal' install option and your other changes don't implement such a thing.  So this should be 'add_task standard [...]'.

> +		add_task minimal.standard ubuntustudio-desktop ubuntustudio-audio ubuntustudio-graphics ubuntustudio-video ubuntustudio-photography desktop-default-languages

Redundant with the previous line, as explained.  Except you also have the addition here of 'desktop-default-languages', which is not an ubuntustudio task at all: this is an Ubuntu-specific seed with no automatic equivalent for other flavors.  So this line should just be stricken.

> +		add_task minimal.standard.live ubuntustudio-live

"add_task standard.live [...]"

> +		add_package minimal cloud-init
> +		remove_package minimal.standard.live ubiquity-fronten-gtk
> +		add_snap minimal.standard.live ubuntustudio-system-installer/classic
> +		add_package minimal.standard.live linux-$KERNEL_FLAVOURS casper

Similarly, the add/remove package functions take a layer as the first argument, so need to be aligned as per the above.

But also, these commands all *override* the seeds.  You should just avoid doing this, and update your seeds instead so that no overrides are required.

One might be inclined to say it's a bad idea to put the kernel list in the live seed since that means the flavors will be encoded in both the seeds and in livecd-rootfs, but, well, I've actually done exactly that for Ubuntu with linux-generic-hwe-22.04 to fix an issue with package duplication between the package pool and the squashfs because germinate was ignorant of the kernels being manually added to the squashfs.  So there's precedent.

> +		seeded_langs="$(get_seeded_languages ubuntustudio-desktop)"
> +		echo "$seeded_langs" | tr ' ' ',' > config/seeded-languages
> +		derive_language_layers minimal.standard ubuntustudio-desktop ubuntustudio-desktop-default-languages "$seeded_langs"

get_seeded_languages () {
        # We assume any seed name of the form ${no_lang_seed}-${foo} where
        # ${foo} is only two or three characters long is a default language
        # seed.

This is a no-op for UbuntuStudio and I think you should just omit it unless you have plans to implement such a thing.  (It's also a no-op for budgie and should probably just be dropped there, too.)

> +		cat <<-EOF > config/standard.catalog-in.yaml
> +			name: "Ubuntu Studio Desktop"
> +			description: >-
> +			  A full featured Ubuntu Studio Desktop.
> +			id: ubuntustudio-desktop
> +			type: fsimage-layered
> +			variant: desktop
> +			locale_support: langpack
> +			default: yes
> +		EOF

This all looks correct as far as it goes.  There have been some issues with unexpected outcomes from cargo-culted subiquity install catalogs though, because they're not yet widely used, so we might find that we have to iterate.

Also I'm going to tag the subiquity team in here on the review because I don't know what 'locale_support: langpack' does and I'm not sure whether it does something sensible on flavor images.  (Curiously, our ubuntu OEM image build config does NOT use this value.)

> +		/usr/share/livecd-rootfs/checkout-translations-branch \
> +			https://git.launchpad.net/subiquity po config/catalog-translations
>  		;;
>  
>  	ubuntu-server)


-- 
https://code.launchpad.net/~ubuntustudio-dev/livecd-rootfs/+git/livecd-rootfs/+merge/456901
Your team Ubuntu Studio Development is subscribed to branch ~ubuntustudio-dev/livecd-rootfs:studio-new-installer.




More information about the Ubuntu-reviews mailing list