gh-Mbed-TLS/mbedtls-framework

Watch

Fork 0

all.sh inconsistencies about generated files #188

New issue

Open

opened 2025-07-18 07:53:32 -05:00 by gilles-peskine-arm · 5 comments

gilles-peskine-arm commented

2025-07-18 07:53:32 -05:00

(Migrated from github.com)

all.sh starts every component with the configuration-independent platform-independent generated files present in Mbed TLS (both 3.6 and 4.x), but not in TF-PSA-Crypto. This means that the same component code in the two repositories can have different effects. For example, in TF-PSA-Crypto, check_test_cases misses the generated test cases, and check_names may miss some things (resulting in false positives or false negatives) if we forget to generate enough files.

Cleanup before components does not remove generated files. Thus, in TF-PSa-Crypto, when we run a component locally, we may run it in a different state from the CI if we happen to have some generated test cases present. If in the future we group multiple components in the same node instantiation on the CI, components may run in a variable state (with or without generated files present).

Historically, when we first introduced generated files, not all builds could cope with them (at first only Make was supported but not CMake), so all.sh prepared the generated files. We've moved on from that, and now every build should be able to cope with a fresh checkout where the generated files are missing. (Builds should also cope with generated files being present, but that's usually not a problem.) However, some of the checks that don't do builds assume that the generated files are present.

I'm not sure what the right thing is, but it should be consistent across repositories and invocation scenarios.

`all.sh` [starts every component with the configuration-independent platform-independent generated files present](https://github.com/Mbed-TLS/mbedtls-framework/blob/febf6887dafb179ee16ac5bdc4b6427fe725493d/scripts/all-core.sh#L1024) in Mbed TLS (both 3.6 and 4.x), but not in TF-PSA-Crypto. This means that the same component code in the two repositories can have different effects. For example, in TF-PSA-Crypto, [`check_test_cases`](https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/fe9f986980b983fbff2a71e4d46885652616efca/tests/scripts/components-basic-checks.sh#L62) misses the generated test cases, and [`check_names`](https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/fe9f986980b983fbff2a71e4d46885652616efca/tests/scripts/components-basic-checks.sh#L143) may miss some things (resulting in false positives or false negatives) if we forget to generate enough files. [Cleanup before components](https://github.com/Mbed-TLS/mbedtls-framework/blob/febf6887dafb179ee16ac5bdc4b6427fe725493d/scripts/all-core.sh#L363) does not remove generated files. Thus, in TF-PSa-Crypto, when we run a component locally, we may run it in a different state from the CI if we happen to have some generated test cases present. If in the future we group multiple components in the same node instantiation on the CI, components may run in a variable state (with or without generated files present). Historically, when we first introduced generated files, not all builds could cope with them (at first only Make was supported but not CMake), so `all.sh` prepared the generated files. We've moved on from that, and now every build should be able to cope with a fresh checkout where the generated files are missing. (Builds should also cope with generated files being present, but that's usually not a problem.) However, some of the checks that don't do builds assume that the generated files are present. I'm not sure what the right thing is, but it should be consistent across repositories and invocation scenarios.

ronald-cron-arm commented

2025-07-18 08:56:41 -05:00

(Migrated from github.com)

I believe the difference between Mbed TLS and TF-PSA-Crypto is due to the fact that, when we adapted all.sh for TF-PSA-Crypto, there was no equivalent of make generated_files.

Now that we have make_generated_files.py, I would suggest aligning TF-PSA-Crypto with Mbed TLS in the short term.

As for the longer term, I would lean toward reducing the component preparation and clean-up code: less all.sh code to maintain and a workflow that more closely matches local and manual testing.

I believe the difference between Mbed TLS and TF-PSA-Crypto is due to the fact that, when we adapted all.sh for TF-PSA-Crypto, there was no equivalent of `make generated_files`. Now that we have `make_generated_files.py`, I would suggest aligning TF-PSA-Crypto with Mbed TLS in the short term. As for the longer term, I would lean toward reducing the component preparation and clean-up code: less all.sh code to maintain and a workflow that more closely matches local and manual testing.

gilles-peskine-arm commented

2025-07-18 09:10:10 -05:00

(Migrated from github.com)

How about, for the time being, aligning both projects to: start every check component with generated files and others without? It's a bit too much magic to my taste, but would solve the minor bugs and friction (took me a while to understand why my check was failing) in crypto, while still checking that cmake builds of crypto work from a blank start.

How about, for the time being, aligning both projects to: start every `check` component with generated files and others without? It's a bit too much magic to my taste, but would solve the minor bugs and friction (took me a while to understand why my check was failing) in crypto, while still checking that cmake builds of crypto work from a blank start.

ronald-cron-arm commented

2025-07-18 09:21:18 -05:00

(Migrated from github.com)

Yes that sounds a good compromise to me. My only concern is the potential issues when we stop generating files for test and build components in mbedtls.

gilles-peskine-arm commented

2025-07-18 09:24:05 -05:00

(Migrated from github.com)

I'll try it out, and consider always-generate as a plan B if either us or the CI turn out not to like it.

👍 1

gilles-peskine-arm commented

2025-07-24 03:03:05 -05:00

(Migrated from github.com)

After trying this out, this just seems to be too much work to clean up.

Build components that cross-compile need the generated files to be built first, because generating files involves compiling and running a C program, and we would need additional work. (Maybe setting HOST_CC=cc in the environment would work, I haven't tried it.)
The libtestdriver1 build requires generated files to be present, otherwise tests/libtestdriver1/library/Makefile tries to invoke ../framework/exported.make which we aren't creating. This absolutely could be resolved by doing rewriting or copying more stuff, but I haven't gone further down this rabbit hole to see how much more would need to be changed.
make && make test does not generate tests/opt-test/cases/*.sh, so we miss test coverage.
In several components, we explicitly run cmake -DGEN_FILES=OFF. Sometimes it seems to be by design (to simulate a release) and sometimes I'm not sure. Where it's by design we need to run make_generated_files.py first.
Maybe more that I haven't found yet among the other issues.

After trying this out, this just seems to be too much work to clean up. * Build components that cross-compile need the generated files to be built first, because generating files involves compiling and running a C program, and we would need additional work. (Maybe setting `HOST_CC=cc` in the environment would work, I haven't tried it.) * The libtestdriver1 build requires generated files to be present, otherwise `tests/libtestdriver1/library/Makefile` tries to invoke `../framework/exported.make` which we aren't creating. This absolutely could be resolved by doing rewriting or copying more stuff, but I haven't gone further down this rabbit hole to see how much more would need to be changed. * `make && make test` does not generate `tests/opt-test/cases/*.sh`, so we miss test coverage. * In several components, we explicitly run `cmake -DGEN_FILES=OFF`. Sometimes it seems to be by design (to simulate a release) and sometimes I'm not sure. Where it's by design we need to run `make_generated_files.py` first. * Maybe more that I haven't found yet among the other issues.

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#188

No description provided.

Rows
Columns