rust-lang / cmake-rs

Rust build dependency for running cmake
https://docs.rs/cmake
Apache License 2.0
301 stars 121 forks source link

Cross-compile improvements break the `x86_64-fortanix-unknown-sgx` target #170

Open sburton84 opened 1 year ago

sburton84 commented 1 year ago

I am using this crate to compile CMake projects using the x86_64-fortanix-unknown-sgx target to run under Intel SGX, and the latest update to this crate (v0.1.49) is causing the following errors that weren't present in the previous version:

  System is unknown to cmake, create:
  Platform/unknown to use this system, please post your config file on discourse.cmake.org so it can be added to cmake
  CMake Error: try_run() invoked in cross-compiling mode, please set the following cache variables appropriately:
     RUN_RESULT (advanced)
     RUN_RESULT__TRYRUN_OUTPUT (advanced)

I believe this is likely due to the changes made in https://github.com/rust-lang/cmake-rs/pull/158. These result in -DCMAKE_SYSTEM_NAME=unknown being passed to CMake, which it does not understand.

In my own crates I am able to fix this by overriding the variables manually as follows:

    if env::var("CARGO_CFG_TARGET_ENV").unwrap() == "sgx" {
        config.define("CMAKE_CROSSCOMPILING", "OFF");
        config.define("CMAKE_SYSTEM_NAME", "Linux");
        config.define("CMAKE_SYSTEM_PROCESSOR", "x86_64");
    }

But I might not always have the option of making this modification for any third-party crates.

I'm not sure what the correct fix for this would be, possibly just disabling the setting of the cross-compile variables when the x86_64-fortanix-unknown-sgx target is being used?

sburton84 commented 1 year ago

I have just realised that the main reason for this failure is that this particular CMake project uses try_run and this may not be an issue with projects that don't use try_run because the "System is unknown" messages are actually only warnings.

As such it seems I can also fix the error by explicitly setting the RUN_RESULT and RUN_RESULT__TRYRUN_OUTPUT as mentioned in the error message. Though I'm still not sure this should really be necessary for SGX as you're not really compiling for a different CPU so, unlike with a true cross-compile, the test binary being executed by try_run can just be run directly.

thomcc commented 1 year ago

I don't see an obvious way of having this "just work" with try_run, but this may be inexperience on my part.

FWIW, I don't have access to the target so I won't fix it myself, but I'm certainly willing to take changes to avoid issues that we hit on more obscure targets (so long as they're reasonable and not extremely complicated for the sake of a single target). So I think it would be reasonable to tweak that logic for SGX's case.

This seems like it might be more to do with the specific CMake script you're using, though (although I'm not certain about this).

sburton84 commented 1 year ago

I don't see an obvious way of having this "just work" with try_run

Yeah, certainly there's no way of automatically setting RUN_RESULT and RUN_RESULT__TRYRUN_OUTPUT, or of automatically determining whether the test binary alone should be excluded from cross-compiling. In this specific case the binary is used for detecting which CPU extensions are available, so it should give the same results whether it's compiled for SGX or not, as long as it's on the same CPU, but that might not always be the case, and there might be projects using try_run with binaries that would produce misleading output if run outside of SGX.

I guess the only question is whether just unilaterally disabling cross-compiling and setting CMAKE_CROSSCOMPILING=OFF for x86_64-fortanix-unknown-sgx targets would be appropriate, or whether that could itself have unintended effects...? Despite being implemented as a target-triple, compiling code for SGX is not really a cross-compile in the typical sense.