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

Custom target support #423

Open
xTachyon opened this issue Jul 17, 2023 · 4 comments
Open

Custom target support #423

xTachyon opened this issue Jul 17, 2023 · 4 comments

Comments

@xTachyon
Copy link

I tried setting a custom target while also using build-std:

set(Rust_CARGO_TARGET "${CMAKE_CURRENT_SOURCE_DIR}/rust_lib/custom-x86_64-unknown-linux-gnu.json")

FetchContent_MakeAvailable(Corrosion)
corrosion_import_crate(MANIFEST_PATH rust_lib/Cargo.toml
    FLAGS -Z build-std=std
)

target_link_libraries(the_bin PRIVATE rust_lib)

The build seems to have succeeded, but this warning appeared:

CMake Warning at build/_deps/corrosion-src/cmake/FindRust.cmake:113 (message):
  Failed to parse target-triple `custom-x86_64-unknown-linux-gnu`.Corrosion
  attempts to link required C libraries depending on the OS specified in the
  Rust target-triple for Linux, MacOS and windows.

  Note: If you are targeting a different OS you can surpress this warning by
  setting the CMake cache variable
  `CORROSION_NO_WARN_PARSE_TARGET_TRIPLE_FAILED`.Please consider opening an
  issue on github if you encounter this warning.
Call Stack (most recent call first):
  build/_deps/corrosion-src/cmake/FindRust.cmake:750 (_corrosion_parse_target_triple)
  build/_deps/corrosion-src/cmake/Corrosion.cmake:63 (find_package)
  build/_deps/corrosion-src/CMakeLists.txt:73 (include)

Removing the "custom-" also removes the warning, but I feel a custom target should have a slightly different name than the original, so it's clear what you're compiling to when you're checking the logs.

Is setting CORROSION_NO_WARN_PARSE_TARGET_TRIPLE_FAILED the official™ solution here? At the very least, I feel the documentation should mention this, or maybe I just missed it.

@jschwe
Copy link
Collaborator

jschwe commented Jul 17, 2023

Corrosion attempts to link required C libraries depending on the OS specified in the
Rust target-triple for Linux, MacOS and windows.

This part of the warning is actually outdated and should/will be removed.

Corrosion does still attempt to parse the target triple and sets the following variables for users convenience:

set(Rust_CARGO_TARGET_ARCH "${rust_arch}" CACHE INTERNAL "Target architecture")
set(Rust_CARGO_TARGET_VENDOR "${rust_vendor}" CACHE INTERNAL "Target vendor")
set(Rust_CARGO_TARGET_OS "${rust_os}" CACHE INTERNAL "Target Operating System")
set(Rust_CARGO_TARGET_ENV "${rust_env}" CACHE INTERNAL "Target environment")

I guess we don't need a warning message here anymore, the variables could just remain unset.
You could also rename your target to something like x86_64-unknown-custom_linux-gnu. Corrosion splits the string at - dashes, so you can use underscores in the OS or env part of the target triple.

@ogoffart
Copy link
Collaborator

Not strictly related, but i'm also getting this error with xtensa-esp32s3-none-elf
So maybe we need to add esp32[a-z0-9]* as known vendors.
Or just deprecate this target parsing if not needed since it's error prone.

@jschwe
Copy link
Collaborator

jschwe commented Jul 18, 2023

I see I already added esp into the list of known vendors for the target riscv32imc-esp-espidf. Is this vendor related to your esp32s3? If yes we could just replace esp with esp[a-z0-9]*.

Or just deprecate this target parsing if not needed

In some places we need to check the ABI, and CMake seems to treat that as an implementation detail / internal variable. This mostly affects the msvc abi, but since regular clang can also output msvc abi, I don't know a good way how to reliably detect the abi, aside from parsing the Rust target triple. Strictly speaking we don't need to parse it, but I think it might be useful for users. At least for the big targets, this is tested and works - I just don't actively follow which new targets / vendors are added.

@jschwe
Copy link
Collaborator

jschwe commented Jul 29, 2023

Update from my side:

  • I relaxed the vendor parsing for espressif, so all current esp toolchains should be parsed correctly.

I guess we don't need a warning message here anymore

I retract this statement. I want to keep the warning, since corrosion still does some target specific things based on the result of the parsed target triple, mainly determining file extensions or prefixes (e.g. .exe / .dll on windows OS, .dylib for darwin). Currently besides windows and mac all other operating systems fall into the other category, and basically are assumed to be the same as linux, but this could be different for new targets.

CMake also (at least partially) provides such information for the target OS, but we also support the hostbuild option, so we end up needing to parse the information from the target triple anyway.

Is setting CORROSION_NO_WARN_PARSE_TARGET_TRIPLE_FAILED the official™ solution here

It ends up working out despite the warning because you are targetting linux which falls into others category. But if you e.g. defined a custom target for windows or mac or some other OS that uses different file extensions, then things would start failing, so giving a warning here seems like the right direction.

I wonder if in the future we may be able to query file extensions for a given target triple directly from cargo / rustc.

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

No branches or pull requests

3 participants