[Merge] ~tchavadar/livecd-rootfs:install-image-definitions-deb-option into livecd-rootfs:ubuntu/master

Steve Langasek mp+464755 at code.launchpad.net
Thu Apr 25 02:19:18 UTC 2024


Review: Needs Fixing

I am willing to merge this general implementation, subject to a few small fix-ups mentioned below.

I do think it would be better for you to do package recipe builds of livecd-rootfs the way the CPC team does, so that the only package you have to branch into your private ppa is livecd-rootfs - instead of creating an entirely new ubuntu-images package.

Diff comments:

> diff --git a/live-build/auto/config b/live-build/auto/config
> index a229f39..9e8df78 100755
> --- a/live-build/auto/config
> +++ b/live-build/auto/config
> @@ -533,10 +533,19 @@ case $IMAGEFORMAT in
>  			IMAGE_PROJECT=$PROJECT
>  			[ "$IMAGE_PROJECT" = "ubuntu-cpc" ] && IMAGE_PROJECT="ubuntu-server"
>  
> -			LB_UBUNTU_IMAGES_REPO="${LB_UBUNTU_IMAGES_REPO:-git://git.launchpad.net/ubuntu-images}"
> -			LB_UBUNTU_IMAGES_BRANCH="${LB_UBUNTU_IMAGES_BRANCH:-$SUITE}"
> -			git clone "$LB_UBUNTU_IMAGES_REPO" -b "$LB_UBUNTU_IMAGES_BRANCH" image-definitions
> -			IMAGE_DEFINITION="image-definitions/$IMAGE_PROJECT-$MODEL.yaml"
> +			if [ -z "${LB_USE_UBUNTU_IMAGES_DEB:-}" ]; then
> +				LB_UBUNTU_IMAGES_ROOT=${LB_UBUNTU_IMAGES_ROOT:-image-definitions}
> +				LB_UBUNTU_IMAGES_REPO="${LB_UBUNTU_IMAGES_REPO:-git://git.launchpad.net/ubuntu-images}"
> +				LB_UBUNTU_IMAGES_BRANCH="${LB_UBUNTU_IMAGES_BRANCH:-$SUITE}"
> +				git clone "$LB_UBUNTU_IMAGES_REPO" -b "$LB_UBUNTU_IMAGES_BRANCH" $LB_UBUNTU_IMAGES_ROOT
> +			else
> +				LB_UBUNTU_IMAGES_ROOT=${LB_UBUNTU_IMAGES_ROOT:-/usr/share/ubuntu-images}

> so people can create it freely if they want to use it in private livefs builds

Supporting such flexibility is an anti-feature.  Please drop support for this being overrideable.

(Honestly, I'm not sure why any of the other variables are overrideable either here since there's no way to set them when invoking livecd-rootfs through launchpad - and if you're not invoking livecd-rootfs through launchpad, why are you using livecd-rootfs at all instead of invoking ubuntu-image directly!)

Also, I think it would be more idiomatic in livecd-rootfs to, instead of setting LB_UBUNTU_IMAGES_ROOT to a different path, copy the contents from /usr/share/ubuntu-images to image-definitions.  (See lines 50ff of live-build/auto/config)

> +				echo "Using $LB_USE_UBUNTU_IMAGES_DEB package instead of git repo.."
> +				sudo apt install $LB_USE_UBUNTU_IMAGES_DEB -y
> +			fi
> +
> +			IMAGE_DEFINITION="$LB_UBUNTU_IMAGES_ROOT/$IMAGE_PROJECT-$MODEL.yaml"
> +			echo "LB_UBUNTU_IMAGES_ROOT=$LB_UBUNTU_IMAGES_ROOT" >> config/common

So you are emitting this value to config/common, but then nothing consumes it.  Please drop.

>  			echo "IMAGE_DEFINITION=$IMAGE_DEFINITION" >> config/common
>  			echo "Configured ubuntu-image to use image definition file $IMAGE_DEFINITION which has the following contents:"
>  			cat "$IMAGE_DEFINITION"
> @@ -568,7 +577,7 @@ case $IMAGEFORMAT in
>  esac
>  
>  if [ "$PREINSTALLED" = "true" ]; then
> -	# LP Bug: 2044154 Enable universe on pre-installed images 
> +	# LP Bug: 2044154 Enable universe on pre-installed images

unrelated whitespace change

>  	touch config/universe-enabled
>  fi
>  


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




More information about the Ubuntu-reviews mailing list