[Merge] ~rbalint/livecd-rootfs:ubuntu/master into livecd-rootfs:ubuntu/master

David Krauser david.krauser at canonical.com
Wed Nov 13 15:29:51 UTC 2019


Review: Needs Fixing

So it sounds like the root of the issue is that Python is attempting to talk with PackageKit over dbus, but it can't for some reason? So we avoid using python+dbus, and use apt-mark instead?

Would you mind adding a more descriptive explanation to the git commit message? The message includes what you changed, but not a good explanation of why.

Also added inline comments.

Diff comments:

> diff --git a/live-build/auto/build b/live-build/auto/build
> index 24a5536..bf1a4ad 100755
> --- a/live-build/auto/build
> +++ b/live-build/auto/build
> @@ -440,7 +440,11 @@ EOF
>  		(cd chroot && find usr/share/doc -maxdepth 1 -type d | xargs du -s | sort -nr)
>  		echo END docdirs
>  
> -		/usr/share/livecd-rootfs/minimize-manual chroot
> +		auto_packages=$(/usr/share/livecd-rootfs/minimize-manual chroot)
> +		if [ -n "$auto_packages" ]; then
> +			Chroot chroot apt-mark auto $auto_packages
> +		fi
> +		[ -z "$(/usr/share/livecd-rootfs/minimize-manual chroot 2> /dev/null)" ]

So in the case that we hit the count limit, will the build just fail? Is that what we want? Should we print out a more useful error message?

>  
>  		clean_debian_chroot
>  	fi
> diff --git a/minimize-manual b/minimize-manual
> index 5dcd9d3..1518ac6 100755
> --- a/minimize-manual
> +++ b/minimize-manual
> @@ -36,7 +36,8 @@ def main():
>                  print("    Visiting", pkg, file=sys.stderr)
>  
>                  if pkg not in roots and pkg not in ubiquity_depends:
> -                    pkg.mark_auto()
> +                    if not pkg.is_auto_installed:
> +                        print(pkg.name)

The docstring at the top of this file is no longer accurate:

 """Minimize the number of manually installed packages in the image.
 
 Finds all manually installed meta packages, and marks their dependencies
 as automatically installed.
 """

Would you mind updating it?

Also with your change, minimize-manual no longer does any work - it just prints out package names to do work on. Can we rename the script to be more indicative of the new functionality?

>  
>                  for dep in (pkg.installed.dependencies +
>                              pkg.installed.recommends):


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



More information about the Ubuntu-reviews mailing list