Redesign of knowledge about generated files #215

New issue

Open

opened 2025-09-17 05:27:05 -05:00 by gilles-peskine-arm · 0 comments

gilles-peskine-arm commented

2025-09-17 05:27:05 -05:00

(Migrated from github.com)

There are several problems with make_generated_files.py as it currently stands. Some of these problems also affect the legacy make generated_files in mbedtls. The goal of this issue is to come up with a plan to fix these problems. At this point, I don't know if there should be a series of incremental improvements, or a complete redesign. The problems with make generated_files may go away naturally when we remove that.

A central theme is that everything works in a static universe, but changes such as adding a new generation scripts are very difficult and time-consuming. The first time this happened after make_generated_files.py was in place is with scripts/generate_config_checks.py, which required a lot of steps. For the new crypto script:

https://github.com/Mbed-TLS/mbedtls-framework/pull/196 — declare the new script, but make it optional (a new concept)
https://github.com/Mbed-TLS/mbedtls-framework/pull/211 — lack of consistency of handling of output directories
https://github.com/Mbed-TLS/mbedtls-framework/pull/212 — make the files from the script optional, which is subtly different from the script itself being optional
https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/404 — generate but don't consume files from the new script
https://github.com/Mbed-TLS/mbedtls/pull/10340 — teach mbedtls to call the new script
https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/269 — now crypto can consume the new script

Some identified design problems:

make generated_files in Mbed TLS has to know all the generation scripts in TF-PSA-Crypto. So when adding a new script in crypto, you need to first create it but not use its result, then teach mbedtls about the new script, and then you can use it in crypto.
make_generated_files.py in the framework knows all the generation scripts. So to create a new script, you first have to have it but not use it in the consuming branch, then register it in the framework, and then you can start using it. And even so, I'm not 100% sure that this works for crypto depending on when mbedtls updates its submodules. Another approach that does work is registering the new script in make_generated_files.py but conditionally on the script's existence, as done in github.com/Mbed-TLS/mbedtls-framework@5b6c405e6f.
There is a lot of diversity and undocumented expectations when it comes to generating files in a non-default location (which is done in CMake out-of-tree builds, where we don't touch the source tree).
- Each generation script must have a --list option that prints the output file locations relative to the project root when called from the project root. An absolute path doesn't work. This is reasonable but undocumented, and make_generated_files.py --check --root DIR will look for files in the wrong place if --list does something different.
- In the build_tree module, guess_project_root() returns an absolute path. This is the natural function to use, but it doesn't work for the needs of generate_xxx.py --list, since make_generated_files.py insists on a relative path.
- All generation scripts support a way to put the output in a non-default location, but they do it in different ways (output directory or file, positional argument or option). make_generated_files.py doesn't enforce that a way exist, but will look for files in the wrong place if this way is missing.
make_generated_files.py --check temporarily overwrites the existing files. It's weird that a “check” facility disrupts what it checks. If we felt that we had a reliable way to control the file locations, we could instead do the generation in an alternative location, and compare the results.
make_generated_files.py --check stops as soon as something is wrong (it only knows to exit with an exception). It should record common errors (missing file, failure of a generation script, unexpected content) but keep going, and finally exit with 1 if there was an error.

This list may be incomplete. You can help by expanding it.

There are several problems with `make_generated_files.py` as it currently stands. Some of these problems also affect the legacy `make generated_files` in mbedtls. The goal of this issue is to come up with a plan to fix these problems. At this point, I don't know if there should be a series of incremental improvements, or a complete redesign. The problems with `make generated_files` may go away naturally when we remove that. A central theme is that everything works in a static universe, but changes such as adding a new generation scripts are very difficult and time-consuming. The first time this happened after `make_generated_files.py` was in place is with `scripts/generate_config_checks.py`, which required a lot of steps. For the new crypto script: 1. https://github.com/Mbed-TLS/mbedtls-framework/pull/196 — declare the new script, but make it optional (a new concept) 2. https://github.com/Mbed-TLS/mbedtls-framework/pull/211 — lack of consistency of handling of output directories 3. https://github.com/Mbed-TLS/mbedtls-framework/pull/212 — make the files from the script optional, which is subtly different from the script itself being optional 4. https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/404 — generate but don't consume files from the new script 5. https://github.com/Mbed-TLS/mbedtls/pull/10340 — teach mbedtls to call the new script 6. https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/269 — now crypto can consume the new script Some identified design problems: * `make generated_files` in Mbed TLS has to know all the generation scripts in TF-PSA-Crypto. So when adding a new script in crypto, you need to first create it but not use its result, then teach mbedtls about the new script, and then you can use it in crypto. * `make_generated_files.py` in the framework knows all the generation scripts. So to create a new script, you first have to have it but not use it in the consuming branch, then register it in the framework, and then you can start using it. And even so, I'm not 100% sure that this works for crypto depending on when mbedtls updates its submodules. Another approach that does work is registering the new script in `make_generated_files.py` but conditionally on the script's existence, as done in https://github.com/Mbed-TLS/mbedtls-framework/commit/5b6c405e6ff97bbeab11ee8f53a74d4abb82f3f7. * There is a lot of diversity and undocumented expectations when it comes to generating files in a non-default location (which is done in CMake out-of-tree builds, where we don't touch the source tree). * Each generation script must have a `--list` option that prints the output file locations _relative to the project root_ when called from the project root. An absolute path doesn't work. This is reasonable but undocumented, and `make_generated_files.py --check --root DIR` will look for files in the wrong place if `--list` does something different. * In the `build_tree` module, `guess_project_root()` returns an absolute path. This is the natural function to use, but it doesn't work for the needs of `generate_xxx.py --list`, since `make_generated_files.py` insists on a relative path. * All generation scripts support a way to put the output in a non-default location, but they do it in different ways (output directory or file, positional argument or option). `make_generated_files.py` doesn't enforce that a way exist, but will look for files in the wrong place if this way is missing. * `make_generated_files.py --check` temporarily overwrites the existing files. It's weird that a “check” facility disrupts what it checks. If we felt that we had a reliable way to control the file locations, we could instead do the generation in an alternative location, and compare the results. * `make_generated_files.py --check` stops as soon as something is wrong (it only knows to exit with an exception). It should record common errors (missing file, failure of a generation script, unexpected content) but keep going, and finally exit with 1 if there was an error. _This list may be incomplete. You can help by expanding it._

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/mbedtls-framework#215

No description provided.

Rows
Columns