Re: [libvirt] [jenkins-ci PATCH v2 8/9] lcitool: support generating cross compiler dockerfiles

On Tue, 2019-02-05 at 17:53 +0000, Daniel P. Berrangé wrote: [...] Is i386 really not supported as a cross compilation target on Debian 9? It seems like it would be such a low hanging fruit... [...] I don't think this belongs in docker.yml, since there's nothing specific to Docker here: we could just as well use this information to build a virtual machine in which to perform cross builds. I would create a new fact file, eg. cross-build.yml, for this. To be completely honest I'm not 100% sold on the structure and placement of the data, but I can't quite put my fingers on why that's the case - which of course makes it very difficult to come up with an alternative suggestion ;) So let's use this structure for now, we can always change it later if needed. I cannot really understand what you're trying to say with this comment, can you try to reword it please? For these and all other blacklisted packages, you should list the name of the mapping (eg. netcf) rather than the concrete Debian package name (eg. libnetcf-dev). Then of course you'll have to act on the blacklist earlier, before mapping is performed. This is not a prefix, and the package is not GCC :) I would use 'cross_gcc', which incidentally is also the name of the variable you use in the Python script to store this value. (More on this later, though.) Same here, you later use 'cross_prefix' in the script but also store it as 'TARGET' in the environment... I'd suggest 'cross_target'. [...] crossbuild-essential-mipsel is available in Debian 9, so you should use that instead. [...] > +++ b/guests/host_vars/libvirt-debian-sid/docker.yml > @@ -1,2 +1,64 @@ > --- > docker_base: debian:sid > +cross_build: > + # appears to be dropped from the 'sid' tree > + - sheepdog So it has! And it's apparently not going to be in the next Ubuntu release, either. This has nothing to do with cross compilation, though: you should just update the regular mappings file, in a separate commit, based on this information. [...] Debian Sid has crossbuild-essential-* packages available for all cross compilation targets, so please use those everywhere. Another idea would be to drop these from the facts entirely and have a bunch of mappings like cross-gcc-mipsel: Debian9: gcc-mipsel-linux-gnu DebianSid: crossbuild-essential-mipsel instead; then you could programmatically add cross-gcc-$arch to the package list in the script before mapping is performed. That sounds to me like it would be much nicer. [...] s/CROSS-ARCH/CROSS_ARCH/ [...] This first check is kinda pointless, since the one right after it would be triggered when trying to generate a Dockerfile for cross compilation and the required information has not been provided by maintainers. raise Error("Host {} doesn't support cross compilation".format(host)) [...] I can see how sticking with the Debian-style architecture names makes this bit trivial, but I'm still entirely convinced we should use the same architecture names as libvirt does. Especially once we'll have integrated this into libvirt's own build system, the difference in names would introduce needless friction, as the target audience (libvirt developers) is certainly more familiar with the architecture names used in that project than Debian's. We can avoid that easily enough, by performing the necessary conversion in the script with a small dedicated utility function. [...] Oh, clever! :) The same considerations about formatting the list of packages as in patch 7/9 also apply here. As you mention in a previous commit message, each ENV statement will create and additional layer, and it's considered best practice to have as few of those as possible. Another reason why I don't like this is that it's different from what we already do for the "cross compilation" target we already support, MinGW: in that case, we have a single image with pristine environment, and we inject the additional variables only when actually running the build. In my opinion, that's a saner way to tweak the build, since the interaction is very explicit and doesn't require any knowledge of other components that might even be hosted, as is the case here, in a separate git repository! Additionally, and I only thought about this now :) did we consider the possibility of providing a single Debian image with all cross compilation packages installed, rather than one image per target architecture? Assuming that's feasible, it would also fit very nicely with how we have a single Fedora image that we use for both MinGW variants... I apologize for the length of this message. If it's any consolation, it clearly shows that your patches were good food for thought :) -- Andrea Bolognani / Red Hat / Virtualization