Python upscale: pass mypy and create library directory #3953
Conversation
This file contains 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
.py files may be modules which are not standalone program, so allow them not to be executable. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Executable scripts must have shebang (#!) line to be effectively executable on most Unix-like systems. Enforce this, and conversely enforce that files with a shebang line are executable. Check that the specified interperter is consistent with the file extension. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Add enough type annotations to pass mypy 0.782 with Python 3.5. The source code will still run normally under older Python versions. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This makes the code cleaner. As a bonus, mypy no longer gets confused. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Since no typing stubs are available for mbed_host_tests.py, mypy
errors out on mbedtls_test.py with
error: Skipping analyzing 'mbed_host_tests': found module but no type hints or library stubs
Ignore this import to get at least some benefit from mypy without
spending significant effort to write stubs.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Allow Python scripts in tests/scripts to import modules located in the scripts directory. To do this, use ``` import scripts_path # pylint: disable=unused-import ``` Declare the scripts directory to pylint and to mypy. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Generalize the very ad hoc run_c function into a function to generate a C program to print the value of a list of expressions. Refactor the code into several functions to make it more manageable. No intended behavior change. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Create a directory mbedtls_dev intended to contain various Python module for use by Python scripts located anywhere in the Mbed TLS source tree. Move get_c_expression_values and its auxiliary functions into a new Python module mbedtls_dev.c_build_helper. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Tell mypy to support packages without an __init__.py (PEP 420 namespace packages). Python 3.3 and (modern) Pylint support them out of the box, but mypy needs to be told to support them. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
added
mbed TLS team
needs-review
1 task
mypy automatically checks the modules when it encounters them as imports. Don't make it check them twice, because it would complain about encountering them through different paths (via the command line as scripts/mbedtls_dev/*.py and via imports as just mbedtls_dev/*.py). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
With just the option --can-pylint or --can-mypy, check whether the requisite tool is available with an acceptable version and exit. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
force-pushed
the
python-mypy-mkdir
branch
from
d3084e2 to
faa8c06
Compare
daverodgman
reviewed
Jan 11, 2021
ronald-cron-arm
removed
the
needs-reviewer
gilles-peskine-arm
force-pushed
the
python-mypy-mkdir
branch
from
faa8c06 to
1cc6a8e
Compare
|
Since the Docker images in our CI have been updated with Pylint 2.4.4, I've rescinded faa8c06. I now expect the CI to pass. |
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Now that the script might additionally run mypy, it's more user-friendly to indicate what's going on at the beginning as well. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This reduces dependencies, doesn't require maintainers to know awk, and makes the version parsing more robust. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
As indicated in the comments in the can_mypy function, we don't just need a mypy executable to be present, we need it to work. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Make sure to run mypy with the desired Python version. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
ronald-cron-arm
requested changes
Jan 20, 2021
0.780 works. The previous release, 0.770, does not. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
`tests/scripts/all.sh check_python_files` now runs mypy (in addition to pylint) if it's available. So install mypy. Install mypy 0.780, which is the earliest version that works on our code at this time. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
force-pushed
the
python-mypy-mkdir
branch
from
0849bfc to
f71ff1f
Compare
ronald-cron-arm
approved these changes
Jan 21, 2021
|
@daverodgman are you still ok with the last version? |
daverodgman
approved these changes
Jan 29, 2021
| $PYTHON -m pylint -j 2 scripts/*.py tests/scripts/*.py | ||
| check_version () { | ||
| $PYTHON - "$2" <<EOF | ||
| import packaging.version |
There was a problem hiding this comment.
I hadn't realized when I wrote this that packaging is a third-party module.
# for free
to join this conversation on GitHub.
Already have an account?
# 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 pull request combines several improvements to the structure of our Python code.
mypy(version 0.780 or above).tests/scripts/check-python-files.shnow runsmypyif available. This PR makes mypy run on Travis. It doesn't yet run on Jenkins.tests/scriptscan now import scripts from the toplevelscriptsdirectory without undue hassle and without angering pylint or mypy.scripts/mbedtls_dev.run_cfunction fromtest_psa_constant_names.pyto a new modulembedtls_dev.c_build_helper.Backports: not strictly necessary, but it would be better to backport the changes to the Python scripts that exist in LTS branches to keep them aligned and facilitate future backports.