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

scripts: twister: Don't use match/case statements #78671

Merged

Conversation

golowanow
Copy link
Member

@golowanow golowanow commented Sep 18, 2024

Don't use match/case syntax to allow twister run with python3 versions < 3.10.

To avoid confusing SyntaxError after recent #78345 with older python3 despite Zephyr requires min 3.10.

$ ./scripts/twister --help
Traceback (most recent call last):
  File "./scripts/twister", line 207, in <module>
    from twisterlib.twister_main import main
  File "zephyr/scripts/pylib/twister/twisterlib/twister_main.py", line 16, in <module>
    from twisterlib.statuses import TwisterStatus
  File "zephyr/scripts/pylib/twister/twisterlib/statuses.py", line 26
    match(status):
                 ^
SyntaxError: invalid syntax

Don't use match/case syntax to allow twister to run with
python3 versions < 3.10.

Signed-off-by: Dmitrii Golovanov <[email protected]>
@LukaszMrugala
Copy link
Collaborator

I have no criticism of the PR per se, it should work well.

Although I doubt that chaining ourselves to an arbitrary older version of Python is a good choice. Zephyr requires 3.10, our CMake setup demands 3.8, Python devicetree wants at least 3.6 - where do we draw the line? Note that Python 3.8 is almost at its End of Life.

Shouldn't we simply add a guard at the start of twister_main.py or twister to check the user's version and exit gracefully if it is not >=$min._Zephyr_version, informing the user of the problem?

@golowanow
Copy link
Member Author

golowanow commented Sep 20, 2024

@LukaszMrugala it is a good point to have some log message at Twister instead of a syntax error as well as a guard, although while the min. python requirement is not aligned yet at the project level for all its components, it seems better to avoid enforcing 3.10 new syntax if the same is still doable by simpler/older means.
See also #78317

@henrikbrixandersen
Copy link
Member

I must admit I don't like this. We have a minimum required Python version for a reason. If we start accepting patches for making our scripts compatible with older versions, how will we ever move on?

@nashif
Copy link
Member

nashif commented Sep 20, 2024

I must admit I don't like this. We have a minimum required Python version for a reason. If we start accepting patches for making our scripts compatible with older versions, how will we ever move on?

agree, however our move to 3.10 was not done properly and not communicated well enough, as mentioned above, looks like besode the official minimal version required, every part of zephyr right now has a requirement on something else, which is very confusing. Add to the list above that the zephyr sdk requires 3.8 :(

@golowanow what does actually drive this change? Where do you have 3.8 as a dependency that can't be changed to a newer version?

@golowanow
Copy link
Member Author

what does actually drive this change? Where do you have 3.8 as a dependency that can't be changed to a newer version?

this particular change is driven (as described) by user experience, for example: #78345 (comment) or
https://discord.com/channels/720317445772017664/991025779993370654/1286610279400210482

I think the above mentioned circumstances around min python version, might make an issue where Zephyr and Twister are still used on Ubuntu 20.04 with its EOSS Apr/2025 and which is still mentioned on Zephyr Getting Started Guide
This guide covers Ubuntu version 20.04 LTS and later

I definitely agree Twister should have a guard for python version which it ACTUALLY reequires, and it must be aligned with other Zephyr components, better if done simultaneously with some sensible update like Zephyr or SDK new version release. Until that happens, this PR is just a fix for the recent code added to re-implement it differently with ease.

@fabiobaltieri fabiobaltieri merged commit 4cc3134 into zephyrproject-rtos:main Sep 21, 2024
34 checks passed
@golowanow golowanow deleted the twister-status-dict_20240918 branch September 21, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants