[Merge] livecd-rootfs:canary-build into livecd-rootfs:ubuntu/master

Dan Bungert mp+440431 at code.launchpad.net
Wed Apr 5 19:35:16 UTC 2023


Quick review complete, I didn't try to weigh in on the models as I know so little about them.

Diff comments:

> diff --git a/live-build/auto/config b/live-build/auto/config
> index db1da0d..b582e3c 100755
> --- a/live-build/auto/config
> +++ b/live-build/auto/config
> @@ -731,6 +738,59 @@ case $PROJECT in
>  						;;
>  				esac
>  				;;
> +			canary)
> +				PASSES_TO_LAYERS="true"
> +				# the minimal layer, for minimal installs
> +				add_task minimal minimal standard ubuntu-desktop-minimal ubuntu-desktop-minimal-default-languages
> +				add_package minimal cloud-init
> +				# the standard layer, contains all base common packages for later layers (we're splitting out the snaps)
> +				add_task minimal.standard ubuntu-desktop ubuntu-desktop-default-languages

yea, I guess not if we're starting it with the subiquity snap instead of u-d-i

> +				# the classic layer, basically only contains snaps from the standard tasks
> +				add_pass minimal.standard.classic
> +				mv config/package-lists/livecd-rootfs.snaplist.chroot_minimal.standard.full \
> +					config/package-lists/livecd-rootfs.snaplist.chroot_minimal.standard.classic.full
> +				# the live layer, contains all packages for the live session installer
> +				# TODO: we should probably add the kernel per KERNEL_FLAVOURS
> +				add_package minimal.standard.live linux-generic casper lvm2 mdadm

in theory lvm2 / mdadm get installed live from the pool as needed

> +				remove_package minimal.standard.live ubiquity-frontend-gtk
> +				# the enhanced-secureboot layer, contains all packages for the enhanced secureboot install
> +				add_package minimal.standard.enhanced-secureboot cryptsetup
> +				# now let's create the neccessary catalog files
> +				cat <<-EOF > config/minimal.catalog-in.yaml
> +					name: "Ubuntu Desktop (minimized)"
> +					description: >-
> +					  A minimal but usable Ubuntu Desktop.
> +					id: ubuntu-desktop-minimal
> +					type: fsimage-layered
> +					variant: desktop
> +					locale_support: langpack

Without having read the relevant code yet, I suspect you want to declare `locale_support: none` or `locale_support: locale-only` like live-server does, or keep this but add the derive_language_layers calls.

> +				EOF
> +				cat <<-EOF > config/minimal.standard.catalog-in.yaml
> +					name: "Ubuntu Desktop"
> +					description: >-
> +					  A full featured Ubuntu Desktop.
> +					id: ubuntu-desktop
> +					type: fsimage-layered
> +					variant: desktop
> +					locale_support: langpack
> +					default: yes
> +				EOF
> +				cat <<-EOF > config/minimal.standard.classic.catalog-in.yaml
> +					id: ubuntu-desktop
> +					variations:
> +					  classic:
> +					    path: minimal.standard.squashfs
> +				EOF
> +				cat <<-EOF > config/minimal.standard.enhanced-secureboot.catalog-in.yaml
> +					id: ubuntu-desktop
> +					variations:
> +					  enhanced-secureboot:
> +					    path: minimal.standard.enhanced-secureboot.squashfs
> +					    snapd_system_label: enhanced-secureboot-desktop
> +				EOF
> +				/usr/share/livecd-rootfs/checkout-translations-branch \
> +					https://git.launchpad.net/subiquity po config/catalog-translations
> +				;;
>  			*)
>  				touch config/universe-enabled
>  				PASSES_TO_LAYERS="true"
> diff --git a/update-source-catalog b/update-source-catalog
> index 6616d60..f217784 100755
> --- a/update-source-catalog
> +++ b/update-source-catalog
> @@ -28,45 +28,56 @@ with open(opts.template) as fp:
>      template = yaml.safe_load(fp)
>  
>  
> -template['size'] = int(opts.size)
> -template['path'] = opts.squashfs
> -
> -en_name = template['name']
> -en_description = template['description']
> -
> -template['name'] = {'en': en_name}
> -template['description'] = {'en': en_description}
> -
> -for mo in glob.glob(os.path.join(opts.translations, '*.mo')):
> -    with open(mo, 'rb') as fp:
> -        t = gettext.GNUTranslations(fp=fp)
> -    t_name = t.gettext(en_name)
> -    if t_name != en_name:
> -        lang = os.path.splitext(os.path.basename(mo))[0]
> -        template['name'][lang] = t_name
> -    t_description = t.gettext(en_description)
> -    if t_description != en_description:
> -        lang = os.path.splitext(os.path.basedescription(mo))[0]
> -        template['description'][lang] = t_description
> -    if opts.langs is not None:
> -        template['preinstalled_langs'] = opts.langs.split(',')
> -
> -output.append(template)
> +id = template['id']
> +for entry in output:
> +    # First, look if this source catalogue template id is already present.
> +    # If so, use the template to extend the existing entry with additional
> +    # variations.
> +    if entry['id'] == id:
> +        if 'variations' not in template:
> +            print("Non unique id in source catalog but no variations!")
> +            sys.exit(1)
> +        if 'variations' not in entry:
> +            entry['variations'] = {}
> +        for k, variation in template['variations'].items():
> +            variation['size'] = int(opts.size)
> +        entry['variations'].update(template['variations'])

suggestion: entry.setdefault('variations', {}).update(template['variations']), then can drop the above 'not in entry' check

> +        break
> +else:
> +    # No entry with this id found, so add a new one.
> +    template['size'] = int(opts.size)
> +    template['path'] = opts.squashfs
> +
> +    en_name = template['name']
> +    en_description = template['description']
> +
> +    template['name'] = {'en': en_name}
> +    template['description'] = {'en': en_description}
> +
> +    for mo in glob.glob(os.path.join(opts.translations, '*.mo')):
> +        with open(mo, 'rb') as fp:
> +            t = gettext.GNUTranslations(fp=fp)
> +        t_name = t.gettext(en_name)
> +        if t_name != en_name:
> +            lang = os.path.splitext(os.path.basename(mo))[0]
> +            template['name'][lang] = t_name
> +        t_description = t.gettext(en_description)
> +        if t_description != en_description:
> +            lang = os.path.splitext(os.path.basedescription(mo))[0]
> +            template['description'][lang] = t_description
> +        if opts.langs is not None:
> +            template['preinstalled_langs'] = opts.langs.split(',')
> +
> +    output.append(template)
>  
>  default_count = 0
> -ids = set()
>  for entry in output:
>      if entry.get('default', False):
>          default_count += 1
> -    ids.add(entry['id'])
> -
>  
>  if default_count > 1:
>      print("Too many defaults in source catalog!")
>      sys.exit(1)
>  
> -if len(ids) != len(output):
> -    print("Non unique ids in %s!" % output)
> -
>  with open(opts.output, 'w') as fp:
>      yaml.dump(output, fp)


-- 
https://code.launchpad.net/~ubuntu-core-dev/livecd-rootfs/+git/livecd-rootfs/+merge/440431
Your team Ubuntu Core Development Team is requested to review the proposed merge of livecd-rootfs:canary-build into livecd-rootfs:ubuntu/master.




More information about the Ubuntu-reviews mailing list