register esp-idf kconfigs as rustc cfg attributes #378
Conversation
- adds a generate_cfg script that autogenerate a list of rustc cfg, for a combination of different target/sdkconfig files. The list can be furhter tweaked to capture every config we need downstream in hal/svc. - remove the allow unknown_cfg lint - (drive by fix) changed unknown "alloc" feature to "alloc_handler" - (drive by fix) fixed esp_app_desc type from i8 to c_char in case of rep. builds.
|
The PR mostly provides code inside the build.rs script that will be called "out of band" if the coresponding features are called to gether a list of esp-idf kconfigs that we are using, and then saving them to a file. That generative step is automated via the generate_cfg.sh script. Than for actual runs issued by the user this list is used to register them as rustc cfg attributes. |
|
While this PR in its current form is working for esp-idf-sys, we still have the problem of propagation. E.g if the cfg attributes are defined in the build.rs of this crate, esp-idf-hal/svc do not see them. So for that part we would need to extend the embuild solution that we already use, for forwarding stuff from esp-idf-sys, but in this case we would forward the generated collected_cfg.txt, such that the build.rs scripts in hal/svc can rerun the same command. Alternatively we could just copy past the file into hal/svc - though i would not like it because it increases the maintenance burden by having to keep them manually in sync. |
|
I think before merging we should definitely try how this is going to work in the hal and svc crates, as well as in user code, where the problem still persists. |
| @@ -0,0 +1,72 @@ | |||
| CONFIG_MBEDTLS_CERTIFICATE_BUNDLE=y | |||
There was a problem hiding this comment.
Why is this specialized sdkconfig necessary?
| @@ -0,0 +1,26 @@ | |||
| # Examples often require a larger than the default stack size for the main thread and for the sysloop event task. | |||
|
|
||
| // Collect all cfg attributes from the build system and register them as known rustc cfg attributes. | ||
| // | ||
| // This is limited to what the current run's sdkconfig will enable. Thats why we run this out of band for |
There was a problem hiding this comment.
This sounds like a non-starter then?
As in, we need an sdkconfig that is listing ALL possible configs. Wow. But if we go this route, we can just as well list ALL possible configs from ALL ESP-IDF versions and for ALL MCUs.
And if we do the above, we need a single such file, and we don't have to run any build then, as well have everything already listed in this file?
So, are you 100% sure, that iterating the kconfig data in the build.rs really only gives you those sdkconfig settings which are "enabled" (qurious also what this means for config values which are not boolean)?
If yes, I'm starting to wonder if there is no better way to approach the whole problem. Like parsing the kconfig files themselves. There might even be some python tooling that might be doing this already, in ESP-IDF?
There was a problem hiding this comment.
This might be of interest: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/kconfig/configuration_structure.html
The "kconfig parsing" alternative I'm talking about would be basically iterating all components in the esp-idf source tree, as well as all user-provided components and parsing their kconfig files, so as to gather all possible options.
My point is some of this parsing code is likely to already exist as a python code in the esp-idf source tree. After all, idf.py menuconfig does that (and more).
|
@Vollbrecht Would you be addressing my comments in the PR request? |
|
As i hinted at earlier, most of the stuff will need to be implemented in embuild ( to use the same forward mechanism we use for the current cfg stuff). The goal should be clear: It's to allow the unknown cfg lint, and start a clean up of the whole cfg mess. Why does the solution look like it does? The unknown rust lint wants full knowledge over "every" used cfg attribute ( that we use in the responding crate). This list cannot be gained in a single iteration ever of what we currently get out the build system. That is because we essentially get back the already filled out config for a specific set of combinations( you can find it in the out/build/config dir as either sdkconfig.h or sdkconfig.json). A simple example where this run's into a problem is something like this Why not just parse the entry kconfig tree? I tried that before i played with the idea of doing all the work out of band. The esp-idf kconfig definitions is unfortunately a "special dialect" and not the same as the linux kernel one with some annoying implications, So a rust parser i modified to make it work didn't want to cooperate, even after some working on it ( To many edge cases in the dialect). My goal at that time was not to call more python code, but less ;D So why not continue with integrating the python parser? Well its a lot of work and first i wanted to make a dump solution first that works and maybe don't need all the work. Also what i hope is obvious, even if i do all the nice tree parsing its still not enough since for example esp-idf 5.1 have other stuff that 5.4 has. Newer versions not only add stuff but also delete stuff from there kconfig files. One note though: l we only run into problems in the rust site if we actually use some of this cfgs. If we not using a cfg than there is not a problem. That's why i think this dump approach where we just create a file that covers 95% is just the most pragmatic solution. Overall i think its worthwhile to improve our build system, its just takes work. One specific thing i may want to have a look in embuild is why initially we don't transform non-boolean cfgs just into there key,value pairs, but transform them into booleans. |
|
Thanks. I guess I'll ask my most important question from above again - inline below.
So are you saying: when we iterate over the Or are you saying, that the
(Let's concentrate on boolean kconfig items first, for simplicity.) I think: we absolutely, positively first have to understand what is currently returned - in terms of kconfig values - from the build output - before going to a solution. I personally do not understand/remember anymore what is returned to us. If you do, can you please explain what it is? |
|
As i said earlier we build the cfg's out of the file we get out of out/build/config. That happens here. At that point its already to late, and the above points all hold. We define only stuff that is a "true" boolean value in most cases ( That could be directly addressed there). But that does not solve the bigger problem, as this file (sdkconfig.json) will never include all combinations of cfg's that we may need, duo to " esp-id version specific" or "target specific's". To illustrate the problem i gave the above example with xtal. |
|
I hope this clarity's your questions? Please feel free to ask more persistent if that is not enough ;D |
But then the even bigger problem with your approach is that the (Note that you still don't answer what exactly is contained in So what is the point of running N build configurations and then grabbing kconfig values from the build output, given that we have to - anyway - first create a super large "all kconfigs explicitly mentioned there" monster of an The above ^^^ task is probably possible, but I don't see you doing that? |
Note that this is not my approach, fundamentally its how embuild/esp-idf-sys operated up to this point. I merely build ontop of it to fit in the current schema ( at least so far). If in the sdkconfig.json is a false value, than it gets rejected. E.g everything defined is implicitly true ( or was a non-boolean value converted such that it becomes a true boolean value)
I break it down for you, though i already mention that the sdkconfig.json and it's sibling will not contain all information that we need. ( Given the XTAL example). So what does it include? The build specific configuration, where for any particular option, one option ( of all possible options for that choice) was choosen and set. This is fine for simple boolean type settings, since the output is looking like this For this type of configs we have all info that we need and we could define them even in the negative case. But that is only around 60-70% off all configs in that output we got here. The rest of the cfg are "key - value" cfgs. And that would always only give us the information of that particular config set, and not of all the possibility we need. To stay with the xtal example we would get With this build run we would not get the information we need. E,g the existence of Then there is another complication, every cfg that start's with Again the more problematic cases are the "non-boolean" cases. For SOC that would for example be a value like
First we need something working, than we can "tweak it" to cover the cases we need. Fundamentally its not possible to cover all cases with a single run, because of the outlined problems above. Building at least once for every chip type and for every supported esp-version covers in practice most cases and was therefore just a pragmatic choice as a solution to the problem. In the meantime i may found a maybe more appropriate way of getting more information. While the dynamic nature of kconfig does not allow us to get a guarantee to capture every info we need, and the SOC flag problem would not be solved. The esp-idf parser produces a specific output json file that may help us archive better coverage. While you mention that some python code already does the parsing for us i could not find, so far, a "simple" output that we could leverage, but there exist actually one marked for "s a JSON-formatted version of the menus shown in menuconfig, for use in external IDE UIs." The file is called kconfig_menus.json and is produced with every run. That would mean we need to parse that json, with its recursive elements. Luckily the base element is relatively straight forward. After parsing it we would for the first time have access to the "choices" and could generate a appropriate cfg for every choice. Still overall doing so will not free us from doing multiple runs because of At this point we would have a "reasonable" dictionary that we can register as cfg attributes. If we see some cases are not coverd or "overcovered" we still can tweak. Furthermore once we have this basic, we could as a "next" step more reasonably see a cleanup of our cfg situation overall. Aspects of it is: "Find out the reason what the reason about only having boolified cfg's". "What would we break if instead of boolifing, we declare key-value cfg's as such". "Given that "SOC" cfg's are problematic should we instead only use equivalent cfg's that define choices?" etc. But that is for after we have our nice dictonary :D |
2 participants
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
cargo fmtcommand to ensure that all changed code is formatted correctly.cargo clippycommand to ensure that all changed code passes latest Clippy nightly lints.CHANGELOG.mdin the proper section.