Skip to content
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

Use config.py as a module in depends.py #6493

Merged
merged 5 commits into from
Mar 2, 2023
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 57 additions & 41 deletions tests/scripts/depends.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
This script can be divided into several steps:

First, include/mbedtls/mbedtls_config.h or a different config file passed
in the arguments is parsed to extract any configuration options (collect_config_symbols).
in the arguments is parsed to extract any configuration options (using config.py).

Then, test domains (groups of jobs, tests) are built based on predefined data
collected in the DomainData class. Here, each domain has five major traits:
Expand Down Expand Up @@ -71,6 +71,11 @@
import subprocess
import sys
import traceback
from typing import Union

# Add the Mbed TLS Python library directory to the module search path
import scripts_path # pylint: disable=unused-import

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)

import config

class Colors: # pylint: disable=too-few-public-methods
"""Minimalistic support for colored output.
Expand All @@ -80,6 +85,7 @@ class Colors: # pylint: disable=too-few-public-methods
stop switches the text color back to the default."""
red = None
green = None
cyan = None
bold_red = None
bold_green = None
def __init__(self, options=None):
Expand All @@ -95,6 +101,7 @@ def __init__(self, options=None):
normal = '\033[0m'
self.red = ('\033[31m', normal)
self.green = ('\033[32m', normal)
self.cyan = ('\033[36m', normal)
self.bold_red = ('\033[1;31m', normal)
self.bold_green = ('\033[1;32m', normal)
NO_COLORS = Colors(None)
Expand Down Expand Up @@ -130,34 +137,41 @@ def restore_config(options):
else:
shutil.copy(options.config_backup, options.config)

def run_config_py(options, args):
"""Run scripts/config.py with the specified arguments."""
cmd = ['scripts/config.py']
if options.config != 'include/mbedtls/mbedtls_config.h':
cmd += ['--file', options.config]
cmd += args
log_command(cmd)
subprocess.check_call(cmd)
def option_exists(conf, option):
if option not in conf.settings:
return False
return True
AndrzejKurek marked this conversation as resolved.
Show resolved Hide resolved

def set_config_option_value(conf, option, colors, value: Union[bool, str]):
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this function (without the logging) be in config.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be an improvement if other scripts provide similar code.

"""Set/unset a configuration option, optionally specifying a value.
value can be either True/False (set/unset config option), or a string,
which will make a symbol defined with a certain value."""
if not option_exists(conf, option):
log_line('Symbol {} was not found in {}'.format(option, conf.filename), color=colors.red)
return False

if value is False:
log_command(['config.py', 'unset', option])
conf.unset(option)
else:
if value is True:
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that value can be of Boolean or String type?
Not sure if this is good solution. Safer would be to keep value always as String and assume possible values like: Enable, Disable, <Other Value>.

Then we would have:
set_config_option_value(conf, 'MBEDTLS_TEST_HOOKS', colors, 'Disable')

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to distinguish #define FOO from #define FOO Enable, so no, the current code is right in having a string for #define FOO ... vs True for #define FOO and False for no definition at all.

It would help to convey this through type annotations. value : Union[bool, str] and documentation explaining what I wrote above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added as a new commit.

log_command(['config.py', 'set', option])
conf.set(option)
else:
log_command(['config.py', 'set', option, value])
conf.set(option, value)
AndrzejKurek marked this conversation as resolved.
Show resolved Hide resolved
return True

def set_reference_config(options):
def set_reference_config(conf, options, colors):
"""Change the library configuration file (mbedtls_config.h) to the reference state.
The reference state is the one from which the tested configurations are
derived."""
# Turn off options that are not relevant to the tests and slow them down.
run_config_py(options, ['full'])
run_config_py(options, ['unset', 'MBEDTLS_TEST_HOOKS'])
log_command(['config.py', 'full'])
conf.adapt(config.full_adapter)
set_config_option_value(conf, 'MBEDTLS_TEST_HOOKS', colors, False)
if options.unset_use_psa:
run_config_py(options, ['unset', 'MBEDTLS_USE_PSA_CRYPTO'])

def collect_config_symbols(options):
"""Read the list of settings from mbedtls_config.h.
Return them in a generator."""
with open(options.config, encoding="utf-8") as config_file:
rx = re.compile(r'\s*(?://\s*)?#define\s+(\w+)\s*(?:$|/[/*])')
for line in config_file:
m = re.match(rx, line)
if m:
yield m.group(1)
set_config_option_value(conf, 'MBEDTLS_USE_PSA_CRYPTO', colors, False)

class Job:
"""A job builds the library in a specific configuration and runs some tests."""
Expand Down Expand Up @@ -185,19 +199,16 @@ def announce(self, colors, what):
elif what is False:
log_line(self.name + ' FAILED', color=colors.red)
else:
log_line('starting ' + self.name)
log_line('starting ' + self.name, color=colors.cyan)

def configure(self, options):
def configure(self, conf, options, colors):
'''Set library configuration options as required for the job.'''
set_reference_config(options)
set_reference_config(conf, options, colors)
for key, value in sorted(self.config_settings.items()):
if value is True:
args = ['set', key]
elif value is False:
args = ['unset', key]
else:
args = ['set', key, value]
run_config_py(options, args)
ret = set_config_option_value(conf, key, colors, value)
if ret is False:
return False
return True

def test(self, options):
'''Run the job's build and test commands.
Expand Down Expand Up @@ -392,11 +403,11 @@ def config_symbols_matching(self, regexp):
return [symbol for symbol in self.all_config_symbols
if re.match(regexp, symbol)]

def __init__(self, options):
def __init__(self, options, conf):
"""Gather data about the library and establish a list of domains to test."""
build_command = [options.make_command, 'CFLAGS=-Werror']
build_and_test = [build_command, [options.make_command, 'test']]
self.all_config_symbols = set(collect_config_symbols(options))
self.all_config_symbols = set(conf.settings.keys())
# Find hash modules by name.
hash_symbols = self.config_symbols_matching(r'MBEDTLS_(MD|RIPEMD|SHA)[0-9]+_C\Z')
# Find elliptic curve enabling macros by name.
Expand Down Expand Up @@ -455,16 +466,19 @@ def get_jobs(self, name):
else:
return [self.jobs[name]]

def run(options, job, colors=NO_COLORS):
def run(options, job, conf, colors=NO_COLORS):
"""Run the specified job (a Job instance)."""
subprocess.check_call([options.make_command, 'clean'])
job.announce(colors, None)
job.configure(options)
if not job.configure(conf, options, colors):
job.announce(colors, False)
return False
conf.write()
success = job.test(options)
job.announce(colors, success)
return success

def run_tests(options, domain_data):
def run_tests(options, domain_data, conf):
"""Run the desired jobs.
domain_data should be a DomainData instance that describes the available
domains and jobs.
Expand All @@ -480,7 +494,7 @@ def run_tests(options, domain_data):
backup_config(options)
try:
for job in jobs:
success = run(options, job, colors=colors)
success = run(options, job, conf, colors=colors)
if not success:
if options.keep_going:
failures.append(job.name)
Expand Down Expand Up @@ -546,7 +560,9 @@ def main():
default=True)
options = parser.parse_args()
os.chdir(options.directory)
domain_data = DomainData(options)
conf = config.ConfigFile(options.config)
domain_data = DomainData(options, conf)

if options.tasks is True:
options.tasks = sorted(domain_data.domains.keys())
if options.list:
Expand All @@ -555,7 +571,7 @@ def main():
print(domain_name)
sys.exit(0)
else:
sys.exit(0 if run_tests(options, domain_data) else 1)
sys.exit(0 if run_tests(options, domain_data, conf) else 1)
except Exception: # pylint: disable=broad-except
traceback.print_exc()
sys.exit(3)
Expand Down