Closed daggerrz closed 1 year ago
@ivmarkov, perhaps you have thoughts on this?
In general, I think you need the feedback of @N3xed , as he is our expert in the ESP IDF build system.
With that said, and w.r.t. re-using the Python component manager vs re-implementing our own, the latter would make sense only if we expect the component manager not to change every now and then. I have no idea whether that's the case? If you could share your opinion?
I have a few nits w.r.t. Rust code organization that I'll add inline - if you could address.
As I mentioned - a few nits really. Nothing major.
Thanks, I've added some commits and will squash if this moves forward.
With that said, and w.r.t. re-using the Python component manager vs re-implementing our own, the latter would make sense only if we expect the component manager not to change every now and then. I have no idea whether that's the case? If you could share your opinion?
I do not think they will change it as the API seems pretty set. However, there are a few features built into the Python version that would require some more effort. E.g:
Most of this should be pretty straightforward to implement (and I can take care of it). I personally find it to be easier to understand the build process when "all" of it is in the Cargo build, but I don't have a very strong opinion about it as long as it works.
Added checks for outdated managed dependencies using idf_component.yml
metadata in the component tarballs to make it easier to change version dependencies. This now works well for my "mDNS use case" and a component lock-file for reproducible builds is the only remaining must-have feature I can think of right now.
Great if @N3xed could take a look!
I'm sorry for my inactivity on this issue.
I'm all for implementing things in Rust, but I don't think it is worth it for the component manager, or any aspect that is already covered by the esp-idf for that matter. Though I completely understand your reason, as bridging the gap between the Rust and esp-idf cmake world is indeed tricky.
Most of this should be pretty straightforward to implement (and I can take care of it).
It may be straightforward to implement, but it would very much increase the complexity and amount of dependencies to embuild (which already has a good few) and indirectly to esp-idf-sys (as build dependencies). I think, having our own component manager doesn't come exactly cheap.
I personally find it to be easier to understand the build process when "all" of it is in the Cargo build, but I don't have a very strong opinion about it as long as it works.
Everything else of the C/C++ build uses the esp-idf build system though, and it's probably not a good idea to mix the two.
That's my opinion at least. I've been looking into integrating the esp-idf component manager, so that components, that contain an idf_component.yml
, just work (see https://github.com/esp-rs/esp-idf-sys/issues/193 for example). I've wanted to do that since a while ago but haven't had the time until now.
Not a problem - I've found this experiment to be very useful way to learn the build system, so no time wasted! Let me know if there's anything I can do to help with the CMake integration.
One way or the other, I believe we do need this easy syntax supported soon-ish, and (obviously) without any patches to esp-idf-sys
, as the original post here suggests, as requests for generating bindings (and linking against) components that now live in the ESP IDF Component Registry keep popping up. In other words, this syntax, as suggested by @daggerrz:
[[package.metadata.esp-idf-sys.extra_components]]
component_refs = [ { name = "espressif/mdns", version = "1.2" } ]
Reading the esp-idf-sys
post from above, it amounts to adding a few simple changes to the CMakeLists.txt
template, and then probably some glue in the esp-idf-sys
Rust code so that it can pull the components' names and versions and then stuff these in the ESP_IDF_COMPONENTS
environment variable?
@N3xed - are you saying you have some time to work on this these days? Otherwise, why don't we ask @daggerrz to work on that (if he is willing to, of course)? I don't want this topic to fall into oblivion again, given how important it is. And I'm swamped with other work ATM.
are you saying you have some time to work on this these days? Otherwise, why don't we ask @daggerrz to work on that (if he is willing to, of course)? I don't want this topic to fall into oblivion again, given how important it is. And I'm swamped with other work ATM.
Yes, I've been using https://github.com/esp-rs/esp-idf-sys/issues/193#issuecomment-1636900127 as a starting point, I'll make a PR today.
Reading the
esp-idf-sys
post from above, it amounts to adding a few simple changes to theCMakeLists.txt
template, and then probably some glue in theesp-idf-sys
Rust code so that it can pull the components' names and versions and then stuff these in theESP_IDF_COMPONENTS
environment variable?
The integration between Rust and CMake is a bit more tricky though, since we want the lock file in the Rust workspace, and a way to specify dependencies in the Rust side. As with specifying components in the Cargo.toml, I'll do something similar as @daggerrz suggested.
Since 5.0, some components like
mDNS
are distributed separately from IDF. I've been exploring approaches to how this can be resolved and am looking for feedback on the general approach before I do more work here. An alternative solution would be to simply use the Python component manager bundled with the IDF build system, but I found that to be more challenging to integrate into the build, than writing one in Rust.A proof of concept of the Cargo component manager client and download manager is included here in
embuild
and the integration into the Cargo driver is here: https://github.com/esp-rs/esp-idf-sys/pull/220.This works for building a project depending on mDNS if
ESP_IDF_COMP_MDNS_ENABLED
is set inesp-idf-sys
and the following is added to the project'sCargo.toml
:This is my first dive into the innards of the build system, so any guidance will be greatly appreciated. I have a lot more questions if the approach is considered sound.