[bug] cross compiles should pass 3 settings to CMake

datalogics-robb commented 3 years ago

When we upgraded Conan to 1.30.2 from 1.29.2 our SPARC Solaris 32-bit builds broke. After some investigation, and discussion with CMake people,

https://gitlab.kitware.com/cmake/cmake/-/issues/21433

Conan passes -DCMAKE_SYSTEM_NAME="SunOS" in this case, but it should also pass -DCMAKE_SYSTEM_PROCESSOR="sparc" (same as Conan setting "arch")

without the -DCMAKE_SYSTEM_PROCESSOR flag, Cmake is being told to cross build, but not all the information it needs.

The CMake issue referenced above also suggests we need to pass -DCMAKE_SYSTEM_VERSION, which is probably also true in general.

Environment Details (include every applicable attribute)

Operating System+version: SPARC Solaris 11.3 64-bit OS
Compiler+version: gcc-9.2.0
Conan version: 1.30.2
Python version: 3.7.6

Steps to reproduce (Include if Applicable)

Conan profile:

[settings] os=SunOS os_build=SunOS arch=sparc arch_build=sparc compiler=gcc compiler.version=9.2 compiler.libcxx=libstdc++ build_type=Release [options] [build_requires] [env] PATH=['/opt/cmake-3.18/bin', '/opt/gcc-9.2.0/bin:/usr/gnu/bin'] CC=/opt/gcc-9.2.0/bin/gcc CXX=/opt/gcc-9.2.0/bin/g++ CFLAGS=-pthread -m32 CXXFLAGS=-pthread -m32 boost:CXXFLAGS=-pthread -D_FILE_OFFSET_BITS=32 -m32

Use this profile to build 32-bit binaries with CMake on a 64-bit SPARC Solaris 11.3 machine. In the CMakeLists.txt query for CMAKE_SYSTEM_PROCESSOR, and it will be empty.

Logs (Executed commands with output) (Include/Attach if Applicable)

memsharded commented 3 years ago

Hi @datalogics-robb

I am trying to understand, if this is a regression or a "complete this bugfix" request. As you said this broke will upgrading Conan, just to make sure, you didn't upgrade other things like CMake, did you?

Then the issue appears because in Conan 1.30 Conan now passes -DCMAKE_SYSTEM_NAME="SunOS", but it didn't define it in Conan 1.29? If that is the case, then your request would be to instead of fixing this as a regression, to accept it as a good fix, but that it request some extra changes?

datalogics-robb commented 3 years ago

Correct. Conan 1.29 does NOT add the -DCMAKE_SYSTEM_NAME flag to the cmake command line, and the setting for CMAKE_SYSTEM_PROCESSOR is correctly computed. When using Conan 1.30, CMAKE_SYSTEM_PROCESSOR is empty.

cmake -G Unix Makefiles -DCMAKE_BUILD_TYPE=Release -DCONAN_IN_LOCAL_CACHE=OFF -DCONAN_COMPILER=gcc -DCONAN_COMPILER_VERSION=9.2 -DCONAN_CXX_FLAGS=-m32 -DCONAN_SHARED_LINKER_FLAGS=-m32 -DCONAN_C_FLAGS=-m32 -DCONAN_LIBCXX=libstdc++ -DCMAKE_INSTALL_PREFIX=/raid/proj/procyon/checkouts-procyon/robb/apdfl-18box/pdfl18_all/sparc32_build/package -DCMAKE_INSTALL_BINDIR=bin -DCMAKE_INSTALL_SBINDIR=bin -DCMAKE_INSTALL_LIBEXECDIR=bin -DCMAKE_INSTALL_LIBDIR=lib -DCMAKE_INSTALL_INCLUDEDIR=include -DCMAKE_INSTALL_OLDINCLUDEDIR=include -DCMAKE_INSTALL_DATAROOTDIR=share -DCMAKE_MODULE_PATH=/raid/proj/procyon/checkouts-procyon/robb/apdfl-18box/pdfl18_all/sparc32_build -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON -DCONAN_EXPORTED=1 -DCMAKE_VERBOSE_MAKEFILE=TRUE -DDLI=ON -DDL_PROD_PLUS_STRING=P1e -DDL_MAJOR_VERSION=18 -DDL_MINOR_VERSION=0 -DDL_SUB_VERSION=3 -DDL_DEV_STAGE=128 -DDL_COPYRIGHT=2020 -DDL_PDFL_REL_VERSION=5 -DDL_ACE_REL_VERSION=2 -DDL_ADOBEXMP_REL_VERSION=0 -DDL_AGM_REL_VERSION=3 -DDL_ARE_REL_VERSION=1 -DDL_AXE8EXPAT_REL_VERSION=0 -DDL_BIB_REL_VERSION=1 -DDL_BIBUTILS_REL_VERSION=1 -DDL_COOLTYPE_REL_VERSION=1 -DDL_JP2K_REL_VERSION=0 -DDL_PDFPORT_REL_VERSION=3 -DDL_PDFSETTINGS_REL_VERSION=0 -DDL_PDFFLATTENER_REL_VERSION=2 -DDL_PDFPROCESSOR_REL_VERSION=1 -DDL_XPS2PDF_REL_VERSION=1 -Wno-dev /raid/proj/procyon/checkouts-procyon/robb/apdfl-18box/pdfl18_all

It would be perfectly fine to simply eliminate the new flag that conan 1.30 added as a fix.

datalogics-robb commented 3 years ago

IMHO, a 32-bit build on a 64-bit OS does not have all the aspects of a cross compile that require a cross compile to have special handling. Autoconf used to create a binary, then attempt to run it, if it didn't run Autoconf declared that you need to explicitly pass the "host triplet"s for a cross.

memsharded commented 3 years ago

Hi @datalogics-robb

I am trying to reproduce this, but if the os == os_build and arch == arch_build, the CMAKE_SYSTEM_NAME is not added at all. I am doing this test:

    def test_cmake_sunos(self):
        # https://github.com/conan-io/conan/issues/8052
        settings = Settings.loads(get_default_settings_yml())
        settings.os = "SunOS"
        settings.os_build = "SunOS"
        settings.compiler = "gcc"
        settings.compiler.version = "9.2"
        settings.compiler.libcxx = "libstdc++"
        settings.arch = "sparc"
        settings.arch_build = "sparc"

        conanfile = ConanFileMock()
        conanfile.settings = settings

        cmake = CMake(conanfile)
        self.assertNotIn('CMAKE_SYSTEM_NAME', cmake.command_line)
        self.assertIn('-G "Unix Makefiles"', cmake.command_line)

And seems to be fine. Is it possible that you have it defined elsewhere? In an env-var or something?

datalogics-robb commented 3 years ago

This message is also only output when using Conan 1.30:

 Cross-build from 'SunOS:sparcv9' to 'SunOS:sparc'

memsharded commented 3 years ago

So it is possible that your profile above is not fully correct and it is:

arch=sparc
arch_build=sparcv9

instead of?

arch=sparc
arch_build=sparc

Using arch_build=sparcv9 indeed shows the issue, now I am checking what is easier and makes more sense, if remove CMAKE_SYSTEM_NAME or add the others. Please tell me.

datalogics-robb commented 3 years ago

I've verified that this also resolves the issue using Conan 1.30.

SSE4 commented 3 years ago

conan-io / conan