Skip to content

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 27 commits into from
Nov 14, 2019
Merged

Conversation

dgreen-arm
Copy link
Contributor

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:

git show development:scripts/config.pl >|config.pl 
chmod +x config.pl
./tests/scripts/test_config_script.py -d old -s ./config.pl 
./tests/scripts/test_config_script.py -d new
diff -ruw old new

and checking that it only reported a differing nonzero error code in a few places, as in the previous PR.

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.
Copy link
Collaborator

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gilles-peskine-arm gilles-peskine-arm added the needs: review The pull request is ready for review. This generally means that it has no known issues. label Nov 13, 2019
Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Patater Patater removed the needs: review The pull request is ready for review. This generally means that it has no known issues. label Nov 14, 2019
@Patater Patater merged commit 63ad668 into ARMmbed:development Nov 14, 2019
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants