Log build steps in the outcome file #129
Conversation
Add an outcome line when all.sh calls make. Signed-off-by: Gilles Peskine <[email protected]>
Some versions on CMake (on some platforms?) run make (not $(MAKE)) under the hood on targets named cmTC_* (CMake TryCompile). Ignore those calls. Signed-off-by: Gilles Peskine <[email protected]>
This should make failures.csv on the CI be a complete list of failures (although it will still have incomplete information on what has failed, if what failed wasn't tracked individually). Signed-off-by: Gilles Peskine <[email protected]>
This comes up in `psa_collect_statuses.py` and possibly (I didn't finish the analysis) some CMake builds, which were reporting duplicate `make` steps. Signed-off-by: Gilles Peskine <[email protected]>
When emitting a line for the outcome file for `make`, indicate if the target is being built in a different directory or with a different makefile. We don't currently do this explicitly with an ambiguous target name, but it happens under the hood with older versions of CMake (observed with CMake 3.10.2, not with CMake 3.22.1). Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
added
needs-ci
Align the build in component_test_tfm_config_no_p256m with its corresponding non-driver build test_tfm_config_p256m_driver_accel_ec. There didn't seem to be a reason not to build programs, and Mbed-TLS/mbedtls-framework#129 would cause the discrepancy to be reported as a failure in outcome analysis. Signed-off-by: Gilles Peskine <[email protected]>
Align the build in component_test_tfm_config_no_p256m with its corresponding non-driver build test_tfm_config_p256m_driver_accel_ec. There didn't seem to be a reason not to build programs, and Mbed-TLS/mbedtls-framework#129 would cause the discrepancy to be reported as a failure in outcome analysis. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
added
needs-review
`platform;component_xxx;all.sh;whole;PASS;` could be misleading: it might look like “whole” means the whole of `all.sh`. Change this to `platform;component_xxx;all.sh;component_xxx;PASS;`. Signed-off-by: Gilles Peskine <[email protected]>
| @@ -75,5 +75,6 @@ else | |||
| rm "${TMPFILE}" | |||
|
|
|||
| # Propagate the exit status | |||
| exit $EXIT_STATUS | |||
| set_status () { return $1; } | |||
| set_status $EXIT_STATUS | |||
There was a problem hiding this comment.
Why the indirection?
If we want to preserve the status, could't we just return "EXIT_STATUS" ? What am I missing?
There was a problem hiding this comment.
make includes (“sources”) quiet.sh with the . command. Calling exit inside quiet.sh would exit the whole program. This was ok until now because make didn't want to do anything else in this case. But now make wants to run additional logging code. So we have to return from quiet.sh, and communicate the status.
Note that in sh, there is no language feature to return early from a sub-script sourced with the . command. In ksh, bash and zsh, you can do it with the return builtin, but that extension is not available in a basic sh shell such as dash.
| fi | ||
| case "$target" in | ||
| "clean"|"neat") continue;; # Boring | ||
| cmTC_*) continue;; # Weirdness from CMake |
There was a problem hiding this comment.
Those are temporary target files that will be cleaned up. So the logic is sound. It is not obscure but standard even though unfortunately undocumented behavior from cmake. ;(
| return;; | ||
| --assume-new|--assume-old|--directory|--file| \ | ||
| --directory=*|--file=*|-C?*|-f?*) # Modifier option with its argument |
There was a problem hiding this comment.
Why are both "--directory=somedir" and "--directory somedir" required?
For example would passing "--directory" and nothing else assert skip=1?
There was a problem hiding this comment.
With --directory=..., we want to remember the option and move on to the next argument which is another option or a non-option argument. With --directory, we want to skip the next argument, which is the argument of the --directory option.
make --directory is invalid. I don't know what the code here does in this case, but I don't think this wrapper needs to care about invalid usage of make.
Add some logging to the output file:
makeinvocation (including CMake-driven builds):"${MBEDTLS_TEST_PLATFORM};${MBEDTLS_TEST_CONFIGURATION};${TOOL};${target};${result};${cause}""${MBEDTLS_TEST_PLATFORM};${component_name};all.sh;${component_name};${result};"Extracted from Mbed-TLS/mbedtls#9286.
This is supposed to be transparent for a consuming branch, so we can merge it at our leisure.
Needs preceding PR: Mbed-TLS/mbedtls#9932 and Mbed-TLS/mbedtls#9933 to get the CI to pass.
Follow-up: Mbed-TLS/mbedtls#9934
PR checklist
Please add the numbers (or links) of the associated pull requests for consuming branches. You can omit branches where this pull request is not needed.