[mbedcrypto] all.sh: make it possible to run a subset of the components #7
Conversation
Move almost all the code of this script into functions. There is no intended behavior change. The goal of this commit is to make subsequent improvements easier to follow. A very large number of lines have been reintended. To see what's going on, ignore whitespace differences (e.g. diff -w). I followed the following rules: * Minimize the amount of code that gets moved. * Don't change anything to what gets executed or displayed. * Almost all the code must end up in a function. * One function does one thing. For most of the code, that's from one "cleanup" to the next. * The test sequence functions (run_XXX) are independent. The change mostly amounts to putting chunks of code into a function and calling the functions in order. A few test runs are conditional; in those cases the conditional is around the function call.
Call cleanup from run_component instead of calling it from each individual component function. Clean up after each component rather than before. With the new structure it makes more sense for each component to leave the place clean. Run cleanup once at the beginning to start from a clean slate.
Add an option to list the available components. This is not useful yet, but a subsequent commit will add the ability to run specific components.
Allow the list to use wildcards, e.g. you can run the sanity checks with all.sh --except "test_*" "build_*"
In all.sh, always save config.h before running a component, instead of doing it manually in each component that requires it (except when we forget, which has happened). This would break a script that requires config.h.bak not to exist, but we don't have any of those.
…h-mbedcrypto Merge the work on all.sh that was done on mbedtls-2.14.0 with the changes from mbedtls-2.14.0 to the current tip of mbed-crypto/development. There is a merge conflict in test/scripts/all.sh, which is the only file that was modified in the all.sh work branch. I resolved it by taking the copy from the all.sh branch and applying the changes between mbedtls-2.14.0 and mbedtls-2.16.0. As evidenced by `git diff mbedtls-2.14.0 d668bae`, many of the commits in `git log mbedtls-2.14.0 d668bae` cancelled each other or were redundant with parallel commits that had also occured via another branch included in mbedtls-2.14.0, leaving the following differences: * Removal of one unimportant blank line. * The changes from db2b8db "psa: Add storage implementation for files", to turn off PSA storage when MBEDTLS_FS_IO is turned off, which I manually replayed.
gilles-peskine-arm
added
enhancement
|
Run of |
Only whitespace changes in this commit.
Use `cmake -D CMAKE_BUILD_TYPE=Asan` rather than manually setting `-fsanitize=address`. This lets cmake determine the necessary compiler and linker flags. With UNSAFE_BUILD on, force -Wno-error. This is necessary to build with MBEDTLS_TEST_NULL_ENTROPY.
Call `set disable-randomization off` only if it seems to be supported. The goal is to neither get an error about disable-randomization not being supported (e.g. on FreeBSD), nor get an error if it is supported but fails (e.g. on Ubuntu). Only fiddle with disable-randomization from all.sh, which cares because it reports the failure of ASLR disabling as an error. If a developer invokes the Gdb script manually, a warning about ASLR doesn't matter.
Don't bail out of all.sh if the OS isn't Linux. We only expect everything to pass on a recent Linux x86_64, but it's useful to call all.sh to run some components on any platform. In all.sh, always run both MemorySanitizer and Valgrind. Valgrind is slower than ASan and MSan but finds some things that they don't. Run MSan unconditionally, not just on Linux/x86_64. MSan is supported on some other OSes and CPUs these days. Use `all.sh --except test_memsan` if you want to omit MSan because it isn't supported on your platform. Use `all.sh --except test_memcheck` if you want to omit Valgrind because it's too slow. Make the test scripts more portable (tested on FreeBSD): don't insist on GNU sed, and recognize amd64 as well as x86_64 for `uname -m`. The `make` utility must still be GNU make.
MAKEFLAGS was set to -j if it was already set, instead of being set if not previously set as intended. So now all.sh will do parallel builds if invoked without MAKEFLAGS in the environment.
|
New run of |
gilles-peskine-arm
force-pushed
the
all_sh-mbedcrypto
branch
from
78d82b4 to
e878987
Compare
Extract the list of available components by looking for definitions of functions called component_xxx. The previous code explicitly listed all components in run_all_components, which opened the risk of forgetting to list a component there. Add a conditional execution facility: if a function support_xxx exists and returns false then component_xxx is not executed (except when the command line lists an explicit set of components to execute).
Build the list of components to run in $RUN_COMPONENTS as part of command line parsing. After parsing the command line, it no longer matters how this list was built.
Only look for armcc if component_build_armcc is to be executed, instead of requiring the option --no-armcc. You can still pass --no-armcc, but it's no longer required when listing components to run. With no list of components or an exclude list on the command line, --no-armcc is equivalent to having build_armcc in the exclude list.
Don't require openssl, mingw, etc. if we aren't going to run a component that uses them.
It's all about tool detection.
After backing up and restoring config.h, `git diff-files` may report it as potentially-changed because it isn't sure whether the index is up to date. Use `git diff` instead: it actually reads the file.
The deletion of "$OUT_OF_SOURCE_DIR" had mistakenly been lumped together with Yotta and then removed when Yotta support was removed. Bring it back.
Wildcard patterns now work with command line COMPONENT arguments without --except as well as with. You can now run e.g. `all.sh "check_*` to run all the sanity checks.
Valgrind is what it does. `memcheck` is how it's implemented.
|
Should we instead merge mbedtls/development after your similar change goes in to that branch? I'd prefer to avoid unnecessary merge conflicts when we merge mbedtls/development into mbed-crypto/development, and that way seems more likely to avoid conflicts than to merge this and that and then later merge in mbedtls/development. |
|
Yes, we can merge |
|
After #12, your PR to mbedtls/development should apply here as well, at least more easily, but would still need mbed-crypto-specific tweaking still for any all.sh tests we've added (or removed). At least the tooling updates would be single-sourced and we'd not have to resolve conflicts between two patch sets of tooling updates. |
|
#12 is too hairy to merge. So, it's not likely to be ready sooner than this PR. We'll deal with the conflicts and updating to the latest Mbed TLS crypto later, and enjoy a better all.sh today. |
|
I thought there wouldn't be any merge conflicts on |
|
Test failure is |
Patater
removed
the
needs: review
This is a side-port of Mbed-TLS/mbedtls#2325 plus Mbed-TLS/mbedtls#2332 to
mbed-crypto/development.Because this PR contains commits that are very tedious to replay and to review, I merged the previously-reviewed PR against mbedtls-2.14.0 rather than rebasing it on top of the current branch. The commits before the merge were previously reviewed in Mbed-TLS/mbedtls#2235, therefore this PR needs a review of:
mbed-crypto/developmentinto my working branch.run_all_componentswhere a slightly different list of components got deleted.