On Tue, Feb 26, 2019 at 07:04:16PM +0100, Andrea Bolognani wrote: > On Tue, 2019-02-26 at 11:00 +0000, Daniel P. Berrangé wrote: > > Debian's filesystem layout has a nice advantage over Fedora which is > > that it can install non-native RPMs > > s/RPMs/packages/ ;) > > [...] > > guests/host_vars/libvirt-debian-9/main.yml | 44 +++++++++++++ > > guests/host_vars/libvirt-debian-sid/main.yml | 45 ++++++++++++++ > > guests/lcitool | 65 +++++++++++++++++++- > > guests/vars/mappings.yml | 59 ++++++++++++++++++ > > 4 files changed, 211 insertions(+), 2 deletions(-) > > It would be great if the changes to code and inventory were in > separate commits. It should be trivial to split: just add the new > information to the inventory first, and code relying on them being > present last. > > [...] > > +++ b/guests/host_vars/libvirt-debian-9/main.yml > > @@ -21,3 +21,47 @@ os_name: 'Debian' > > os_version: '9' > > > > ansible_python_interpreter: /usr/bin/python3 > > + > > +arches: > > + aarch64: > > + package_arch: arm64 > > + abi: aarch64-linux-gnu > > + cross_gcc: gcc-aarch64-linux-gnu > > We previously agreed to store this information in cross-build.yml > rather than main.yml, and that's what it looked like in v3... Can > you please move it back there?
I moved it back here as the pacakge_arch / abi fields are not specific to cross builds. They're general information about the distro / toolchain... but this all goes away with a static mapping in the code instead so it doesn't matter. > I also questioned a few respins back whether we need to have the > Debian 9 configuration at all. For MinGW builds we use Fedora Rawhide > as the base, so using Debian Sid for cross-builds would make sense, > especially since it supports more target architectures: we are only > going to build a single set of buildenv-cross-* container images > anyway... What we do with MinGW right now is not a good example to follow. There are enough differences betweeen software versions that I see failures building on stable Fedora that don't appear on rawhide & vica-versa. This is particularly causing me pain for virt-viewer release builds as the msi build process is frequently failing due to changed DLL versions. So when I add MSI builds to the CI, I'll want MinGW packages across mulitple distro versions. The other problem is that both rawhide & sid are rolling unstable distros which break. Right now I can't even test the cross arch builds on Sid because some recent update has broken the ability to install the cross compiler toolchain at all. So only supporting the unstable Sid is not a good idea. > > + i686: > > + package_arch: i386 > > + abi: i686-linux-gnu > > + cross_gcc: gcc-i686-linux-gnu > > The value of 'abi' doesn't look right: based on eg. > > https://packages.debian.org/sid/i386/libglib2.0-dev/filelist > > it should be 'i386-linux-gnu' instead. The 'abi' is what is prefixed on the toolchain binaries, and so for this purpose i686-linux-gnu is actually correct: https://packages.debian.org/sid/amd64/gcc-i686-linux-gnu/filelist > The value of 'cross_gcc', though, is unfortunately correct, which > means that in this case we can't automatically figure out the name > of the cross-compiler package from the ABI like I described above. > Bummer :( Actually we can. > [...] > > +++ b/guests/lcitool > > @@ -367,6 +367,10 @@ class Application: > > > > add_hosts_arg(dockerfileparser) > > add_projects_arg(dockerfileparser) > > + dockerfileparser.add_argument( > > + "-x", "--cross-arch", > > + help="cross compiler architecture", > > + ) > > You added the add_gitrev_arg() helper for --git-revision despite the > fact that it's only used by a single action, so it would make sense > to have add_crossarch_arg() here for consistency. > > [...] > > @@ -537,10 +541,23 @@ class Application: > > os_name = facts["os_name"] > > os_version = facts["os_version"] > > os_full = os_name + os_version > > + cross_facts = None > > You initialize 'cross_facts' here... > > [...] > > + if args.cross_arch is not None: > > + if "arches" not in facts: > > + raise Error("Non x86_64 arches not supported for this > > host") > > + if args.cross_arch not in facts["arches"]: > > + raise Error("Arch {} not supported for this host".format( > > + args.cross_arch)) > > + if "cross_gcc" not in facts["arches"][args.cross_arch]: > > + raise Error("Arch {} cross compiler not supported for this > > host".format > > + (args.cross_arch)) > > + > > + cross_build_facts = facts["arches"][args.cross_arch] > > ... but then set 'cross_build_facts' and use the latter from here > on. And this is why dynamic languages are so much fun! O:-) > > I'd stick with 'cross_facts' for the variable name, since we already > have 'pkgs'/'cross_pkgs' and 'keys'/'cross_keys'. This dict goes away if we use a static mapping table. > [...] > > @@ -80,6 +84,7 @@ mappings: > > > > avahi: > > deb: libavahi-client-dev > > + deb-cross-arch: foreign > > pkg: avahi > > rpm: avahi-devel > > So, I'll come right out and say that I don't love the idea of storing > this information in the mappings file. That said, doing so allows us > to reuse a lot of existing infrastructure and I don't really have a > better alternative to offer at the moment, so let's just do it this > way :) The compelling reason for using mappings.xml is that it provides a single source of information for packages. This way when adding new packages to the list, it is more likely we'll remember to add the right cross arch info at the same time. > One last point is about the possible values: 'skip' and 'native' are > perfectly intuitive, but I feel like 'foreign' is not that great a > name, and it also already has a meaning in the context of Debian's > Multi-Arch implementation which might muddy the waters for people. > I think 'cross' would be a better value. "foreign" is the logical inverse of "native", so I think it is actually the right word to use here. "cross" is misleading as we're not installing a cross-built package, we're installing the native built package from the foreign architecture. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
