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

convert options scope to text_type() when matching invalid options #7664

Conversation

Projects
None yet
3 participants
@cosmicexplorer
Copy link
Contributor

commented May 6, 2019

Problem

When using Python 2.7, users may not have from __future__ import unicode_literals or similar in their python sources, so their literal strings may end up being bytes, rather than unicode. However, our matching for invalid command-line options assumes they are already unicode, and errors out if not. This isn't necessary.

Solution

  • Wrap fields of _ScopedFlagNameForFuzzyMatching in text_type().

Result

Fixes breakage downstream. Tests weren't added since we are dropping py2 support very soon.

@cosmicexplorer cosmicexplorer added this to the 1.16.x milestone May 6, 2019

@Eric-Arellano
Copy link
Contributor

left a comment

External repos running pants in python 2.7 may not have literal strings being the exact string type we expect.

This took me a few tries to understand. I think this is what you're after:

When using Python 2.7, users may not have `from __future__ import unicode_literals` so their literal strings may end up being bytes, rather than unicode. We want to ensure it is always unicode, which our matching assumes is the case.
@cosmicexplorer

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

Thank you! I have adapted it slightly -- let me know if this works!

@stuhood

stuhood approved these changes May 6, 2019

@cosmicexplorer cosmicexplorer merged commit cc679b4 into pantsbuild:master May 7, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.