-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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
Tests: Use portable #! in python scripts (/usr/bin/env) #8270
Conversation
@@ -1,4 +1,4 @@ | |||
#!/usr/bin/python2 | |||
#!/usr/bin/env python2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this say #!/usr/bin/env python
, since #7723?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, security-check
works with python 3 too. The build system calls all these scripts by passing them to the python interpreter, so it doesn't care what is on the first line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made no attempt to check for portability of the python scripts between python releases, just a straight s/// on all *.py files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the trailing 2's according to the pull request I linked above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 5 files using python2, only security-check.py and symbol-check.py (notably not test-security-check.py) have been changed in #7723. Accordingly I've changed only the same two files.
contrib/devtools/security-check.py:#!/usr/bin/env python2
contrib/devtools/symbol-check.py:#!/usr/bin/env python2
contrib/devtools/test-security-check.py:#!/usr/bin/env python2
contrib/zmq/zmq_sub.py:#!/usr/bin/env python2
share/rpcuser/rpcuser.py:#!/usr/bin/env python2
I'm happy to abandon this PR in favour of incorporating the changes directly in #7723 if it simplifies things.
@@ -1,4 +1,4 @@ | |||
#!/usr/bin/python2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we actually test that this works on python3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petertodd See @laanwj's comment #8270 (comment)
utACK 7b01ce2 |
utACK 7b01ce2 |
1 similar comment
utACK 7b01ce2 |
7b01ce2 Favour python over python2 as per PR #7723 (Matthew King) 873e81f Use portable #! in python scripts (/usr/bin/env) (Matthew King)
…n/env) 7b01ce2 Favour python over python2 as per PR bitcoin#7723 (Matthew King) 873e81f Use portable #! in python scripts (/usr/bin/env) (Matthew King)
Fix MacOS tests Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.
Fix MacOS tests Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.
Fix MacOS tests Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.
…n/env) 7b01ce2 Favour python over python2 as per PR bitcoin#7723 (Matthew King) 873e81f Use portable #! in python scripts (/usr/bin/env) (Matthew King)
These changes should also be backported to the stable releases so that they build and test on systems which don't install python to /usr
It also changes python scripts which are not related to tests because it was easier not to have grep discriminate.