Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support unknown architectures #582

Closed
wants to merge 1 commit into from
Closed

Conversation

jhol
Copy link
Contributor

@jhol jhol commented Oct 30, 2023

Conan supports non-standard architectures through the installation of custom settings.yml files. However, conan-cmake currently expects to be able to translate the architecture from the CMake naming scheme to the Conan naming scheme, and fails with an error if the architecture is unknown.

This patch resolves the issue by making the script simply pass through the CMake architecture to Conan unchanged in these cases.

Conan supports non-standard architectures through the installation of
custom settings.yml files. However, conan-cmake currently expects to be
able to translate the architecture from the CMake naming scheme to the
Conan naming scheme, and fails with an error if the architecture is
unknown.

This patch resolves the issue by making the script simply pass through
the CMake architecture to Conan unchanged in these cases.
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say this is valid, probably less blocking than not setting an arch at all. I'll ask for second opinion to @jcar87 though

@jcar87
Copy link
Contributor

jcar87 commented Nov 15, 2023

@jhol - is there a specific use case that isn't covered - that is, a specific value for CMAKE_SYSTEM_PROCESSOR that we are unable to map to a Conan architecture?

We now have the ability to define CONAN_HOST_PROFILE to tell Conan (as invoked by cmake-conan) which profile to use - in case the autodetection is insufficient: https://github.com/conan-io/cmake-conan/tree/develop2#customizing-conan-profiles

The problem with having this fallback, is that there isn't anything that guarantees that the CPU architecture as known by CMake, maps to anything that is known by Conan in settings.yml - it could still result in an error, and it offers no hint whatsoever. But hard to comment without a specific use case.

@jhol
Copy link
Contributor Author

jhol commented Nov 15, 2023

I'm working on a project that uses a proprietary MIPS-like architecture on a device provided to us by a commercial partner organization. Sorry, I'm not at liberty to disclose the exact name of the architecture, but we have received a forked version of GCC to target this device.

In this case, we install a custom settings.yml file using conan install, to register the architecture with Conan.

Once this is installed, Conan is perfectly able to behave as we would normally expect. In our settings.yml file, we gave the architecture the same name as CMAKE_SYSTEM_PROCESSOR has, which I believe comes from gcc -dumpmachine.

The current implementation is certainly broken. It should not be possible for execution to proceed out of this CMake function with _ARCH unset.

It is true to say that my change is making something of a leap to say that for unknown arches, the Conan arch name is the same the GCC arch name, but I don't think this is an unreasonable expectation.

@jcar87
Copy link
Contributor

jcar87 commented Nov 15, 2023

The current implementation is certainly broken. It should not be possible for execution to proceed out of this CMake function with _ARCH unset.

Not quite. This is covered here: https://github.com/conan-io/cmake-conan/tree/develop2#customizing-conan-profiles, the detected settings are overlaid on top of the default profile, which is assumed to exist. So the expectation would be that profile with name default has an architecture that Conan understands, even if cmake-conan cannot map from the one detected in CMake. This PR would change that behaviour in that it now might result in a non-working profile - although I do not expect this to be a widespread issue.

You can define the CMake variable CONAN_HOST_PROFILE to either avoid autodetection altogether, or to pass a profile file that only specifies specific values (for examples, ones that autodetection cannot currently reason about). This is specially reserved in those cases for custom settings and profiles that we have no visibility of, like the situation you mention.

I have two additional questions:

  • Am I correct in assuming that a CMake Toolchain file is being passed to CMake? If I'm not mistaken, CMake expects CMAKE_SYSTEM_PROCESSOR to be defined in a toolchain when cross-building - Unless the build is happening on the device with the MIPS processor?
  • Are you using conan config install or similar to propagate the customised settings.yml?

If the answer is yes in both cases, it may be worth a shot adding a profile (or set of profiles) alongside settings.yml, and defining CONAN_HOST_PROFILE in the Cmake toolchain files, to reference those.

@jhol
Copy link
Contributor Author

jhol commented Nov 15, 2023

If I'm not mistaken, CMake expects CMAKE_SYSTEM_PROCESSOR to be defined in a toolchain when cross-building

Partially correct. I was wrong to say the value is set from gcc -dumpmachine. The value is set in CMakeLists.txt before the call to project(...).

Are you using conan config install or similar to propagate the customised settings.yml?

Another mistake on my part. Yes, we are using conan config install.

If the answer is yes in both cases, it may be worth a shot adding a profile (or set of profiles) alongside settings.yml, and defining CONAN_HOST_PROFILE in the Cmake toolchain files, to reference those.

We could try that, certainly.

@jhol
Copy link
Contributor Author

jhol commented Nov 15, 2023

Well it works to use a profile. So this PR can be closed.

Though I will give this feedback:

In our system we support multiple target devices using CMake options. Therefore, CMAKE_SYSTEM_PROCESSOR is set depending on the target platform value. On this basis, it is convenient to be able to configure conan_provider.cmake programatically.

In our case, our CMakeLists.txt is already generating a Conan profile file based on other configuration options, so it's not a huge problem to add a [settings] section to it to specify the arch etc. in addition to the [options] section we had previously.

It would be much nicer to have a programmatic way to do all this. I would like to be able to pass CMake lists of Conan settings and options, and have conan_provider.cmake create the requisite profile file, rather than it having to be done by the application build script.

Certainly, the original design of cmake-conan was more convenient in this regard.

@jhol jhol closed this Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants