Skip to content

BUG: fix checkout_output with non-ascii paths in PATH #7804

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 13, 2017
Merged

BUG: fix checkout_output with non-ascii paths in PATH #7804

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df27722
BUG: fix checkout_output with non-ascii paths in PATH
tacaswell Jan 12, 2017
e023faa
STY: fix whitespace
tacaswell Jan 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
BUG: fix checkout_output with non-ascii paths in PATH 
Closes #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.
  • Loading branch information
@tacaswell
tacaswell committed Jan 12, 2017
commit df27722abe45859bfee8bd6fc88c29bcef11c39c
3 changes: 3 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ before_install:
- pip install --upgrade virtualenv
- virtualenv --python=python venv
- source venv/bin/activate
# test with non-ascii in path
- mkdir /tmp/λ
- export PATH=$PATH:/tmp/λ
- export PATH=/usr/lib/ccache:$PATH

install:
Expand Down
13 changes: 7 additions & 6 deletions lib/matplotlib/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ def ge(self, level):

def checkdep_dvipng():
try:
s = subprocess.Popen(['dvipng', '-version'], stdout=subprocess.PIPE,
s = subprocess.Popen([str('dvipng'), '-version'],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
stdout, stderr = s.communicate()
line = stdout.decode('ascii').split('\n')[1]
Expand All @@ -374,7 +375,7 @@ def checkdep_ghostscript():
for gs_exec in gs_execs:
try:
s = subprocess.Popen(
[gs_exec, '--version'], stdout=subprocess.PIPE,
[str(gs_exec), '--version'], stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
stdout, stderr = s.communicate()
if s.returncode == 0:
Expand All @@ -390,7 +391,7 @@ def checkdep_ghostscript():

def checkdep_tex():
try:
s = subprocess.Popen(['tex', '-version'], stdout=subprocess.PIPE,
s = subprocess.Popen([str('tex'), '-version'], stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
stdout, stderr = s.communicate()
line = stdout.decode('ascii').split('\n')[0]
Expand All @@ -404,7 +405,7 @@ def checkdep_tex():

def checkdep_pdftops():
try:
s = subprocess.Popen(['pdftops', '-v'], stdout=subprocess.PIPE,
s = subprocess.Popen([str('pdftops'), '-v'], stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
stdout, stderr = s.communicate()
lines = stderr.decode('ascii').split('\n')
Expand All @@ -419,7 +420,7 @@ def checkdep_pdftops():
def checkdep_inkscape():
if checkdep_inkscape.version is None:
try:
s = subprocess.Popen(['inkscape', '-V'], stdout=subprocess.PIPE,
s = subprocess.Popen([str('inkscape'), '-V'], stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
stdout, stderr = s.communicate()
lines = stdout.decode('ascii').split('\n')
Expand All @@ -436,7 +437,7 @@ def checkdep_inkscape():

def checkdep_xmllint():
try:
s = subprocess.Popen(['xmllint', '--version'], stdout=subprocess.PIPE,
s = subprocess.Popen([str('xmllint'), '--version'], stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
stdout, stderr = s.communicate()
lines = stderr.decode('ascii').split('\n')
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/animation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def bin_path(cls):
subclass. This is a class method so that the tool can be looked for
before making a particular MovieWriter subclass available.
'''
return rcParams[cls.exec_key]
return str(rcParams[cls.exec_key])

@classmethod
def isAvailable(cls):
Expand Down
17 changes: 9 additions & 8 deletions lib/matplotlib/backends/backend_pgf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
# assuming fontconfig is installed and the command 'fc-list' exists
try:
# list scalable (non-bitmap) fonts
fc_list = check_output(['fc-list', ':outline,scalable', 'family'])
fc_list = check_output([str('fc-list'), ':outline,scalable', 'family'])
fc_list = fc_list.decode('utf8')
system_fonts = [f.split(',')[0] for f in fc_list.splitlines()]
system_fonts = list(set(system_fonts))
Expand Down Expand Up @@ -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)
Copy link
Contributor

@anntzer anntzer Jan 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Not needed around "-v" (I guess).

tools_available.append("pdftocairo")
except:
pass
Expand All @@ -191,14 +191,15 @@ def make_pdf_to_png_converter():
# pick converter
if "pdftocairo" in tools_available:
def cairo_convert(pdffile, pngfile, dpi):
cmd = ["pdftocairo", "-singlefile", "-png",
cmd = [str("pdftocairo"), "-singlefile", "-png",
"-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)
Copy link
Contributor

Choose a reason for hiding this comment

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

fits on one line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this went through some thrashing..

return cairo_convert
elif "gs" in tools_available:
def gs_convert(pdffile, pngfile, dpi):
cmd = [gs, '-dQUIET', '-dSAFER', '-dBATCH', '-dNOPAUSE', '-dNOPROMPT',
cmd = [str(gs), '-dQUIET', '-dSAFER', '-dBATCH', '-dNOPAUSE', '-dNOPROMPT',
'-sDEVICE=png16m', '-dUseCIEColor', '-dTextAlphaBits=4',
'-dGraphicsAlphaBits=4', '-dDOINTERPOLATE', '-sOutputFile=%s' % pngfile,
'-r%d' % dpi, pdffile]
Expand Down Expand Up @@ -300,7 +301,7 @@ def __init__(self):
self.latex_header = LatexManager._build_latex_header()
latex_end = "\n\\makeatletter\n\\@@end\n"
try:
latex = subprocess.Popen([self.texcommand, "-halt-on-error"],
latex = subprocess.Popen([str(self.texcommand), "-halt-on-error"],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
cwd=self.tmpdir)
Expand All @@ -318,7 +319,7 @@ def __init__(self):
raise LatexError("LaTeX returned an error, probably missing font or error in preamble:\n%s" % stdout)

# open LaTeX process for real work
latex = subprocess.Popen([self.texcommand, "-halt-on-error"],
latex = subprocess.Popen([str(self.texcommand), "-halt-on-error"],
stdin=subprocess.PIPE, stdout=subprocess.PIPE,
cwd=self.tmpdir)
self.latex = latex
Expand Down Expand Up @@ -895,7 +896,7 @@ def _print_pdf_to_fh(self, fh, *args, **kwargs):
fh_tex.write(latexcode)

texcommand = get_texcommand()
cmdargs = [texcommand, "-interaction=nonstopmode",
cmdargs = [str(texcommand), "-interaction=nonstopmode",
"-halt-on-error", "figure.tex"]
try:
check_output(cmdargs, stderr=subprocess.STDOUT, cwd=tmpdir)
Expand Down
8 changes: 4 additions & 4 deletions lib/matplotlib/cbook.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1437,7 +1437,7 @@ def report_memory(i=0): # argument may go away
pid = os.getpid()
if sys.platform == 'sunos5':
try:
a2 = Popen('ps -p %d -o osz' % pid, shell=True,
a2 = Popen(str('ps -p %d -o osz') % pid, shell=True,
stdout=PIPE).stdout.readlines()
except OSError:
raise NotImplementedError(
Expand All @@ -1446,7 +1446,7 @@ def report_memory(i=0): # argument may go away
mem = int(a2[-1].strip())
elif sys.platform.startswith('linux'):
try:
a2 = Popen('ps -p %d -o rss,sz' % pid, shell=True,
a2 = Popen(str('ps -p %d -o rss,sz') % pid, shell=True,
stdout=PIPE).stdout.readlines()
except OSError:
raise NotImplementedError(
Expand All @@ -1455,7 +1455,7 @@ def report_memory(i=0): # argument may go away
mem = int(a2[1].split()[1])
elif sys.platform.startswith('darwin'):
try:
a2 = Popen('ps -p %d -o rss,vsz' % pid, shell=True,
a2 = Popen(str('ps -p %d -o rss,vsz') % pid, shell=True,
stdout=PIPE).stdout.readlines()
except OSError:
raise NotImplementedError(
Expand All @@ -1464,7 +1464,7 @@ def report_memory(i=0): # argument may go away
mem = int(a2[1].split()[0])
elif sys.platform.startswith('win'):
try:
a2 = Popen(["tasklist", "/nh", "/fi", "pid eq %d" % pid],
a2 = Popen([str("tasklist"), "/nh", "/fi", "pid eq %d" % pid],
stdout=PIPE).stdout.read()
except OSError:
raise NotImplementedError(
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/dviread.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ def find_tex_file(filename, format=None):
The library that :program:`kpsewhich` is part of.
"""

cmd = ['kpsewhich']
cmd = [str('kpsewhich')]
if format is not None:
cmd += ['--format=' + format]
cmd += [filename]
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/font_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def _call_fc_list():
'This may take a moment.'))
timer.start()
try:
out = subprocess.check_output(['fc-list', '--format=%{file}'])
out = subprocess.check_output([str('fc-list'), '--format=%{file}'])
except (OSError, subprocess.CalledProcessError):
return []
finally:
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/sphinxext/tests/test_tinypages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def setup_class(cls):
cls.html_dir = pjoin(cls.page_build, 'html')
cls.doctree_dir = pjoin(cls.page_build, 'doctrees')
# Build the pages with warnings turned into errors
cmd = ['sphinx-build', '-W', '-b', 'html',
cmd = [str('sphinx-build'), '-W', '-b', 'html',
'-d', cls.doctree_dir,
TINY_PAGES,
cls.html_dir]
Expand Down
4 changes: 2 additions & 2 deletions lib/matplotlib/testing/compare.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,14 @@ def _update_converter():
gs, gs_v = matplotlib.checkdep_ghostscript()
if gs_v is not None:
def cmd(old, new):
return [gs, '-q', '-sDEVICE=png16m', '-dNOPAUSE', '-dBATCH',
return [str(gs), '-q', '-sDEVICE=png16m', '-dNOPAUSE', '-dBATCH',
'-sOutputFile=' + new, old]
converter['pdf'] = make_external_conversion_command(cmd)
converter['eps'] = make_external_conversion_command(cmd)

if matplotlib.checkdep_inkscape() is not None:
def cmd(old, new):
return ['inkscape', '-z', old, '--export-png', new]
return [str('inkscape'), '-z', old, '--export-png', new]
converter['svg'] = make_external_conversion_command(cmd)


Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/tests/test_backend_pgf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def check_for(texsystem):
\\@@end
"""
try:
latex = subprocess.Popen([texsystem, "-halt-on-error"],
latex = subprocess.Popen([str(texsystem), "-halt-on-error"],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE)
stdout, stderr = latex.communicate(header.encode("utf8"))
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/texmanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@

def dvipng_hack_alpha():
try:
p = Popen(['dvipng', '-version'], stdin=PIPE, stdout=PIPE,
p = Popen([str('dvipng'), '-version'], stdin=PIPE, stdout=PIPE,
stderr=STDOUT, close_fds=(sys.platform != 'win32'))
stdout, stderr = p.communicate()
except OSError:
Expand Down