Modify path handling in some build scripts to be closer to mbedtls #376
Conversation
This makes it easier to merge changes related to adding or removing 3rdparty items. No semantic change.
Other third-party components can now be added by just adding lines to the definitions of @thirdparty_header_dirs and @thirdparty_source_dirs. No semantic change. The output does not change at all.
There's no need to keep the directory lists separated until the last minute. No semantic change. The generated files change slightly because there was one directory list where slashes were not changed to backslashes like in the other five. This does not affect their semantics.
generate_visualc_files.pl has a list of directories that it pulls headers from, so it knows what directories to put on the include path. Make it inject the include path into the output files, rather than hard-coding the include paths in template files. A similar change (but with different code) was made in Mbed TLS in commit b78cf2b "Adjust visual studio file generation to always use the crypto submodule". No semantic change: this commit does not change the generated files.
Tweak the code to be slightly simpler and closer to mbedtls. This changes non-significant whitespace in the generated files.
This makes reconciliation with other branches that don't have it (mbedtls, backports) easier.
gilles-peskine-arm
added
enhancement
mpg
left a comment
mpg
left a comment
There was a problem hiding this comment.
I checked that the PR is not harmful and doesn't make any semantic change; the resulting code is more generic and cleaner too.
No code change. This commit just moves two functions to make the order of component definitions match the one in mbedtls.
mpg
left a comment
mpg
left a comment
There was a problem hiding this comment.
LGTM
By the way over the long run perhaps we should have a rule about ordering of the components in all.sh (could be as simple as lexical order, which would also naturally group build/check/test together). But that's obviously out of scope for this PR.
simonbutcher
left a comment
simonbutcher
left a comment
There was a problem hiding this comment.
I'm fine with the PR. There's one minor pre-existing issue, but I don't think it's worth rework, unless there's some being done anyway, so approving.
|
|
||
| exit( main() ); | ||
|
|
||
| sub check_dirs { |
There was a problem hiding this comment.
Minor - Pre-existing really, but the error message of "Must but run from mbedTLS root or scripts dir" doesn't quite match the failure condition, which is if a required directory is not found, the script fails.
I suggest the error message be changed to:
"Necessary source directory not found."
"This script must be but run from the mbedtls root directory or its scripts directory."
simonbutcher
added
needs: ci
|
I checked the CI results and the only failures are in mbed-os tests, which are known and expected to fail at this point, so can be ignored for this PR. Therefore I'm removing "needs: ci" and labeling "ready for merge". |
mpg
added
ready for merge
3 participants
Mostly this aligns the Visual Studio templates with mbedtls and makes the handling of directories in
generate_visual_studio.plmore generic.There is no completeness goal in this pull request, only a goal of non-harmfulness (no semantic change).