Skip to content

Commit

Permalink
Merge pull request #823 from pv/fix-pip-install
Browse files Browse the repository at this point in the history
 Run pip in a different working directory instead of --force-reinstall
  • Loading branch information
pv authored May 30, 2019
2 parents b67ee45 + 774fc36 commit 2710cab
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 25 deletions.
20 changes: 11 additions & 9 deletions asv/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ def _interpolate_commands(self, commands):
Returns
-------
run_commands : list of (cmd, env, return_codes)
run_commands : list of (cmd, env, return_codes, cwd)
Parsed commands to run.
"""
Expand Down Expand Up @@ -595,12 +595,14 @@ def _interpolate_commands(self, commands):
# Interpolate, and raise useful error message if it fails
return [util.interpolate_command(c, kwargs) for c in commands]

def _interpolate_and_run_commands(self, commands, cwd):
def _interpolate_and_run_commands(self, commands, default_cwd):
interpolated = self._interpolate_commands(commands)

for cmd, env, return_codes in interpolated:
for cmd, env, return_codes, cwd in interpolated:
environ = dict(os.environ)
environ.update(env)
if cwd is None:
cwd = default_cwd
self.run_executable(cmd[0], cmd[1:], timeout=self._install_timeout, cwd=cwd,
env=environ, valid_return_codes=return_codes)

Expand Down Expand Up @@ -661,14 +663,14 @@ def _install_project(self, repo, commit_hash, build_dir):
cmd = self._install_command
if cmd is None:
# Run pip via python -m pip, avoids shebang length limit on Linux.
# Ensure --force-reinstall so that package being present e.g. on cwd
# does not mess things up.
cmd = ["python -mpip install --force-reinstall {wheel_file}"]
# Don't run it in build directory, because it may contain Python packages
# that pip believes to be already installed.
cmd = ["in-dir={env_dir} python -mpip install {wheel_file}"]

if cmd:
commit_name = repo.get_decorated_hash(commit_hash, 8)
log.info("Installing {0} into {1}".format(commit_name, self.name))
self._interpolate_and_run_commands(cmd, cwd=build_dir)
self._interpolate_and_run_commands(cmd, default_cwd=build_dir)

def _uninstall_project(self):
"""
Expand All @@ -685,7 +687,7 @@ def _uninstall_project(self):

if cmd:
log.info("Uninstalling from {0}".format(self.name))
self._interpolate_and_run_commands(cmd, cwd=self._env_dir)
self._interpolate_and_run_commands(cmd, default_cwd=self._env_dir)

def _build_project(self, repo, commit_hash, build_dir):
"""
Expand All @@ -700,7 +702,7 @@ def _build_project(self, repo, commit_hash, build_dir):
if cmd:
commit_name = repo.get_decorated_hash(commit_hash, 8)
log.info("Building {0} for {1}".format(commit_name, self.name))
self._interpolate_and_run_commands(cmd, cwd=build_dir)
self._interpolate_and_run_commands(cmd, default_cwd=build_dir)

def can_install_project(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion asv/template/asv.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
// Customizable commands for building, installing, and
// uninstalling the project. See asv.conf.json documentation.
//
// "install_command": ["python -mpip install {wheel_file}"],
// "install_command": ["in-dir={env_dir} python -mpip install {wheel_file}"],
// "uninstall_command": ["return-code=any python -mpip uninstall -y {project}"],
// "build_command": [
// "python setup.py build",
Expand Down
17 changes: 16 additions & 1 deletion asv/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,7 @@ def interpolate_command(command, variables):
- ``ENVVAR=value <command>``: parsed as declaring an environment variable
named 'ENVVAR'.
- ``return-code=value <command>``: parsed as declaring valid return codes.
- ``in-dir=value <command>``: parsed as declaring working directory for command.
Parameters
----------
Expand All @@ -1243,6 +1244,8 @@ def interpolate_command(command, variables):
Environment variables declared in the command.
return_codes : {set, int, None}
Valid return codes.
cwd : {str, None}
Current working directory for the command, if any.
"""

Expand All @@ -1260,6 +1263,7 @@ def interpolate_command(command, variables):

return_codes_set = False
return_codes = {0}
cwd = None

while result:
m = re.match('^([A-Za-z_][A-Za-z0-9_]*)=(.*)$', result[0])
Expand Down Expand Up @@ -1295,9 +1299,20 @@ def interpolate_command(command, variables):
"{0!r} when substituting into command {1!r} "
"".format(result[0], command))

if result[0].startswith('in-dir='):
if cwd is not None:
raise UserError("Configuration error: multiple in-dir specifications "
"in command {0!r} "
"".format(command))
break

cwd = result[0][7:]
del result[0]
continue

break

return result, env, return_codes
return result, env, return_codes, cwd


try:
Expand Down
9 changes: 6 additions & 3 deletions docs/source/asv.conf.json.rst
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Airspeed Velocity rebuilds the project as needed, using these commands.
The defaults are::

"install_command":
["python -mpip install --force-reinstall {wheel_file}"],
["in-dir={env_dir} python -mpip install {wheel_file}"],

"uninstall_command":
["return-code=any python -mpip uninstall -y {project}"],
Expand All @@ -91,8 +91,8 @@ will only call ``install_command`` but not ``build_command``. (The
number of cached builds retained at any time is determined by the
``build_cache_size`` configuration option.)

The ``install_command`` and ``build_command`` are launched in
``{build_dir}``. The ``uninstall_command`` is launched in the
The ``install_command`` and ``build_command`` are by default launched
in ``{build_dir}``. The ``uninstall_command`` is launched in the
environment root directory.

The commands are specified in typical POSIX shell syntax (Python
Expand All @@ -104,6 +104,9 @@ variable specifications in in form ``VARNAME=value`` at the beginning.
In addition, valid return codes can be specified via
``return-code=0,1,2`` and ``return-code=any``.

The ``in-dir=somedir`` specification changes the working directory
for the command.

The commands can be supplied with the arguments:

- ``{project}``: the project name from the configuration file
Expand Down
26 changes: 25 additions & 1 deletion test/test_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,6 @@ def get_env():

def test_installed_commit_hash(tmpdir):
tmpdir = six.text_type(tmpdir)
tmpdir = six.text_type(tmpdir)

dvcs = generate_test_repo(tmpdir, [0], dvcs_type='git')
commit_hash = dvcs.get_branch_hashes()[0]
Expand Down Expand Up @@ -709,3 +708,28 @@ def get_env():
env = get_env()
assert env.installed_commit_hash == None
assert env._env_vars.get('ASV_COMMIT') == None


def test_install_success(tmpdir):
# Check that install_project really installs the package. (gh-805)
# This may fail if pip in install_command e.g. gets confused by an .egg-info
# directory in its cwd to think the package is already installed.
tmpdir = six.text_type(tmpdir)

dvcs = generate_test_repo(tmpdir, [0], dvcs_type='git')
commit_hash = dvcs.get_branch_hashes()[0]

conf = config.Config()
conf.env_dir = os.path.join(tmpdir, "env")
conf.pythons = [PYTHON_VER1]
conf.repo = os.path.abspath(dvcs.path)
conf.matrix = {}
conf.build_cache_size = 0

repo = get_repo(conf)

env = list(environment.get_environments(conf, None))[0]
env.create()
env.install_project(conf, repo, commit_hash)

env.run(['-c', 'import asv_test_repo as t, sys; sys.exit(0 if t.dummy_value == 0 else 1)'])
6 changes: 5 additions & 1 deletion test/test_repo_template/setup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
from distutils.core import setup
from setuptools import setup

setup(name='asv_test_repo',
version="{version}",
packages=['asv_test_repo'],

# The following forces setuptools to generate .egg-info directory,
# which causes problems in test_environment.py:test_install_success
include_package_data=True,
)
23 changes: 14 additions & 9 deletions test/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,25 +297,28 @@ def test_json_non_ascii(tmpdir):
def test_interpolate_command():
good_items = [
('python {inputs}', dict(inputs='9'),
['python', '9'], {}, {0}),
['python', '9'], {}, {0}, None),

('python "{inputs}"', dict(inputs='9'),
['python', '9'], {}, {0}),
['python', '9'], {}, {0}, None),

('python {inputs}', dict(inputs=''),
['python', ''], {}, {0}),
['python', ''], {}, {0}, None),

('HELLO="asd" python "{inputs}"', dict(inputs='9'),
['python', '9'], {'HELLO': 'asd'}, {0}),
['python', '9'], {'HELLO': 'asd'}, {0}, None),

('HELLO="asd" return-code=any python "{inputs}"', dict(inputs='9'),
['python', '9'], {'HELLO': 'asd'}, None),
['python', '9'], {'HELLO': 'asd'}, None, None),

('HELLO="asd" return-code=255 python "{inputs}"', dict(inputs='9'),
['python', '9'], {'HELLO': 'asd'}, {255}),
['python', '9'], {'HELLO': 'asd'}, {255}, None),

('HELLO="asd" return-code=255 python "{inputs}"', dict(inputs='9'),
['python', '9'], {'HELLO': 'asd'}, {255}),
['python', '9'], {'HELLO': 'asd'}, {255}, None),

('HELLO="asd" in-dir="{somedir}" python', dict(somedir='dir'),
['python'], {'HELLO': 'asd'}, {0}, 'dir'),
]

bad_items = [
Expand All @@ -326,13 +329,15 @@ def test_interpolate_command():
('return-code=, python', {}),
('return-code=1,,2 python', {}),
('return-code=1 return-code=2 python', {}),
('in-dir=a in-dir=b python', {}),
]

for value, variables, e_parts, e_env, e_codes in good_items:
parts, env, codes = util.interpolate_command(value, variables)
for value, variables, e_parts, e_env, e_codes, e_cwd in good_items:
parts, env, codes, cwd = util.interpolate_command(value, variables)
assert parts == e_parts
assert env == e_env
assert codes == e_codes
assert cwd == e_cwd

for value, variables in bad_items:
with pytest.raises(util.UserError):
Expand Down

0 comments on commit 2710cab

Please sign in to comment.