BUG: fix checkout_output with non-ascii paths in PATH #7804
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes matplotlib#7715 This ensures that on python2 the values passed into `check_output` or `Popen` are bytes. If they are passed in as unicode (due to unicode_literals at the top of most of our files) they will cause an exception when there is a path in PATH that has non-ascii values. The reason is that when locating possible executable our input (as unicode) is concatenated with the non-ascii path (as bytes) which then fails to encode as ascii and raises. The other two places we currently use `check_output` (setupext.py and tools/github_stats.py) we do not need this handling as we do not import unicode_iterals in those files.
3b2a36d to
df27722
Compare
anntzer
reviewed
Jan 13, 2017
| @@ -179,7 +179,7 @@ def make_pdf_to_png_converter(): | |||
| tools_available = [] | |||
| # check for pdftocairo | |||
| try: | |||
| check_output(["pdftocairo", "-v"], stderr=subprocess.STDOUT) | |||
| check_output([str("pdftocairo"), str("-v")], stderr=subprocess.STDOUT) | |||
There was a problem hiding this comment.
Not needed around "-v" (I guess).
anntzer
reviewed
Jan 13, 2017
| "-r %d" % dpi, pdffile, os.path.splitext(pngfile)[0]] | ||
| # for some reason this doesn't work without shell | ||
| check_output(" ".join(cmd), shell=True, stderr=subprocess.STDOUT) | ||
| check_output(cmd, shell=True, | ||
| stderr=subprocess.STDOUT) |
There was a problem hiding this comment.
Yeah, this went through some thrashing..
|
There seems to be a PEP8 problem. |
|
This way by far one of the most frustrating bugs to fix I have fixed in a while...not clear why though 🤷♂️ . |
|
Because Py2? :) Anyways, LGTM (after squashing I'd say). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #7715
This ensures that on python2 the values passed into
check_outputarebytes. If they are passed in as unicode (due to unicode_literals at
the top of most of our files) they will cause an exception when there
is a path in PATH that has non-ascii values. The reason is that when
locating possible executable our input (as unicode) is concatenated
with the non-ascii path (as bytes) which then fails to encode as ascii
and raises.
The other two places we currently use
check_output(setupext.py andtools/github_stats.py) we do not need this handling as we do not
import unicode_iterals in those files.