[Merge] ~toabctl/livecd-rootfs:master-replace-apt-key-lp#1948929 into livecd-rootfs:ubuntu/master
Robert C Jennings
mp+410866 at code.launchpad.net
Wed Oct 27 15:25:35 UTC 2021
Review: Abstain
Hey Thomas, saw this in passing and thought I'd give a quick review... hope it's helpful. (Marking this as 'abstain' because I don't want to block you in case I don't get back to this thread in a timely fashion).
Diff comments:
> diff --git a/live-build/auto/build b/live-build/auto/build
> index 950d95d..93820ac 100755
> --- a/live-build/auto/build
> +++ b/live-build/auto/build
> @@ -431,7 +431,7 @@ deb file:/var/lib/preinstalled-pool/ $LB_DISTRIBUTION $LB_PARENT_ARCHIVE_AREAS
> chroot/var/lib/preinstalled-pool/dists/$R_CODENAME/Release
> mv config/gnupg/Release.asc \
> chroot/var/lib/preinstalled-pool/dists/$R_CODENAME/Release.gpg
> - apt-key --keyring chroot/etc/apt/trusted.gpg add config/gnupg/pubring.gpg
This command added the contents of pubring.gpg to a shared keyring file and you could identify each key by listing keys on the keyring, but...
> + cp config/gnupg/pubring.gpg /etc/apt/trusted.gpg.d/
this "new" model puts individual keyring files in the trusted.gpg.d/ directory. Finding a key could mean iterating over all the keyrings in the directory. For that reason, and for general hygiene, I think you should choose a more descriptive filename than pubring.gpg to describe which keys are contained in the file (or what they relate to, or what added them). I think you should also be defensive and check that the target filename doesn't already exist in the target directory before your 'cp' to avoid hard to trace issues in the future.
> find chroot/var/lib/preinstalled-pool/ -name Packages | xargs rm
>
> Chroot chroot "apt-get update"
--
https://code.launchpad.net/~toabctl/livecd-rootfs/+git/livecd-rootfs-1/+merge/410866
Your team Ubuntu Core Development Team is subscribed to branch livecd-rootfs:ubuntu/master.
More information about the Ubuntu-reviews
mailing list