-
Notifications
You must be signed in to change notification settings - Fork 96
Replace config.pl by config.py #321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is meant to be a drop-in replacement for config.pl which can additionally be used as a library in a Python script. So far this script supports the commands 'get', 'set' and 'realfull' but not the other built-in configurations.
Also fix 'realfull' to only affect the appropriate sections. Tested to produce the same results as config.pl on the default configuration. This commit deliberately contains a direct copy the lists of symbol names from config.pl.
These options haven't existed for a long time.
They're easier to maintain that way. The old lists were partly alphabetized, partly based on config.h order, and partly in the order in which symbols had been added to config.pl.
git grep -Fl /config.pl | xargs sed -i -e 's!/config\.pl!/config.py!g' Also: * Change one comment in include/mbedtls/check_config.h. * Change PERL to PYTHON in CMakeLists.txt.
Keep config.pl in Perl in case people are running "perl config.pl".
config.h is encoded in UTF-8.
The `set` command can act on any known symbol.
By default, this script looks for include/mbedtls/config.h relative to the current directory. This allows running config.py from outside the build tree. To support out-of-tree builds where config.h and config.py are in the source tree and the current directory is in the build tree, also try DIRECTORY_CONTAINING_SCRIPT/../include/mbedtls/config.h, and the equivalent with symbolic links traversed.
Also make that error message end with a newline.
Run config.py with various options and store the results in files. This script also supports the now-removed config.pl. This is a framework to run non-regression tests on config.py: run it with the old version, run it with the new version, and compare the output. This is deliberately not a functional test suite so that we don't need to maintain a set of known outputs. When something changes in config.py (or config.h), run the script before, run it after, and check manually whether any differences in the output are acceptable.
Perl is no longer needed. Python must be version 3. Version 2 is not suitable. The variable is PYTHONINTERP_FOUND, not PYTHON_FOUND.
The test suite generator has been a Python script for a long time, but tests/CMakeLists.txt still looked for Perl. The reference to PYTHON_INTERP only worked due to a call to find_package(PythonInterp) in the toplevel CMakeLists.txt, and cmake would not have printed the expected error message if python was not available.
Normally a valueless symbol remains valueless and a symbol with a value keeps having one. But just in case a symbol does get changed from valueless to having a value, make sure there's a space between the symbol and the value. And if a symbol gets changed from having a value to valueless, strip trailing whitespace. Add corresponding tests. Also fix the case of a valueless symbol added with the set method, which would have resulted in attempting to use None as a string. This only happened with the Python API, not with the command line API.
We currently test setting a symbol with a value even if it didn't originally had one and vice versa. So there's no need to have separate lists of symbols to test with. Just test everything we want to test with each symbol.
The tests were not covering get for a symbol with a value. No symbol has an uncommented value in the default config.h. (Actually there's _CRT_SECURE_NO_DEPRECATE, but that's a bit of a hack that this script is not expected to handle, so don't use it). Add tests of "get FOO" after "set FOO" and "set FOO value", so that we have coverage for "get FOO" when "FOO" has a value.
gilles-peskine-arm
approved these changes
Nov 13, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- This is a faithful backport of Replace config.pl by config.py Mbed-TLS/mbedtls#2765.
- It adapts the definitions of the preset configurations correctly.
Patater
approved these changes
Nov 14, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
gilles-peskine-arm
added a commit
to gilles-peskine-arm/mbed-crypto
that referenced
this pull request
Feb 3, 2020
* ARMmbed#321: Replace config.pl by config.py * ARMmbed#322: Update Mbed Crypto with latest Mbed TLS changes as of 2019-11-15 * ARMmbed#308: Small performance improvement of mbedtls_mpi_div_mpi() * ARMmbed#324: test_psa_constant_names: support key agreement, better code structure * ARMmbed#320: Link to the PSA crypto portal page from README.md * ARMmbed#293: Always gather MBEDTLS_ENTROPY_BLOCK_SIZE bytes of entropy * ARMmbed#310: Clarify test descriptions in test_suite_memory_buffer_alloc * ARMmbed#307: Add ASN.1 ENUMERATED tag support * ARMmbed#328: Remove dependency of crypto_values.h on crypto_extra.h * ARMmbed#325: Rename psa_asymmetric_{sign_verify} to psa_{sign,verify}_hash Missed listing in the previous submodule update: * ARMmbed#304: Make sure Asan failures are detected in 'make test'
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a fairly straightforward side port of Mbed-TLS/mbedtls#2765 to Mbed Crypto.
The main differences are different options being enabled under full and baremetal options, all.sh having different tests and changing PERL to PYTHON in
everest/CMakeLists.txt
.This was checked by running the following:
and checking that it only reported a differing nonzero error code in a few places, as in the previous PR.