Add sysroot directory paths to export file

[kyrias]

These need to be available so that we can tell bindgen which sysroot to use, as bindgen will otherwise use the system include files^0 to generate bindings.

[kyrias]

CI is failing due to the latest cargo-generate release not having any build artifacts for Linux. (cargo-generate/cargo-generate#708)

[georgik]

Thank you, @kyrias, for the contribution.

@SergioGasquez Please, review and test the change. If it works, we could merge it to 1.63.0.0.

[SergioGasquez]

Hi @kyrias! Thanks for your contribution! Do you mind elaborating on how shall we update our project later to use those environment variables? I guess we should update the build.rs of our project files, but then we will need some kind of logic in build.rs to check if some of those 3 variables (IDF_SYSROOT_ESP32S3, IDF_SYSROOT_ESP32S2 or IDF_SYSROOT_ESP32) is not empty.

Also, we are only setting the variables when we are not installing esp-idf is that something intended? I guess this should also apply when we install esp-idf, am I wrong?

Will we still need the variables once #rust-bindgen/issues/1229 gets closed?

[kyrias]

How I was planning on using them is something like this:

    let target = env::var("TARGET").expect("Build target was not set");
    let sysroot = if target.contains("-esp32s3-") {
        env::var("IDF_SYSROOT_ESP32S3").expect(
            "Sysroot environment variable for ESP32-S3 not set.  You might be using a too old toolchain."
        )
    } else {
        panic!("Target {} not supported", target);
    };

And then adding these arguments to the bindgen builder:

        .clang_arg("--sysroot")
        .clang_arg(sysroot)

As for when installing esp-idf, rustbuild doesn't install any compilers in that case, and if the esp-idf install script does, I have no clue where it installs those or if it somehow exposes those paths.

And as for whether they'll still be needed, that depends entirely on how, if at all, they choose to resolve it. If their solution is to say that you just need to specify the correct sysroot, then these will always be required.

[SergioGasquez]

Thanks for the explanation! Don't we need an IDF_SYSROOT_ESP32C3? Added in: https://github.com/esp-rs/rust-build/blob/5c9e32da1d890201c807e3afc32ae7de470648e2/install-rust-toolchain.sh#L625

[kyrias]

Very good point, I'd missed that one. Fixed now. :)

[georgik]

@n3xed Please could you review the patch in context of Embuild?

[N3xed]

Well, in the context of esp-idf-sys and embuild this change shouldn't be needed and we already get the sysroot through other ways.

We either:

1. use the sysroot of the CompileGroup we get from cmake (cmake-file-api) if there is one (the CompileGroup also gives us the includes and defines); or alternatively
2. first get the compiler from cmake and then use that compiler to query the sysroot with --print-sysroot (here)

CMake ultimately gets the compiler(s) that idf_tools.py export outputs or the one from the environment if it's an activated esp-idf environment. The esp-idf cmake build system then selects the right gcc depending on the selected chip. (It's similar for the platformio build).

This could make a difference for upstream crates that want to generate their own bindings since there is currently no nice way to get the used compiler or sysroot. (What we currently export/propagate to dependent build scripts is the used $PATH (here), the path to the esp-idf (here), the C includes and the linker args).

Maybe instead of adding these sysroot environment variables, we should propagate them from the esp-idf-sys build script? (Or are they useful for baremetal? I thought baremetal used LLVM only.)

Note also that it is not required to run the export file to build the esp-idf-sys since setting up the environment to build for the esp32* chips should involve the user as little as possible. So we should never and currently don't depend on such env variables in esp-idf-sys.

[kyrias]

Propagation would only work for downstream dependencies, there would then be no way to tell other dependencies to use the correct sysroot when running e.g. bindgen.

[kyrias]

Would be nice if some kind of decision was made here. To be clear, I think it would be completely reasonable for the esp-idf-sys build script to propagate them such that the simple case is handled more easily. But I do think that they should be exposed separately as well so that they can be used in other places than for direct downstream dependencies.

[kyrias]

While we've switched to espup now, this still needs to be decided and solved somehow.

Until now this has only been an inconvenience but now we're starting to look into using one of the RISC-V chips and bindgen just completely fails when not specifying the sysroot.

[SergioGasquez]

We are trying to move away from GCC as a linker, see https://github.com/esp-rs/esp-hal/pull/357. So basically, in the near future espup won't download GCC at all and:

• no_std will use LLD as linker
• std will use GCC that is downloaded/installed via esp-idf-sys

[kyrias]

Makes sense. Then it would at least be nice if the toolchain that's fetched for building ESP-IDF itself was exposed so that there is some official way of compiling C code.

The annoying thing with just exposing it through esp-idf-sys somehow though is that there would be no reasonable way to have separate crates building C code since then those crates would need to have esp-idf-sys set up to build for a compatible chip. :/

[SergioGasquez]

Makes sense. Then it would at least be nice if the toolchain that's fetched for building ESP-IDF itself was exposed so that there is some official way of compiling C code.

The annoying thing with just exposing it through esp-idf-sys somehow though is that there would be no reasonable way to have separate crates building C code since then those crates would need to have esp-idf-sys set up to build for a compatible chip. :/

Do you mind opening an issue on esp-idf-sys with your suggestions?

[georgik]

Issue transfered to esp-idf-sys project. Closing.