[Merge] livecd-rootfs:uc18-kassel into livecd-rootfs:ubuntu/master

Ɓukasz Zemczak lukasz.zemczak at canonical.com
Wed May 15 10:48:24 UTC 2019


Review: Needs Information

This generally looks good. I included a few 'propositions' inline that might be good to consider, but I the way it's done now is ok as well.

But I do want to stop and think for a moment, similarly to Michael's position earlier. Is this for a temporary solution or do we want this to be the final design for such subarchs? Since normally, for official devices we support, I would rather expect us to have a special model assertion created, signed and uploaded to the store instead. This model assertion would have the list of needed snaps added to required-snaps, so no extra_snaps handling is needed. Then, support in livecd-rootfs would be simply adding the new model name to the model switch-case.

Is that not a good idea for some reason? I mean, do we only want to have 'generic' model assertions in the store and then customize them elsewhere? But I guess model assertions aren't just for generic cases I'd say, would be quite a waste if we didn't use them. Or maybe it's the fact that we want those 'extra snaps' to be removable from the end image?

Anyway, this obviously would only work for ubuntu-core. For classic we still have to figure something else out.

What do you think?

Diff comments:

> diff --git a/live-build/auto/config b/live-build/auto/config
> index 46246eb..70b4a42 100755
> --- a/live-build/auto/config
> +++ b/live-build/auto/config
> @@ -355,12 +356,14 @@ case $IMAGEFORMAT in
>  
>  		if [ $PROJECT = "ubuntu-core" ]; then
>  			# snap-based core images
> +			EXTRA_SNAPS=""
>  
>  			case $MODEL in
>  				pc-amd64|pc-i386)
> -					UBUNTU_IMAGE_ARGS="--image-size 3700M" ;;
> -				*)
> -					UBUNTU_IMAGE_ARGS="" ;;
> +					[ -z "${SUBARCH:-}" ] \

If we stick with this approach, maybe we could move this down to the switch-case where we actually do per-subarch configuration (next to defining  EXTRA_SNAPS)?

> +					&& UBUNTU_IMAGE_ARGS="--image-size 3700M"
> +					;;
> +				*) ;;
>  			esac
>  			case $SUITE in
>  				xenial)
> @@ -371,6 +374,15 @@ case $IMAGEFORMAT in
>  					MODEL="ubuntu-core-18-${MODEL#pc-}" ;;
>  			esac
>  
> +			case "$ARCH+${SUBARCH:-}" in

We already have a switch-case for ARCH and SUBARCH when determining the model. Maybe we could move the EXTRA_SNAPS variable to the top (where UBUNTU_IMAGE_ARGS is defined now) and do the EXTRA_SNAPS filling in the top model-deterining switch-case? Sure, extra snaps don't matter for classic, so it might be an unused assignment in case where we'd want (one of these days) build a classical image with this subarch, but to me it seems a bit better conceptually. But that might only be my personal feeling.

> +				amd64+kassel)
> +					EXTRA_SNAPS="core bluez alsa-utils pulseaudio udisks2"
> +					;;
> +				*) ;;
> +			esac
> +			for snap in $EXTRA_SNAPS; do
> +				UBUNTU_IMAGE_ARGS="$UBUNTU_IMAGE_ARGS --extra-snaps $snap"
> +			done
>  			echo "IMAGEFORMAT=$IMAGEFORMAT" >> config/common
>  			echo "UBUNTU_IMAGE_ARGS=\"$UBUNTU_IMAGE_ARGS\"" >> config/common
>  			# Store model assertion in top dir to get it picked up later as a build artifact


-- 
https://code.launchpad.net/~ubuntu-core-dev/livecd-rootfs/+git/livecd-rootfs/+merge/366327
Your team Ubuntu Core Development Team is subscribed to branch livecd-rootfs:ubuntu/master.



More information about the Ubuntu-reviews mailing list