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

Virtualenv cell magic #1891

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 64 additions & 0 deletions IPython/extensions/tests/test_virtualenvmagic.py
@@ -0,0 +1,64 @@
# -*- coding: utf-8 -*-
"""
Tests for the virtualenvmagic extension
Author: Flávio Codeço Coelho - @fccoelho
"""

import nose.tools as nt
import os, shutil
from nose import SkipTest
import virtualenv
from IPython.utils.tempdir import TemporaryDirectory

home_dir = os.getenv('HOME')
user_venv_dir = os.path.join(home_dir,'Envs/') if not "WORKON_HOME" \
in os.environ else os.getenv("WORKON_HOME")
if not os.path.exists(os.path.join(home_dir,'Envs/')):
os.mkdir(os.path.join(home_dir,'Envs/'))

global testenv_dir



def setup():
global testenv_dir
ip = get_ipython()
#creating a testing virtualenv
testenv_dir = TemporaryDirectory()
os.environ['WORKON_HOME'] = testenv_dir.name
virtualenv.create_environment(os.path.join(testenv_dir.name,'testenv'))
ip.extension_manager.load_extension('virtualenvmagic')

def teardown():
del os.environ['WORKON_HOME']
testenv_dir.cleanup()



def test_virtualenv_testenv():
ip = get_ipython()
result = ip.run_cell_magic('virtualenv', 'testenv', 'import sys;print(sys.path)')
nt.assert_equals('tmp', os.path.split(eval(result)[1])[0].split(os.path.sep)[1])
Copy link
Member

Choose a reason for hiding this comment

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

Not a very robust test - all sorts of things can be in /tmp. And it's probably not called /tmp on Windows.

What about something like nt.assert_in(testenv_dir.name, result)?


def test_virtualenv_nonexisting():
ip = get_ipython()
result = ip.run_cell_magic('virtualenv', 'nonexistingenv', 'print("hello")')
nt.assert_equals(None, result)

def test_capturing_python_error():
ip = get_ipython()
result = ip.run_cell_magic('virtualenv', 'testenv', 'print(x)')
# traceback is sent to stderror
nt.assert_equals("",result)

@nt.nottest
def env_exists(env):
"""
check if environment exists
"""
if 'WORKON_HOME' not in os.environ:
return False
if os.path.exists(os.path.join(os.environ['WORKON_HOME'], env)):
return True
else:
return False
71 changes: 71 additions & 0 deletions IPython/extensions/virtualenvmagic.py
@@ -0,0 +1,71 @@
# -*- coding: utf-8 -*-
"""
Virtualenv magic
Requires virtualenv and virtualenv Wrapper
Author: Flávio Codeço Coelho - @fccoelho
Thanks to @turicas for helpful tips
"""
import sys
import os
import shlex
from subprocess import Popen, PIPE
from IPython.core.magic import cell_magic


@cell_magic('virtualenv')
def virtualenv(line, cell):
"""
This magic enables the execution of code in a cell on a
pre-existing virtual env. It Requires Virtualenv and VirtualenvWrapper
to be installed in the system.
The virtual env to be used must be created in advance.

Usage
=====
To activate this magic just write at the top of the cell:

%%virtualenv my_env
import sys
print sys.version


"""
if not os.path.exists(os.path.join(os.environ['WORKON_HOME'], line)):
Copy link
Member

Choose a reason for hiding this comment

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

This can be safely used if workon_home is undefined, because it is normal to specify the absolute path of an env.

Something like:

def get_workon_home():
    if 'WORKON_HOME' in os.environ:
        return os.environ['WORKON_HOME']
    default = os.path.join(os.environ['HOME'], '.virtualenvs')
    if os.path.exists(default):
        return default
    # fallback on empty, absolute or cwd-relative paths must be used:
    return ''

print >> sys.stderr, "Environment {0} does not exist.".format(line)
return
cmd = get_activate_cmd(line)
p = Popen(cmd, stdout=PIPE, stderr=PIPE, stdin=PIPE)
Copy link
Member

Choose a reason for hiding this comment

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

Specify shell=True here, and we shouldn't need bash -c, I think.

Copy link
Author

Choose a reason for hiding this comment

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

shell= False is safer, according to the subprocess documentation, moreover, this line is not likely to stay as is, if I find a way to make this work on windows. So I am leaving it as is for now.

Copy link
Member

Choose a reason for hiding this comment

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

You're effectively recreating shell=True by using bash -c. But the vulnerability the docs mention is about untrusted input injecting shell code - which we don't need to worry about, because the notebook exists to let the user run arbitrary code.

Copy link
Author

Choose a reason for hiding this comment

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

I think you are right, but I am going to have to remove the dependency on
bash anyway, to make it work on windows.

cheers,

Flávio

On Sat, Jun 9, 2012 at 10:35 AM, Thomas Kluyver <
reply@reply.github.com

wrote:

  • =====
  • To activate this magic just write at the top of the cell:
  •    %%virtualenv my_env
    
  •    import sys
    
  •    print sys.version
    
  • """
  • if not os.path.exists(os.environ['WORKON_HOME'] + '/' + line):
  •    print >> sys.stderr, "Environment {} does not
    
    exist.".format(line)
  •    return
    
  • env_activate_cmd = 'bash -c "source {}/{}/bin/activate && python
    -"'\
  • .format(os.environ['WORKON_HOME'], line)
  • cmd = shlex.split(env_activate_cmd)
  • p = Popen(cmd, stdout=PIPE, stderr=PIPE, stdin=PIPE)

You're effectively recreating shell=True by using bash -c. But the
vulnerability the docs mention is about untrusted input injecting shell
code - which we don't need to worry about, because the notebook exists to
let the user run arbitrary code.


Reply to this email directly or view it on GitHub:
https://github.com/ipython/ipython/pull/1891/files#r955665

Flávio Codeço Coelho

+55(21) 3799-5567
Professor
Escola de Matemática Aplicada
Fundação Getúlio Vargas
Rio de Janeiro - RJ
Brasil

Copy link
Author

Choose a reason for hiding this comment

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

Thomas,

I have made some new commits which introduce the beginnings of windows
support. I decided to check sys.platform and handle env activation
differently. Haven't tested on windows, since I don't have a windows box
handy, but the Linux part is still functional and passes all existing tests.

As per your suggestion I have also added the programmatic creation of a
test env just for running the tests which gets deleted on teardown.

I'll appreciate any comments as usual.

Flávio

On Sat, Jun 9, 2012 at 12:28 PM, Flavio Coelho fccoelho@gmail.com wrote:

I think you are right, but I am going to have to remove the dependency on
bash anyway, to make it work on windows.

cheers,

Flávio

On Sat, Jun 9, 2012 at 10:35 AM, Thomas Kluyver <
reply@reply.github.com

wrote:

  • =====
  • To activate this magic just write at the top of the cell:
  •    %%virtualenv my_env
    
  •    import sys
    
  •    print sys.version
    
  • """
  • if not os.path.exists(os.environ['WORKON_HOME'] + '/' + line):
  •    print >> sys.stderr, "Environment {} does not
    
    exist.".format(line)
  •    return
    
  • env_activate_cmd = 'bash -c "source {}/{}/bin/activate && python
    -"'\
  • .format(os.environ['WORKON_HOME'], line)
  • cmd = shlex.split(env_activate_cmd)
  • p = Popen(cmd, stdout=PIPE, stderr=PIPE, stdin=PIPE)

You're effectively recreating shell=True by using bash -c. But the
vulnerability the docs mention is about untrusted input injecting shell
code - which we don't need to worry about, because the notebook exists to
let the user run arbitrary code.


Reply to this email directly or view it on GitHub:
https://github.com/ipython/ipython/pull/1891/files#r955665

Flávio Codeço Coelho

+55(21) 3799-5567
Professor
Escola de Matemática Aplicada
Fundação Getúlio Vargas
Rio de Janeiro - RJ
Brasil

Flávio Codeço Coelho

+55(21) 3799-5567
Professor
Escola de Matemática Aplicada
Fundação Getúlio Vargas
Rio de Janeiro - RJ
Brasil

out, err = p.communicate(cell)
if err:
print >> sys.stderr, err
return out

_loaded = False

def get_activate_cmd(line):
"""
Returns the correct activate command given the platform
"""
if sys.platform.startswith("win"):
env_activate_cmd = os.path.join(os.environ['WORKON_HOME'],line,
'Scripts','activate') + "; python -"
else:
env_activate_cmd = 'bash -c "source {0}/{1}/bin/activate && python -"'\
.format(os.environ['WORKON_HOME'], line)

cmd = shlex.split(env_activate_cmd)
return cmd


def load_ipython_extension(ip):
"""
Load the extension in IPython.
"""
global _loaded
if not _loaded:
if 'WORKON_HOME' in os.environ:
Copy link
Member

Choose a reason for hiding this comment

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

This is not an accurate test of whether virtualenvwrapper is installed. The WORKON_HOME env need not be defined (and in fact is not under default circumstances), and its default value is $HOME/.virtualenvs, not $HOME/Env as your tests would suggest.

ip.register_magic_function(virtualenv, 'cell')
_loaded = True
else:
print >> sys.stderr, "You must have Virtualenv and \
VirtualenvWrapper installed."