conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.22k stars 979 forks source link

[bug] meson toolchain incorrectly handles cross-compilation on same os/arch tuple. #15392

Closed kuroneko closed 8 months ago

kuroneko commented 9 months ago

Environment details

Steps to reproduce

  1. install a Yocto SDK or similar that produces ABI incompatible binaries on the same arch.
  2. Set up a host profile that has the same os+arch as the build machine, but has an explicit tools.build.cross_building:can_run=no set in [conf].
  3. Attempt to build any meson package and watch it fail when it attempts to test the compilers.

(Note: Support for this to work is necessary as it's entirely possible to have incompatible ABIs between Linux distributions and this is already a problem that plagues binaries hosted by conan-center - In our case, my development distro can't produce binaries that run on our embedded x86_64 distro and vice versa due to ld.so and libc differences so we need to use a cross compilation flow despite the architectures being compatible/identical.)

A quick check of the code shows that MesonToolchain only checks cross_compiling, and not can_run. Even if it's not a full cross_compile, if it can't run the binaries, Meson needs to use the cross_compilation flow.

Logs

... snipped ...
-------- Installing package wayland/1.22.0 (43 of 47) --------
wayland/1.22.0: Building from source
wayland/1.22.0: Package wayland/1.22.0:db57ab7809fab3515e28598fd861124ccd5fedb2
wayland/1.22.0: Copying sources to build folder
wayland/1.22.0: Building your package in /home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b
wayland/1.22.0: Calling generate()
wayland/1.22.0: Generators folder: /home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/build-relwithdebinfo/conan
WARN: deprecated: Please, do not use a Conan option value directly. Convert 'options.enable_libraries' into a valid Pythondata type, e.g, bool(self.options.shared)
WARN: deprecated: Please, do not use a Conan option value directly. Convert 'options.enable_dtd_validation' into a valid Pythondata type, e.g, bool(self.options.shared)
wayland/1.22.0: Generating aggregated env files
wayland/1.22.0: Generated aggregated env files: ['conanbuild.sh']
wayland/1.22.0: Calling build()
wayland/1.22.0: Meson configure cmd: meson setup --native-file "/home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/build-relwithdebinfo/conan/conan_meson_native.ini" "/home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/build-relwithdebinfo" "/home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/src" -Dprefix="/home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/p"
wayland/1.22.0: RUN: meson setup --native-file "/home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/build-relwithdebinfo/conan/conan_meson_native.ini" "/home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/build-relwithdebinfo" "/home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/src" -Dprefix="/home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/p"
DEPRECATION: "pkgconfig" entry is deprecated and should be replaced by "pkg-config"
The Meson build system
Version: 1.3.0
Source dir: /home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/src
Build dir: /home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/build-relwithdebinfo
Build type: native build
Project name: wayland
Project version: 1.22.0

../src/meson.build:1:0: ERROR: Compiler /usr/local/oecore-x86_64/sysroots/x86_64-oesdk-linux/usr/bin/x86_64-oe-linux/x86_64-oe-linux-gcc cannot compile programs.

A full log can be found at /home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/build-relwithdebinfo/meson-logs/meson-log.txt

wayland/1.22.0: ERROR: 
Package 'db57ab7809fab3515e28598fd861124ccd5fedb2' build failed
wayland/1.22.0: WARN: Build folder /home/kuroneko/.conan2/p/b/wayla32f3fb2af00f2/b/build-relwithdebinfo
*********************************************************
Recipe 'wayland/1.22.0' cannot build its binary
It is possible that this recipe is not Conan 2.0 ready
If the recipe comes from ConanCenter, report it at https://github.com/conan-io/conan-center-index/issues
If it is your recipe, check if it is updated to 2.0
*********************************************************

ERROR: wayland/1.22.0: Error in build() method, line 103
    meson.configure()
    ConanException: Error 1 while executing
kuroneko commented 9 months ago

Also noting #11035, it seems that this was overlooked as can_run would appear to be the correct mechanism.

kuroneko commented 8 months ago

bump. This remains an issue and needs to be fixed.

memsharded commented 8 months ago

Implemented in https://github.com/conan-io/conan/pull/15616 as tools.build.cross_building:cross_build configuration conf, to fully control the cross_building() helper behavior from profiles.

kuroneko commented 8 months ago

Implemented in #15616 as tools.build.cross_building:cross_build configuration conf, to fully control the cross_building() helper behavior from profiles.

Can you please explain this in the context of the previous decisions around #11035 and why adding another option was necessary rather than just using the existing one in the way it was intended (despite the name, it's pretty much exactly what it would appear to be for).

My concern is that this is another divergent setting for conan build integration and recipe maintainers to get wrong, and this change will create another ripple of broken recipes as we have to fix them to use the New Logic rather than the pre-existing logic.

memsharded commented 8 months ago

Because those are very different things. armv8 binaries can run in a Mac with intel architecture and viceversa x86_64 binaries can run in a Mac with M1/M2 architecture. So tests can be executed, applications can be run, etc. But still, when you are building a armv8 binary in a intel Mac it is a cross-building scenario, or the same if you are building a x86_64 binary in a M1 Mac, also cross-building.

So can_run and cross_build are different concepts, and no one of them imply the other.

kuroneko commented 8 months ago

Because those are very different things. armv8 binaries can run in a Mac with intel architecture and viceversa x86_64 binaries can run in a Mac with M1/M2 architecture. So tests can be executed, applications can be run, etc. But still, when you are building a armv8 binary in a intel Mac it is a cross-building scenario, or the same if you are building a x86_64 binary in a M1 Mac, also cross-building.

So can_run and cross_build are different concepts, and no one of them imply the other.

Uh, that's a somewhat incomplete opinion on cross-compilation.

In the case where the binaries are transparently executable, then you do not need to use full cross-compilation approaches as binary tests can be executed - these typically fall into the native build flow (ie: not a full cross-compile), whereas the converse, where you are not doing something which fits the normal criteria of cross-building (such as my cross-ABI on same OS & kernel builds) where you cannot run the binaries at all must use a full cross-compile flow.

The can_run test encapsulates that perfectly, and moreover, whilst adding cross_compilation overrides may seem to fix my issue, it does not fix the fact that the logic in the meson tools is using the wrong flag to determine if it should use a cross compilation flow vs native flow.

The distinction as to if something is or isn't a cross-compile is far less useful than the distinction as to if cross-compilation handling is required.

memsharded commented 8 months ago

Uh, that's a somewhat incomplete opinion on cross-compilation.

This is not an opinion on cross-compilation, this is knowledge about what Conan should do based on the needs and feedback of thousands users using Conan.

As an example, there are users doing builds for Android or some Nvidia platforms, from different OSs, Windows, Linux, OSX. Those are clearly cross-builds and all the build system integrations need to configure the toolchains for cross-builds, with the "build" and "host" context profiles being different, tools in the "build" context, binaries for Android/Nvidia in the "host" context, etc. Some of these users have capabilities of running some of those executables from Conan recipes, in the "build" machine, by emulators or remote execution. They need to control whether they can run those executables, depending on the machine doing the cross-build having the running capabilities, like the emulator or the remote connection to the device. While cross_building=True in every case, the can_run can change from False to True to let Conan users control that execution. As I told you, there is no 1:1 correspondence between the two things.

kuroneko commented 8 months ago

Right, so whilst I take your point - if you're going to go so far as to say that conan supports the semi-cross modes through emulation/remote execution despite putting none of the other configuration options in that they need (such as the ability to configure remote execution or emulation options) then I can see the point in adding the distinction, but as this doesn't actually introduce the changes for that, this really feels like you've superficially fixed the problem and ignored the actual one.

Yes, now I can override it using a separate value, but can_run=False should absolutely be forcing meson to be using the cross-compilation workflow even if cross_compiling is not forced (Meson has no choice but to use a full cross-compile in that circumstance because it's native build flow assumes the ability to run the outputs), hence the PR I raised, and ultimately, this issue is not resolved.

kuroneko commented 8 months ago

The other issue, and this is why your approach rubs me completely wrong, is you're mistaking the means for the end.

We know the parameters and behaviours of the systems and tools.

We know what we have and what we want.

As the consumer, we do not particularly care about how Conan or a recipe gets there in the end. What we care about is we get artifacts for the host platform using tools on the build platform. We care that the tools and the recipes have the information they need to make the right decisions.

We do not actually care at the end of the day if the method is that tool's cross-compile method or not. If it's a cross-compile without using explicit cross-compilation support, it's still a cross-compile.

Moreover, the requirements to pick a specific build approach are not necessarily uniform by build tool or project. The tools and recipes should be making the judgements about which flows to use based on the properties of the host and build platforms specified, not because we set some magic flag on - that sort of black and white thinking just results in being equally useless when somebody hits another edgecase. This is pretty much a text-book anti-pattern of making the user deal with internal details, rather than having them deal with the details they have and can control.

can_run obeys the correct philosophy in that it's something that the consumer can easily observe and specify and give a way for the tool helpers and recipes to detect and act appropriately for the case where the default build/host matching doesn't make the right assumption by describing the observable property that should alter the tools behaviour.

You claim it's not connected to cross-compilation, but at the end of the day it is - it's a key determinant property for if a full cross-compile strategy is required vs a softer one for many tools (such as cmake, autotools and meson all alike). That was recognized in #11035, even if only by accident - because in the circumstances described, you have the ability for the outputs to run transparently, many tools stop needing to assume it's a cross-build and can treat it like a native one with just the normal tool overrides and they'll get a cross-compiled output, but without using a cross-compiled flow. It's still a cross-compile, but it's not using the tools internal cross-compile mechanisms because they're superfluous in those cases.

The thing is, can_run as a property works both ways, just as it being explicitly true may permit some tools to elide a cross-compile, if it's explicitly false, it should force some platforms to treat the build explicitly as a cross-compile (even if it's technically not), and it will be by tool or by recipe that that decision is relevant.

If you're going to insist the new flag is useful, that's fine - I disagree for the points above - but at least fix the Meson bug too before closing this ticket.

kuroneko commented 7 months ago

fundamental issue reported in this ticket is still not fixed DESPITE @memsharded's claims otherwise.

Per last line of the report:

"Even if it's not a full cross_compile, if it can't run the binaries, Meson needs to use the cross_compilation flow."

@memsharded trying to pass this off as a case for a global "is_crosscompiling" does not fix the actual bug reported which is meson must respect can_run (which defaults to using the same decision process as cross_compiling, but also respects the ability to run objects transparently).

jcar87 commented 7 months ago

Hi @kuroneko - is the configuration implemented in https://github.com/conan-io/conan/pull/15616 still resulting in errors when building meson packages?