[Merge] ~mwhudson/livecd-rootfs/+git/livecd-rootfs:cpc-file-list into livecd-rootfs:ubuntu/master

Ɓukasz Zemczak lukasz.zemczak at canonical.com
Tue Nov 12 23:04:39 UTC 2019


Review: Approve

Ok, this looks good. I included two comments inline. Those comments are more like comments (though exercises), not something I'd explicitly need changing.

The one thing that needs to be fixed is the conflict in debian/changelog though! But once that's done, we can merge.

Diff comments:

> diff --git a/live-build/functions b/live-build/functions
> index c5a6344..2fdcbe9 100644
> --- a/live-build/functions
> +++ b/live-build/functions
> @@ -51,6 +51,11 @@ create_manifest() {
>      echo "create_manifest call to dpkg-query finished."
>      ./config/snap-seed-parse "${chroot_root}" "${target_file}"
>      echo "create_manifest call to snap_seed_parse finished."
> +    if [ "$PROJECT" = ubuntu-cpc ]; then
> +        echo "create_manifest creating file listing."
> +        local target_filelist=${2/.manifest}.filelist

This seems to be a bashism, so we need to take care that any ubuntu-cpc code that would call this has to use bash instead of sh. Currently that seems to be the case, so it's not a problem (live-build/lb_binary_layered calls create_manifest, but that is non-ubuntu-cpc). Would be best if we could have that bash-independent, but I guess that's like a very very very minor thing.

> +        (cd "${chroot_root}" && find) > "${target_filelist}"

Normally I'd recommend doing 1> "${target_filelist}" 2> /dev/null to not log garbage, but I guess this actually is very unlikely to print errors in this case, right?

> +    fi
>      echo "create_manifest finished"
>  }
>  


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



More information about the Ubuntu-reviews mailing list