Hi Andy,
[...]
Is this approach maintainable? I don't remember if we have similar in Linux kernel, for example. (There are few candidates like compression algorithms that are usually being hosted elsewhere)
No answer?
subtrees is what was decided on OF_UPSRTEAM as well. If you have a better idea feel free to propose it, but for the sake of conformance we are better off doing the same thing on every external tree we pull in
How do they will (or already do) maintain this?
There's a script to do that in dts/update-dts-subtree.sh
At least it's a good to have a few words on the choice made in the cover letter, so we will have no questions on it.
Moreover, due to the Windows-style files from mbedtls git repo, we need to convert the CRLF endings to LF and do a commit manually:
$ git add --renormalize . $ git commit
...
lib/mbedtls/mbedtls_def_config.h | 4262 ++++++++++++++++++++++++++++++
This is ridiculously HUGE! This is unreviewable. Moreover, this is even hard to configure by the user! Can you rather make it modular and maybe create a separate documentation for the most important options (I do not believe one needs _all_ of them to be set / tuned)?
This is a file from MbedTLS and follows its own style.
And this is how MbedTLS is configured - with all features listed in a config file and commenting out the unused features with "//"). The modification here is just to control those existing options with Kconfigs.
And why should we blindly follow this nonsense?
It's easier to follow up future changes tbh. But I do agree the config file is huge. Perhaps splitting in 2 files is going to be easier mbedtls_def_config.h -> contains all the options that rarely need tuning, which I assume is the majority of the header mbedtls_usef_config.h -> contains the imporant options, crypto, checksum algorithms etc
Thoughts?
The problem is on who decides which are "rarely need".
Us. Sure it's going to take some time, but eventually we should be able to isolate options that rarely need a change
The feasible (to me) approach is to split per domain. Like you listed at the very end of your reply. We can also learn from managing MTA configurations, such as Exim4 where user may decide if they want a single file or split version.
I am not sure tbh. Let me backpaddle from my previous suggestion :). Looking at it closer config.h is copied from mbedTLS, commenting out features we don't want.
What we could do instead is either trim those comments out and only define the options we need for now. That would make the patch easily reviewable. If in the future we decide we need a more modular approach, we can retrofit it without breaking anything.
Alternatively, we can undef the options we don't want in our header, but I am not sure how often these change or how scalable this is going to end up being. I prefer option #1
Regards /Ilias
-- With Best Regards, Andy Shevchenko