-
Notifications
You must be signed in to change notification settings - Fork 36
Description
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:
- Introduce config_checks_generator.py #196 — declare the new script, but make it optional (a new concept)
- Fix generate_config_checks.py not actually generating the files #211 — lack of consistency of handling of output directories
- Fix generate_config_checks.py not satisfying make_generated_files --check --root #212 — make the files from the script optional, which is subtly different from the script itself being optional
- Introduce generated config checks in crypto TF-PSA-Crypto#404 — generate but don't consume files from the new script
- Introduce generated config checks in mbedtls mbedtls#10340 — teach mbedtls to call the new script
- Mechanism to error out on removed configuration options TF-PSA-Crypto#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 inmake_generated_files.py
but conditionally on the script's existence, as done in 5b6c405.- 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, andmake_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 ofgenerate_xxx.py --list
, sincemake_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.
- Each generation script must have a
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.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status