Experimental esp-idf native cmake build

[N3xed]

This is very work-in-progress work to compile the esp-idf using cmake.
Currently, it compiles (tested for esp32 on windows) but doesn't run when it has been flashed.

Please give feedback and/or tell me if you find something wrong.

[ivmarkov]

This is a great start, thank you very much for this contribution!

I'll review the patch thoroughly, but let me make some general remarks first:

• Ideally, we should be moving most of your sdk_build_support to cargo-pio. In the meantime, cargo-pio is no longer just for PlatformIO. For example, this, this, this, this and this are generally usable utilities even outside the context of PlatformIO. My idea for cargo-pio was to later rename it to something like embedded-build, where cmake-driven and PlatformIO-driven builds can co-exist in separate modules. Regarding cargo sub-commands, the cargo pio subcommand, would stay, just like cargo pio-link, but the latter will be likely renamed to cargo linkproxy as it has nothing to do with PlatformIO. Your cmake CodeModel utilities for example, seem generic enough and usable even outside of the context of building the ESP-IDF SDK, so those would be a great addition to cargo-pio (i.e. future embedded-build or however we decide to name this crate).

• The outcome of the above is that all your Bindgen-specific work in this PR can take advantage of the bindgen utilities in cargo-pio. Ditto for deriving the MCU based on the Rust target. Ditto for the python executable detection and running (and btw I don't mind merging your cmd! macros work in cargo-pio as well)

• We should not be removing the PlatformIO based build. It should stay, as another option

As for why the build does not run - you are probably not applying these patches to ESP-IDF. All of these that don't start with master_ are necessary for the ESP-IDF version you are using, and some of them (the missing atomics one starting with master_) will be necessary even for ESP-IDF master.

[N3xed]

Thank you for the response!

Ideally, we should be moving most of your sdk_build_support to cargo-pio. In the meantime, cargo-pio is no longer just for PlatformIO.

I agree. I only looked at cargo-pio sparsely and didn't think it was to be more general purpose.

Your cmake CodeModel utilities for example, seem generic enough and usable even outside of the context of building the ESP-IDF SDK, so those would be a great addition to cargo-pio

These are just some utilities that I threw together to get a working example. I intended to contribute them to the cmake crate (maybe under a feature) at some point. But I agree there should be a more general purpose library for compiling embedded stuff.

The outcome of the above is that all your Bindgen-specific work in this PR can take advantage of the bindgen utilities in cargo-pio. Ditto for deriving the MCU based on the Rust target. Ditto for the python executable detection and running (and btw I don't mind merging your cmd! macros work in cargo-pio as well)

That's good to know, will try to do this. And this first try was just to get an initial working example as fast as possible, without considering all the things (like all OS/esp-idf version/... related things).

We should not be removing the PlatformIO based build. It should stay, as another option

That was also my intention (should have stated that more clearly).

As for why the build does not run - you are probably not applying these patches to ESP-IDF. All of these that don't start with master_ are necessary for the ESP-IDF version you are using, and some of them (the missing atomics one starting with master_) will be necessary even for ESP-IDF master.

I've also considered this (need to try it, though I doubt it will work), but I've dismissed it because I thought if this was the problem the linker would've complained. I think this problem has more to do with the way I'm linking the esp-idf. I currently combine all *.a libraries into a single library that gets linked with the rust program and all other linker flags. I think this approach was too naive, though I'll be doing more debugging right now.

[ivmarkov]

I've also considered this (need to try it, though I doubt it will work), but I've dismissed it because I thought if this was the problem the linker would've complained. I think this problem has more to do with the way I'm linking the esp-idf. I currently combine all *.a libraries into a single library that gets linked with the rust program and all other linker flags. I think this approach was too naive, though I'll be doing more debugging right now.

Most of these patches are bugfixes ant not really pollyfils.

But you are probably right - it might be the way you are assembling the final static lib, as link order matters a lot with ESP-IDF. For example, they use link order to overwrite some symbols from newlib. What I do is really to take the "native" link arguments from PIO, and pass them - exactly in the same order and exactly as they are - to the linker.

[ivmarkov]

• Regarding link command line length on Windows: strange, this is what I actually receive on Windows. Aside from being weird, this is actually taking care of the command line length.
• As for some link args being relative, that's annoying and I am also fixing it in a bit of a hacky way.

[N3xed]

Okay, so I've figured some things out.

As I've told you already, I had problems with the link command line length and that is because cmake generates 362 arguments whereas SCons (the build-system platformio uses) generates only 140. This is because the esp-idf has approximately a million circular references and cmake is fine with just using the same library multiple times in the command line while SCons uses --start-group/--end-group.

But I'm really not sure how the esp-idf even links with platformio, because some -u flags towards the end don't even have a corresponding library that defines the symbol (undefined by -u). Seems like I had the wrong idea about the -u flag.

But anyhow, to fix this for cmake I'll just have to do more processing of the linker arguments, to also incorporate --start-group/--end-group (or -(/-) for short). If we incorporate --start-group/--end-group we'll end up with a lot less arguments, but still not as few as platformio (if we do it right).

[ivmarkov]

As I've told you already, I had problems with the link command line length and that is because cmake generates 362 arguments whereas SCons (the build-system platformio uses) generates only 140. This is because the esp-idf has approximately a million circular references and cmake is fine with just using the same library multiple times in the command line while SCons uses --start-group/--end-group.

What is still a mystery for me is how come cmake itself does not have a problem with these 362 link parameters, given that - in the end - it has to call the native linker and pass these 362 link parameters to it? Are we missing something there?

But anyhow, to fix this for cmake I'll just have to do more processing of the linker arguments, to also incorporate --start-group/--end-group (or -(/-) for short). If we incorporate --start-group/--end-group we'll end up with a lot less arguments, but still not as few as platformio (if we do it right).

Yes, we can try this way, but we should bear in mind that any "post-processing" on top of the cmake-generated link arguments we do would be fragile as in that it will be manually implemented effort which makes a lot of ESP-IDF-specific assumptions around what these parameters are. And it might break with a subsequent next release of ESP-IDF. What PlatformIO does is exactly such a workaround - it is interrogating the cmake build system about its linker flags, and then manually pushing these into the Scons build system. At places, it does assumptions (related to linker scripts) and instead of taking those from native cmake, it is just assuming those reside at certain locations inside the ESP-IDF directory tree. This is by the way the reason why I cannot compile ESP-IDF master with PlatformIO ATM - because the PlatformIO python code assumes there are some linker scripts which reside at certain locations in the ESP-IDF and is blindly - without checking if these really exist - putting references to them on the linker command line. Well, these do exist in ESP-IDF V4.3, but no longer exist in ESP-IDF master. Guess what happens.

So the above goes to say - can't we explore other paths to solve the "too long list of arguments" problem? For example, if GCC (&LD) do support passing the arguments to them in a file, we can build a temp file, shuffle all cmake-generated link arguments there and then just pass the temp file to the linker?

And also as per above - can we get to the root cause of why we are having the "too long list of arguments" problem, and cmake itself - when calling the native linker - doesn't?

[N3xed]

So the above goes to say - can't we explore other paths to solve the "too long list of arguments" problem? For example, if GCC (&LD) do support passing the arguments to them in a file, we can build a temp file, shuffle all cmake-generated link arguments there and then just pass the temp file to the linker?

That doesn't work either because even if I pass all the arguments for the esp-idf in a response file, gcc then just calls collect2 with all argument files expanded. And on windows there is a strict limit of 32767 characters for the arguments (a limit of CreateProcess), which is overshot by a mile (50502). The reason this doesn't happen with only cmake is that, like already mentioned, it uses relative paths. Converting all paths to absolute ones inflates the 13448 characters command line to 50502 minus the arguments rust adds (which is about 100 (9238 chars)).

[ivmarkov]

So the above goes to say - can't we explore other paths to solve the "too long list of arguments" problem? For example, if GCC (&LD) do support passing the arguments to them in a file, we can build a temp file, shuffle all cmake-generated link arguments there and then just pass the temp file to the linker?

That doesn't work either because even if I pass all the arguments for the esp-idf in a response file, gcc then just calls collect2 with all argument files expanded. And on windows there is a strict limit of 32767 characters for the arguments (a limit of CreateProcess), which is overshot by a mile (50502). The reason this doesn't happen with only cmake is that, like already mentioned, it uses relative paths. Converting all paths to absolute ones inflates the 13448 characters command line to 50502 minus the arguments rust adds (which is about 100 (9238 chars)).

Got it, great analysis.

I'm starting to think of bigger hacks. First of all, you anyway need to pass all the linker args from esp-idf-sys up to the binary crate itself. This is because cargo:link-arg does not really work transitively (that is: anymore; it used to, for a short period of time due to a regression, but they closed this loophole). I assume you are doing that with a mechanism similar to mine. If so, prior to emitting the linker args with cargo:link-arg (or whatever the name was), can't we symlink the relative-path linker arguments into the out directory of the binary crate? Or even copy them there? I think the out directory of the binary crate is the "current" directory during the build of the binary crate. And I hope that is still the current directory during linking, but then who knows.

Another hack: if you take advantage of my cargo pio-link linker proxy, we might educate it (with a special parameter) to switch back just before calling the GCC linker to a "current directory", which would be the current directory of the esp-idf cmake build. This way you can keep your relative paths, and that might work provided that the Rust libs themselves use absolute paths (don't remember anymore, honestly).

[N3xed]

can't we symlink the relative-path linker arguments into the out directory of the binary crate? Or even copy them there? I think the out directory of the binary crate is the "current" directory during the build of the binary crate.

Didn't even think about symlinks, or if that will even work on windows. But also cargo runs rustc in the workspace root as it seems (rust-lang/cargo#8947), so that would mean we'd have to change the working directory anyway (so that we don't have to pollute the workspace root).

If we change the working directory though, as you said, we need cargo to emit absolute paths, which it does in my case but I don't know if this is portable behavior (ie. that cargo never emits relative paths). If that is so we can just cd into the cmake build directory and all problems are solved (at least until cargo generates a link line that is more than 32K-13K from esp-idf).

I've also looked if somehow gcc itself could use response files in its invocations and this is indeed possible. But it seems that gcc has to be configured with --with-gnu-ld so that this works (more info here, here and here) but other than that maybe someone more knowledgable about gcc could shed some light on this.

[ivmarkov]

I've also looked if somehow gcc itself could use response files in its invocations and this is indeed possible. But it seems that gcc has to be configured with --with-gnu-ld so that this works (more info here, here and here) but other than that maybe someone more knowledgable about gcc could shed some light on this.

I think the question to @igrr would then be: how is Espressif compiling their GCC-based toolchains for Windows? With --with-gnu-ld or not?

[ivmarkov]

If we change the working directory though, as you said, we need cargo to emit absolute paths, which it does in my case but I don't know if this is portable behavior (ie. that cargo never emits relative paths). If that is so we can just cd into the cmake build directory and all problems are solved (at least until cargo generates a link line that is more than 32K-13K from esp-idf).

Well we can at least try this path. You'll need a linker proxy similar to mine. It will just do a chdir() to a directory passed as a special parameter, then run the GCC linker, and then chdir() back.

[N3xed]

As the commit says, I've implemented a workaround that simply copies the cmake build output to the workspace root. This solution isn't great but it is the simplest right now. Making a symlink instead of copying requires elevated permissions on windows.

Well we can at least try this path. You'll need a linker proxy similar to mine. It will just do a chdir() to a directory passed as a special parameter, then run the GCC linker, and then chdir() back.

I don't like that we have to use an extra tool just to work around this, but if we did this it would require us (at least on windows) to parse the response file that cargo gives the linker which contains its command line. We need to do this to handle quoted arguments and paths with spaces. The current carg-pio linker wrapper doesn't do this right now unless I'm mistaken. So the quick fix right now is just to copy the esp-idf directory from the cmake build output to the workspace dir, and adding its name to .gitignore.

But I think the best solution forward is to make a linker wrapper which handles the above. Even if a new build of gcc would use response files internally, we would still have to sanatize all relative paths with absolute ones, which can also go wrong if we get a linker arg that we don't expect. Or we'd have to parse the linker args entirely which is a lot more work.

That's just my reasoning, please tell me what you think.

Also, it compiles and runs fine now. I don't know about the patches though. How should these be applied knowing that using cmake one can specify every version? For which esp-idf version are the non-master patches?

[ivmarkov]

As the commit says, I've implemented a workaround that simply copies the cmake build output to the workspace root. This solution isn't great but it is the simplest right now. Making a symlink instead of copying requires elevated permissions on windows.

I'll look into that later.

Well we can at least try this path. You'll need a linker proxy similar to mine. It will just do a chdir() to a directory passed as a special parameter, then run the GCC linker, and then chdir() back.

I don't like that we have to use an extra tool just to work around this, but if we did this it would require us (at least on windows) to parse the response file that cargo gives the linker which contains its command line. We need to do this to handle quoted arguments and paths with spaces. The current carg-pio linker wrapper doesn't do this right now unless I'm mistaken. So the quick fix right now is just to copy the esp-idf directory from the cmake build output to the workspace dir, and adding its name to .gitignore.

I didn't do it because it is still a bit of a hack, and moreover - I don't think ESP-IDF supports paths with whitespaces on it anyway. But yeah, more diligent processing of that file would be necessary. Not a rocket science, though.

But I think the best solution forward is to make a linker wrapper which handles the above. Even if a new build of gcc would use response files internally, we would still have to sanatize all relative paths with absolute ones, which can also go wrong if we get a linker arg that we don't expect. Or we'd have to parse the linker args entirely which is a lot more work.

In the previous paragraph you say that you kind of don't like the idea of a linker proxy just to workaround the long file paths. Here you say that the best solution forward is to make a linker wrapper. I'm a bit confused. For me, these things (linker proxy & linker wrapper) are one and the same thing. You agree?

That's just my reasoning, please tell me what you think.

Well, I definitely prefer a linker wrapper = linker proxy over spilling out stuff outside of the out directories, which is something Cargo explicitly advises against. It is still a hack, but probably the better hack.
One little additional benefit of having a linker proxy is that you don't have to have the GCC linker (= Espressif toolchain) on your path as you can pass the GCC tollchain to the proxy as an argument. You might say "big deal", but it actually is a win, if you consider that we might be downloading the toolchains during the esp-idf-sys crate build. The linker proxy solves this chicken-and-egg problem.

Also, it compiles and runs fine now. I don't know about the patches though. How should these be applied knowing that using cmake one can specify every version? For which esp-idf version are the non-master patches?

I'm surprised that it works for you without you applying the thread-local patch at least. Without that one, it should break right after "joining the threads" in the demo binary crate.

The non-master patches are for V4.3.

[N3xed]

In the previous paragraph you say that you kind of don't like the idea of a linker proxy just to workaround the long file paths. Here you say that the best solution forward is to make a linker wrapper. I'm a bit confused. For me, these things (linker proxy & linker wrapper) are one and the same thing. You agree?

Yes they're the same. I don't like this solution as it requires a tool that the user has to install beforehand. But it's the best solution because we don't have to sanitize the linker args if/when gcc is updated (or I guess rebuilt), if that doesn't happen it's the only solution. But that doesn't mean I have to like it ^^.

Btw. for me the difference between this linker proxy tool and cmake (I guess that's not even needed, we could install this in the build script too) or python is that the latter are really common if you do anything with programming while the former is not.

I didn't do it because it is still a bit of a hack, and moreover - I don't think ESP-IDF supports paths with whitespaces on it anyway.

I think the esp-idf doesn't even have to support them, as the paths are relative. Nevermind some paths are also absolute, but cmake probably handles this? Edit: Nope.

[ivmarkov]

In the previous paragraph you say that you kind of don't like the idea of a linker proxy just to workaround the long file paths. Here you say that the best solution forward is to make a linker wrapper. I'm a bit confused. For me, these things (linker proxy & linker wrapper) are one and the same thing. You agree?

Yes they're the same. I don't like this solution as it requires a tool that the user has to install beforehand. But it's the best solution because we don't have to sanitize the linker args if/when gcc is updated (or I guess rebuilt), if that doesn't happen it's the only solution. But that doesn't mean I have to like it ^^.

Btw. for me the difference between this linker proxy tool and cmake (I guess that's not even needed, we could install this in the build script too) or python is that the latter are really common if you do anything with programming while the former is not.

Except that our linker proxy is a cargo subcommand. This means two things:

• It is the most natural thing to install by a Rust developer
• It is very easy to install and update. What can be simpler than cargo install cargo-pio? For both install and update? No $PATH changes, no sudo - nothing. I've yet to see somebody complaining that installing cargo-pio is failing on him.

[N3xed]

Except that our linker proxy is a cargo subcommand. This means two things:

• It is the most natural thing to install by a Rust developer
• It is very easy to install and update. What can be simpler than cargo install cargo-pio? For both install and update? No $PATH changes, no sudo - nothing. I've yet to see somebody complaining that installing cargo-pio is failing on him.

That is also true.

[ivmarkov]

@N3xed Sorry for the late notice: there is a meeting this Tuesday of rust-on-esp contributors and some of the Espressif folks. Given that having a cargo-first build story with "native" ESP-IDF is important, you might want to participate?

If so, can you contact me privately on the esp-rs chat. I need an email address of yours to forward you the invite.

In any case, I'll update the agenda for the meeting with the progress done here by you.

[N3xed]

@ivmarkov
Here's my proposal on how to proceed.

(1)
The cargo-pio library is currently pretty specific to platformio, but I like your idea of creating an embedded-build library.
So we would need to refactor the current cargo-pio code so that it can be used for different build environments. After that is done, move the bulk of this logic into the embedded-build library (and reuse anything that's there already).

(2)
Rename cargo-pio-link into cargo-linkproxy like you also already suggested, and add another argument for specifying the working directory. To parse the arguments that cargo potentially gives us in a response file we can just port the seperate_arguments command from cmake (ParseWindowsCommandLine for windows, there's this kwsysSystem__ParseUnixCommand for unix but not sure how easy it is to port).

(3)
I'd also like to create a tool that allows build scripts of dependencies to register commands for the end user. This allows similar behavior to the current cargo-pio (ex. cargo pio espidf menuconfig). This would very much simplify using the cmake version. This can then be used for flashing (ex. cargo bcmd espidf flash) or for creating a filesystem binary (maybe cargo bcmd generate-spiffs <path> <out-file>) and so on. This tool could live in the embedded-build or a separate repository.

I will start with (1) and (2) if you're OK with this plan. I'd also like to know what you think about (3) or alternatives.

[ivmarkov]

I definitely agree with (1) and (2). (3) seems reasonable too - we could use it for flashing, filesystem creation, and why not for monitoring (idf.py ... monitor ...) too? (The ESP-IDF monitor should be capable of decoding addresses into elf symbols. Not a rocket science honestly, can be done with pure Rust too, but why not having the ESP-IDF option too anyway.)

[georgik]

@ivmarkov @N3xed I've requested help with too long path on Windows and gcc flags from our team. They'll look into this issue.

[antmak]

Hi folks! @ivmarkov @N3xed

how is Espressif compiling their GCC-based toolchains for Windows? With --with-gnu-ld or not?

Not. Let me build a toolchain with that option (you need win64 for esp32, right?). Would it help?

[N3xed]

@antmak

Hi folks! @ivmarkov @N3xed

how is Espressif compiling their GCC-based toolchains for Windows? With --with-gnu-ld or not?

Not. Let me build a toolchain with that option (you need win64 for esp32, right?). Would it help?

As far as I can see, without this, response files don't really do anything to alleviate the command-line length limitation on windows. As I've described in a comment above:

So the above goes to say - can't we explore other paths to solve the "too long list of arguments" problem? For example, if GCC (&LD) do support passing the arguments to them in a file, we can build a temp file, shuffle all cmake-generated link arguments there and then just pass the temp file to the linker?

That doesn't work either because even if I pass all the arguments for the esp-idf in a response file, gcc then just calls collect2 with all argument files expanded. And on windows there is a strict limit of 32767 characters for the arguments (a limit of CreateProcess), which is overshot by a mile (50502). The reason this doesn't happen with only cmake is that, like already mentioned, it uses relative paths. Converting all paths to absolute ones inflates the 13448 characters command line to 50502 minus the arguments rust adds (which is about 100 (9238 chars)).

So this option should really always be used on windows (assuming ld is a gnu linker). But maybe there is also some other option which tells gcc whether the linker supports response files.

See also this:

I've also looked if somehow gcc itself could use response files in its invocations and this is indeed possible. But it seems that gcc has to be configured with --with-gnu-ld so that this works (more info here, here and here) but other than that maybe someone more knowledgable about gcc could shed some light on this.

[antmak]

@N3xed thanks! Do you have some fast test to check the using of response files, and to check that it helps us? If not, I'll try to check on my own soon.

--with-gnu-ld build:
https://dl.espressif.com/dl/toolchains/preview/withgnuld/xtensa-esp32-elf-gcc8_4_0-esp-2021r1-7-gec835ae-win64.zip
https://dl.espressif.com/dl/toolchains/preview/withgnuld/xtensa-esp32-elf-gcc8_4_0-esp-2021r1-7-gec835ae-linux-amd64.tar.gz

[N3xed]

@antmak Thank you for the builds. I don't have a quick check currently, but will test if this solves the problem with the command-line length as soon as I make some progress with this PR.

[ivmarkov]

Okay, I've updated the build script alongside my changes of embuild.

The command-line length is pretty close to the limit (31190 chars):

• All cfg values from kconfig amount to about 8409 chars
• the linker args from cmake are about 19414 chars
• and the args purely from rust about 3367.

Now the linker invocation isn't the problem anymore but the rustc invocation from cargo.
We should probably switch to passing the linker args in a file and/or setting only select configuration options from the kconfig file.

Indeed. Let's have the first iteration of the native build working, and then we can take one of these paths.

By the way, for me the native build fails (after all the issues I've mentioned so far are workarounded) with the following error:

  = note: Running ldproxy
          Error: Linker /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/riscv32-esp-elf-gcc failed: exit status: 1
          STDERR OUTPUT:
          /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/bin/ld: /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/lib/rv32im/ilp32/libc.a(lib_a-svfprintf.o): in function `_svfprintf_r':
          /builds/idf/crosstool-NG/.build/HOST-x86_64-apple-darwin12/riscv32-esp-elf/src/newlib/newlib/libc/stdio/vfprintf.c:1217: undefined reference to `__trunctfdf2'
          /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/bin/ld: /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/lib/rv32im/ilp32/libc.a(lib_a-vfprintf.o): in function `_vfprintf_r':
          /builds/idf/crosstool-NG/.build/HOST-x86_64-apple-darwin12/riscv32-esp-elf/src/newlib/newlib/libc/stdio/vfprintf.c:1192: undefined reference to `__trunctfdf2'
          collect2: error: ld returned 1 exit status

[N3xed]

By the way, for me the native build fails (after all the issues I've mentioned so far are workarounded) with the following error:

  = note: Running ldproxy
          Error: Linker /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/riscv32-esp-elf-gcc failed: exit status: 1
          STDERR OUTPUT:
          /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/bin/ld: /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/lib/rv32im/ilp32/libc.a(lib_a-svfprintf.o): in function `_svfprintf_r':
          /builds/idf/crosstool-NG/.build/HOST-x86_64-apple-darwin12/riscv32-esp-elf/src/newlib/newlib/libc/stdio/vfprintf.c:1217: undefined reference to `__trunctfdf2'
          /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/bin/ld: /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/lib/rv32im/ilp32/libc.a(lib_a-vfprintf.o): in function `_vfprintf_r':
          /builds/idf/crosstool-NG/.build/HOST-x86_64-apple-darwin12/riscv32-esp-elf/src/newlib/newlib/libc/stdio/vfprintf.c:1192: undefined reference to `__trunctfdf2'
          collect2: error: ld returned 1 exit status

That's weird. I can't test it as windows spits out file name or extension is too long again, but I'm guessing it because of a missing -lgcc after -lc in the linker args.

Note: I've tested only esp32 up till now as I don't have the other chips on hand.

[ivmarkov]

By the way, for me the native build fails (after all the issues I've mentioned so far are workarounded) with the following error:

  = note: Running ldproxy
          Error: Linker /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/riscv32-esp-elf-gcc failed: exit status: 1
          STDERR OUTPUT:
          /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/bin/ld: /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/lib/rv32im/ilp32/libc.a(lib_a-svfprintf.o): in function `_svfprintf_r':
          /builds/idf/crosstool-NG/.build/HOST-x86_64-apple-darwin12/riscv32-esp-elf/src/newlib/newlib/libc/stdio/vfprintf.c:1217: undefined reference to `__trunctfdf2'
          /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/bin/ld: /Users/imarkov/projects/esp32/rust-esp32-std-hello/.sdk/tools/riscv32-esp-elf/1.24.0.123_64eb9ff-8.4.0/riscv32-esp-elf/bin/../lib/gcc/riscv32-esp-elf/8.4.0/../../../../riscv32-esp-elf/lib/rv32im/ilp32/libc.a(lib_a-vfprintf.o): in function `_vfprintf_r':
          /builds/idf/crosstool-NG/.build/HOST-x86_64-apple-darwin12/riscv32-esp-elf/src/newlib/newlib/libc/stdio/vfprintf.c:1192: undefined reference to `__trunctfdf2'
          collect2: error: ld returned 1 exit status

That's weird. I can't test it as windows spits out file name or extension is too long again, but I'm guessing it because of a missing -lgcc after -lc in the linker args.

Note: I've tested only esp32 up till now as I don't have the other chips on hand.

Are you preserving the order of all linker args that you get from cmake when sending these down to ldproxy? This is really really crucial. If you don't, then we'll be faced with issues like this. The thing is, the linking of ESP-IDF is a bit weird and very sensitive to correct linking order, as they override some symbols from -lc. That's why (a) I had to remove the -lc and -lm that are placed by the Rust libc crate itself - in ldproxy - as it is really breaking the link order; fortunately this won't be needed with the new libc used by Rust 1.56 for ESP-IDF and (b) I'm super careful to preserve the linkage order.

[ivmarkov]

I'll try if we have the same issue with the ESP32.

[N3xed]

Are you preserving the order of all linker args that you get from cmake when sending these down to ldproxy?

Well yeah that is what it's doing, I'm getting these arguments from cmake. And my guess is that they are missing a final -lgcc to resolve link-dependencies in -lc which comes before it.

[N3xed]

Okay, so my guess was partially correct. But it actually happens because rustc unconditionally passes -nodefaultlibs to the linker.

There is a workaround:
Add rustflags = ["-C", "default-linker-libraries"] to the [target.riscv32imc-esp-espidf] section.

Not sure what better way there is to prevent that.

Maybe we could emit something for ldproxy which removes the -nodefaultlibs arg only if it is not specified in the LinkArgs we get from cmake.

[ivmarkov]

Okay, so my guess was partially correct. But it actually happens because rustc unconditionally passes -nodefaultlibs to the linker.

There is a workaround:
Add rustflags = ["-C", "default-linker-libraries"] to the [target.riscv32imc-esp-espidf] section.

Not sure what better way there is to prevent that.

Maybe we could emit something for ldproxy which removes the -nodefaultlibs arg only if it is not specified in the LinkArgs we get from cmake.

Argh, we have been looking at the same problem. See esp-rs/embuild#16

[N3xed]

Argh, we have been looking at the same problem. See ivmarkov/embuild#16

Not to worry, I didn't spend much time on this; you did go much more in-depth than me, nice work.

[N3xed]

I actually don't buy this:

I don't mind it so much for platformio, because if it needs platformio I can guess that the build script needs to install platformio (or just the needed toolchains) for it to work

If I paraphrase the above for the native build, it would sound just as convincing:
"I don't mind it so much for the Espressif idf.py build framework, because if it needs idf.py and the ESP-IDF SDK, I can guess that the build script needs to install the Espressif idf.py build framework and the ESP-IDF SDK for it to work.

That's not really true, don't you have to install idf.py and the toolchains used for the esp yourself?
I see where you're coming from though.

That is to say, whatever we decide to be the default, it should be for both build systems. To summarize, perhaps 4 options:

• Option 1 - install and use the default global locations (i.e. ~/.platformio and ~/.espressif)
  • Pros:
    • Does not pollute the binary crate with stuff that should be .gitignore-d
    • Downloaded toolchains etc. can be re-used across builds of different binary crates
    • Already downloaded toolchains etc. will just be used
  • Cons:
    • Patching ESP-IDF by the build itself is not so ideal as this is kinda build-specific
    • Messes up with the global user configuration of either PlatformIO or Espressif idf.py
• Option 2 - install and use an embuild-specific global location (i.e. ~/.embuild/platformio and ~/.embuild/espressif)
  • Pros:
    • Does not pollute the binary crate with stuff that should be .gitignore-d
    • Downloaded toolchains etc. can be re-used across builds of different binary crates
    • Already downloaded toolchains etc. will just be used
    • Does not mess up with the user configuration
  • Cons:
    • Patching ESP-IDF by the build itself is not so ideal as this is kinda build-specific
    • Creates something outside the binary crate directory
• Option 3 - install and use an embuild-specific location within the root of the binary crate (i.e. .embuild/platformio and .embuild/espressif)
  • Pros:
    • Does not mess up with the user configuration
    • No global state
    • Patching ESP-IDF OK as it remains build specific
  • Cons:
    • Pollutes the binary crate with stuff that should be .gitignore-d
    • Still does not follow the cargo policy that everything should be in the $OUT_DIR
    • Downloaded toolchains etc. cannot be re-used across builds of different binary crates
• Option 4 - install and use an embuild-specific location within the OUT_DIR of the binary crate (i.e. $OUT_DIR/embuild/platformio and $OUT_DIR/embuild/espressif)
  • Pros:
    • Does not mess up with the user configuration
    • No global state
    • Patching ESP-IDF OK as it remains build specific
    • Does not pollute the binary crate with stuff that should be .gitignore-d
    • Does not follow the cargo policy that everything should be int he $OUT_DIR
    • cargo clean means everything is removed, including ESP-IDf patches. Really clean state
  • Cons:
    • cargo clean means everything will be re-downloaded again
    • Downloaded toolchains etc. cannot be re-used across builds of different binary crates

Preferences for a default?
Mines are:

• Option 2
• Option 3 (if global state is something you guys feel uneasy)

In any case, having a flag/switch/env var/whatever so that the build framework is operating in Option 1 mode is a prerequisite IMO.

I don't like option 4 as it implies that every time esp-idf-sys is rebuilt all tools and the esp-idf repo have to be redownloaded.

And like you point out in the cons of option 1, we should probably not interfere with any installation the user did themselves, though in the case of platformio I don't think that is as much of a problem because really only the patches and scons scripts could interfere which I also think are generally useful.

If both build variants (i.e. pio and native) have to be consistent with the way they are installing the tools this would imply options 2 and 3.

But then it doesn't really make sense that this build script installs tools under a .embuild folder instead, the logic for installing these tools should be moved to the embuild library itself. But I'm not sure that we'd want install logic inside embuild specific to the esp-idf.

Maybe the way forward would be to roll our own install logic (currently this leverages idf_tools.py) and install the tools this way. That wouldn't be specific to the esp-idf anymore and would be a good addition to embuild.

Leaving this issue aside for now though, I propose that we leave the installed tools under a folder in the workspace dir for now (there is already an environment variable to change the default directory). But we should definitely pick a better name than .sdk. I propose .espressif.

Other than that I think we should merge this like it is and mention in the README/docs that this is an experimental feature for now (and of course setting the default to pio, since it is experimental).

[ivmarkov]

If both build variants (i.e. pio and native) have to be consistent with the way they are installing the tools this would imply options 2 and 3.

But then it doesn't really make sense that this build script installs tools under a .embuild folder instead, the logic for installing these tools should be moved to the embuild library itself. But I'm not sure that we'd want install logic inside embuild specific to the esp-idf.

Actually, why not? As I mentioned in another comment, ESP-IDF (and Espressif) are big enough to deserve their own little "espidf" module in embuild? I don't consider this necessarily "dirty" given that it will nicely built upon the other modules of embuild.

But this can be a next step.

Maybe the way forward would be to roll our own install logic (currently this leverages idf_tools.py) and install the tools this way. That wouldn't be specific to the esp-idf anymore and would be a good addition to embuild.

I think that would mean extra maintenance burden for us. Why bother? And then - as per above, IMO no big deal to have something ESP-IDF specific in embuild, in its own module.

Leaving this issue aside for now though, I propose that we leave the installed tools under a folder in the workspace dir for now (there is already an environment variable to change the default directory). But we should definitely pick a better name than .sdk. I propose .espressif.

OK I can somehow live with that. However you really really don't like .embuild/espressif? With this name I can shuffle my platformio in .embuild/platformio as well...
So folks will just know of .embuild and won't be bothered which one is the default (we will silently switch from pio to ... how shall we name this? idf? in the next weeks but all embuild artefacts wiould still be in the .embuild folder.).

Other than that I think we should merge this like it is and mention in the README/docs that this is an experimental feature for now (and of course setting the default to pio, since it is experimental).

Agreed.

[N3xed]

I think that would mean extra maintenance burden for us. Why bother? And then - as per above, IMO no big deal to have something ESP-IDF specific in embuild, in its own module.

I mean it is a maintenance burden for us either way if we use idf_tools.py or our own solution. We can still leverage the tools.json in the esp-idf which specifies which tools are where and what version, but without depending on a python script which also needs some workarounds (like the msystem one for windows).

Also, we'd need to add some logic in this regard anyway if we'd want to remove the requirement that the user has to install cmake themselves, which this currently requires as most cmake versions installed by idf_tools.py are too old even one version back (v4.2) and even the current version's (v4.3) cmake doesn't support the toolchains cmake-file-api object.

And then - as per above, IMO no big deal to have something ESP-IDF specific in embuild, in its own module.

Yeah, but having some code that depends on some python script inside the esp-idf which wasn't even documented before v4.2?

With this name I can shuffle my platformio in .embuild/platformio as well...

Do you mean you want to change the default of the platformio variant to <workspace dir>/.embuild/platformio? I think we should let the default of .platformio be or maybe even change it to ~/.embuild/platformio not to inside the workspace.

I picked .espressif as installing the tools doesn't really come from embuild but this build script so having them under .embuild/espressif would be misleading, wouldn't it? And we can change this location later as this is an experimental feature (i.e. unstable).

Or do we want to define now that this will use .embuild/espressif once we add this logic to embuild?
@ivmarkov

[N3xed]

Also, we should probably release a new embuild version before this gets merged, so that I can use the new version in this PR.

[ivmarkov]

Also, we should probably release a new embuild version before this gets merged, so that I can use the new version in this PR.

Done. Also created esp-rs/embuild#17