Skip to content

Remove usage of py.process #622

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 1 commit into from
Sep 11, 2017
Merged

Remove usage of py.process #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 20 additions & 13 deletions tests/test_interpreters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import distutils.spawn
import os
import subprocess
import sys
Expand All @@ -19,20 +20,26 @@ def interpreters():

@pytest.mark.skipif("sys.platform != 'win32'")
def test_locate_via_py(monkeypatch):
class PseudoPy:
def sysexec(self, *args):
assert args[0] == '-3.2'
assert args[1] == '-c'
# Return value needs to actually exist!
return sys.executable

@staticmethod
def ret_pseudopy(name):
assert name == 'py'
return PseudoPy()
# Monkeypatch py.path.local.sysfind to return PseudoPy
monkeypatch.setattr(py.path.local, 'sysfind', ret_pseudopy)
from tox.interpreters import locate_via_py

def fake_find_exe(exe):
assert exe == 'py'
return 'py'

def fake_popen(cmd, stdout):
assert cmd[:3] == ('py', '-3.2', '-c')

class proc:
returncode = 0

@staticmethod
def communicate():
return sys.executable.encode(), None
return proc

# Monkeypatch modules to return our faked value
monkeypatch.setattr(distutils.spawn, 'find_executable', fake_find_exe)
monkeypatch.setattr(subprocess, 'Popen', fake_popen)
assert locate_via_py('3', '2') == sys.executable


Expand Down
18 changes: 9 additions & 9 deletions tox/interpreters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import distutils.util
import inspect
import re
import subprocess
import sys

import py
Expand Down Expand Up @@ -164,16 +166,14 @@ def tox_get_python_executable(envconfig):
def locate_via_py(v_maj, v_min):
ver = "-%s.%s" % (v_maj, v_min)
script = "import sys; print(sys.executable)"
py_exe = py.path.local.sysfind('py')
py_exe = distutils.spawn.find_executable('py')
if py_exe:
try:
exe = py_exe.sysexec(ver, '-c', script).strip()
except py.process.cmdexec.Error:
exe = None
if exe:
exe = py.path.local(exe)
if exe.check():
return exe
proc = subprocess.Popen(
(py_exe, ver, '-c', script), stdout=subprocess.PIPE,
)
out, _ = proc.communicate()
if not proc.returncode:
return out.decode('UTF-8').strip()


def pyinfo():
Expand Down
15 changes: 7 additions & 8 deletions tox/session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import subprocess
import sys
import time
from subprocess import STDOUT

import py

Expand Down Expand Up @@ -138,7 +137,7 @@ def popen(self, args, cwd=None, env=None, redirect=True, returnout=False, ignore
cwd = py.path.local()
try:
popen = self._popen(args, cwd, env=env,
stdout=stdout, stderr=STDOUT)
stdout=stdout, stderr=subprocess.STDOUT)
except OSError as e:
self.report.error("invocation failed (errno %d), args: %s, cwd: %s" %
(e.errno, args, cwd))
Expand Down Expand Up @@ -673,12 +672,12 @@ def report_env(e):

def info_versions(self):
versions = ['tox-%s' % tox.__version__]
try:
version = py.process.cmdexec("virtualenv --version")
except py.process.cmdexec.Error:
versions.append("virtualenv-1.9.1 (vendored)")
else:
versions.append("virtualenv-%s" % version.strip())
proc = subprocess.Popen(
(sys.executable, '-m', 'virtualenv', '--version'),
stdout=subprocess.PIPE,
)
out, _ = proc.communicate()
versions.append('virtualenv-{0}'.format(out.decode('UTF-8').strip()))
Copy link
Member

Choose a reason for hiding this comment

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

I personally don't like the .format way of string formatting, but as long as no one is forcing me to use it myself I can live with it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interesting! I prefer it over % formatting 😆

Copy link
Member

Choose a reason for hiding this comment

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

@obestwalter Maybe https://pyformat.info/ will convince you otherwise? 😉

Copy link
Member

Choose a reason for hiding this comment

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

@The-Compiler I am afraid not. I don't see any value in using .format for simple string formatting. So for now I will still choose

'some text %s bla bla' % somevar

over

'some text {0} bla bla'.format(somevar)

... until I know better.

I also wish that Guido had engaged his time machine and introduced f-strings right away rather than in 3.6 ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

with clever evil hacks, I could probably get this to work in python2.7

# -*- encoding: future_fstrings -*-
thing = 'world'
print(f'hello {thing}')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is done! https://github.com/asottile/future-fstrings

Copy link
Member

Choose a reason for hiding this comment

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

Hah! Oh dear! 😆

Copy link
Member

Choose a reason for hiding this comment

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

O.M.G.

Copy link
Member

Choose a reason for hiding this comment

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

@asottile any performance hit compared to format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They'll be slower that native f string support but this hack will have identical performance to .format (it translates the f string into format calls in fact). Native f strings are only faster because they are implemented directly in byte code (which a backport can't quite do)

self.report.keyvalue("tool-versions:", " ".join(versions))

def _resolve_pkg(self, pkgspec):
Expand Down