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

Always use python3 for zap_cluster_list.py #7589

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

mspang
Copy link
Contributor

@mspang mspang commented Jun 14, 2021

Problem

Unfortunately python is still python2 on some systems and this becomes
the default python interpreter for exec_script(). If we run
zap_cluster_list.py with python2, it's a parse error due to use of
python 3 only optional typing syntax.

If we instead execute the script via gn_run_binary.py, the shebang line
(#!/usr/bin/env python3) in the script will be respected. This is more
likely to work until python2 is dead everywhere...

Change overview

...so do that.

Testing

Tested by building as a subproject of a project that uses the default
script_executable in GN on a system with /usr/bin/python as python2.

Unfortunately python is still python2 on some systems and this becomes
the default python interpreter for exec_script(). If we run
zap_cluster_list.py with python2, it's a parse error due to use of
python 3 only optional typing syntax.

If we instead execute the script via gn_run_binary.py, the shebang line
(#!/usr/bin/env python3) in the script will be respected. This is more
likely to work until python2 is dead everywhere, so do that.

Tested by building as a subproject of a project that uses the default
script_executable in GN on a system with /usr/bin/python as python2.
@mspang mspang force-pushed the for-chip/fix-zap-cluster-py2 branch from ff75bb5 to 1104374 Compare June 14, 2021 05:56
Copy link
Contributor

@woody-apple woody-apple left a comment

Choose a reason for hiding this comment

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

Can we update this to conform to the standard template?

@mspang
Copy link
Contributor Author

mspang commented Jun 14, 2021

Can we update this to conform to the standard template?

I think you'll find that I covered all the bases? Can we please focus on substance over form?

@mspang mspang requested a review from woody-apple June 14, 2021 16:45
@andy31415 andy31415 dismissed woody-apple’s stale review June 14, 2021 17:47

summary updated to match the template

@mspang mspang merged commit 446e7ad into project-chip:master Jun 14, 2021
@mspang mspang deleted the for-chip/fix-zap-cluster-py2 branch June 14, 2021 18:55
mkardous-silabs pushed a commit to mkardous-silabs/connectedhomeip that referenced this pull request Jun 14, 2021
Unfortunately python is still python2 on some systems and this becomes
the default python interpreter for exec_script(). If we run
zap_cluster_list.py with python2, it's a parse error due to use of
python 3 only optional typing syntax.

If we instead execute the script via gn_run_binary.py, the shebang line
(#!/usr/bin/env python3) in the script will be respected. This is more
likely to work until python2 is dead everywhere, so do that.

Tested by building as a subproject of a project that uses the default
script_executable in GN on a system with /usr/bin/python as python2.
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
Unfortunately python is still python2 on some systems and this becomes
the default python interpreter for exec_script(). If we run
zap_cluster_list.py with python2, it's a parse error due to use of
python 3 only optional typing syntax.

If we instead execute the script via gn_run_binary.py, the shebang line
(#!/usr/bin/env python3) in the script will be respected. This is more
likely to work until python2 is dead everywhere, so do that.

Tested by building as a subproject of a project that uses the default
script_executable in GN on a system with /usr/bin/python as python2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants