[libvirt] [jenkins-ci PATCH v6 3/4] lcitool: support generating cross compiler dockerfiles

[Andrea Bolognani]

On Thu, 2019-02-28 at 15:53 +0000, Daniel P. Berrangé wrote:
[...]
> +    @staticmethod
> +    def convert_native_arch_to_abi(native_arch):

I'd s/convert_// for all these small helper functions.

> +        archmap = {
> +            "aarch64":  "aarch64-linux-gnu",

Some extra whitespace sneaked in, both here...

> +            "armv6l": "arm-linux-gnueabi",
> +            "armv7l": "arm-linux-gnueabihf",
> +            "i686":  "i686-linux-gnu",

... and here.

[...]
> +    @staticmethod
> +    def convert_native_arch_to_deb_cross_gcc(native_arch):
> +        abi = Util.convert_native_arch_to_abi(native_arch)
> +        return "gcc-" + abi

This one we can open-code, I think. It's absolutely trivial and we
use it only once.

[...]
> +        def add_cross_arch_arg(parser):
> +            parser.add_argument(
> +                "-x", "--cross-arch",
> +                help="cross compiler architecture",

"target architecture for cross compilation"?

[...]
> +                # Intentionally a separate RUN command from the above
> +                # so that the common packages of all cross-built images
> +                # share a docker image layer.

s/docker/Docker/

[...]
> +                    ENV PKG_CONFIG_LIBDIR "/usr/lib/{cross_abi}/pkgconfig"

We discussed elsewhere how this can't work for i686, but I'm okay
with merging this version of the patchset and figuring out a way
to address that in a follow-up.

[...]
> +++ b/guests/vars/mappings.yml
> @@ -58,6 +58,13 @@
>  #    x86_64-deb: libxen-dev
>  #    x86_64-Fedora: xen-devel
>  #
> +# In parallel with this 'cross-arch-XXX:' entries can used to set the

This is now called 'cross-policy-XXX', and you have a hunk updating
the documentation accordingly in the next patch. Please make sure
it's it *this* patch instead :)


With the above addressed,

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization