-
Notifications
You must be signed in to change notification settings - Fork 76
Update build-script-helper.py to Python 3 #151
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
Conversation
@swift-ci Please test |
#!/usr/bin/env python | ||
|
||
from __future__ import print_function | ||
#!/usr/bin/env 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.
Depending on the python 3 version I'm not sure just this change is enough. It's fine for valid args but running with no args ends up with a stacktrace instead of error - I believe this is caused by the behavior of add_subparsers
. It should now take required=True
in Python >= 3.7.
Utilities/build-script-helper.py
Outdated
@@ -93,7 +91,7 @@ def add_common_args(parser): | |||
parser.add_argument('--sanitize-all', action='store_true', help='build using every available sanitizer in sub-directories of build path') | |||
parser.add_argument('--verbose', '-v', action='store_true', help='enable verbose output') | |||
|
|||
subparsers = parser.add_subparsers(title='subcommands', dest='action', metavar='action') | |||
subparsers = parser.add_subparsers(title='subcommands', dest='action', required=True, metavar='action') |
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.
Does PR CI run on Ubuntu 18.04? 3.7 was released in 2018, so wouldn't surprise me if it didn't have it.
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 just checked, they run Python 3.6.9 😬 I guarded the required=True
option behind a Python version check.
When running locally, it looks like we just need to change the shebang.
#150