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

Makefiles use wrong Python when building on Linux for Windows #4774

Closed
gilles-peskine-arm opened this issue Jul 13, 2021 · 0 comments · Fixed by #4776
Closed

Makefiles use wrong Python when building on Linux for Windows #4774

gilles-peskine-arm opened this issue Jul 13, 2021 · 0 comments · Fixed by #4776
Assignees
Labels
bug component-platform Portability layer and build scripts size-s Estimated task size: small (~2d)

Comments

@gilles-peskine-arm
Copy link
Contributor

In the GNU makefiles, WINDOWS is defined when building on Windows, and WINDOWS_BUILD is defined when building for Windows. The selection of Python (python on Windows, python3 elsewhere — a practical choice that works for us) is based on WINDOWS_BUILD but should be based on WINDOWS.

This does not currently cause any CI failure or uncaught failure, but it's very fragile.

This causes an error when building with mingw on Linux, as in tests/scripts/all.sh build_mingw. Since PYTHON is python, which is Python 2.7, all Python code invoked from makefiles runs with Python 2. For generate_test_code.py, this still works because this script was last modified a long time ago when we ensured that our scripts worked with both Python 2.x and Python 3.x. In the development branch, tests/Makefile runs $(PYTHON) scripts/generate_psa_tests.py --list to enumerate potentially generated test data files, which results in the error

  File "scripts/generate_psa_tests.py", line 40
    def psa_want_symbol(name: str) -> str:
                            ^
SyntaxError: invalid syntax

This is currently harmless on the CI because the error occurs in a $(shell …) make function which does not check the command's return status, and all the generated test data is pre-generated before doing the build.

@gilles-peskine-arm gilles-peskine-arm added bug component-platform Portability layer and build scripts Product Backlog labels Jul 13, 2021
@gilles-peskine-arm gilles-peskine-arm added the size-s Estimated task size: small (~2d) label Jul 13, 2021
@gilles-peskine-arm gilles-peskine-arm self-assigned this Jul 13, 2021
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Jul 13, 2021
The makefiles look for python3 on Unix-like systems where python is often
Python 2. This uses sh code so it doesn't work on Windows. On Windows, the
makefiles just assume that python is Python 3.

The code was incorrectly deciding not to try python3 based on WINDOWS_BUILD,
which indicates that the build is *for* Windows. Switch to checking WINDOWS,
which indicates that the build is *on* Windows.

Fix Mbed-TLS#4774

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Jul 21, 2021
The makefiles look for python3 on Unix-like systems where python is often
Python 2. This uses sh code so it doesn't work on Windows. On Windows, the
makefiles just assume that python is Python 3.

The code was incorrectly deciding not to try python3 based on WINDOWS_BUILD,
which indicates that the build is *for* Windows. Switch to checking WINDOWS,
which indicates that the build is *on* Windows.

Fix Mbed-TLS#4774

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Jul 21, 2021
The makefiles look for python3 on Unix-like systems where python is often
Python 2. This uses sh code so it doesn't work on Windows. On Windows, the
makefiles just assume that python is Python 3.

The code was incorrectly deciding not to try python3 based on WINDOWS_BUILD,
which indicates that the build is *for* Windows. Switch to checking WINDOWS,
which indicates that the build is *on* Windows.

Fix Mbed-TLS#4774

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@mpg mpg closed this as completed in #4776 Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-platform Portability layer and build scripts size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant