Skip to content

Commit

Permalink
Merge pull request spotify#311 from cemiarni/fix-venv-options
Browse files Browse the repository at this point in the history
dh_virtualenv/deployment.py: fix virtualenv options
  • Loading branch information
nailor authored Nov 16, 2020
2 parents 544d601 + 02dbaa4 commit f57cd42
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 6 deletions.
30 changes: 28 additions & 2 deletions dh_virtualenv/cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,28 @@ def _check_for_deprecated_options(
setattr(parser.values, '_test_flag_seen', True)


def _set_builtin_venv(
option, opt_str, value, parser, *args, **kwargs
):
if parser.values.setuptools:
raise OptionValueError(
'--setuptools flag is not supported by builtin venv module'
)
else:
parser.values.builtin_venv = True


def _set_setuptools(
option, opt_str, value, parser, *args, **kwargs
):
if parser.values.builtin_venv:
raise OptionValueError(
'--setuptools flag is not supported by builtin venv module'
)
else:
parser.values.setuptools = True


def get_default_parser():
usage = '%prog [options]'
parser = DebhelperOptionParser(usage, version='%prog ' + version)
Expand All @@ -86,7 +108,9 @@ def get_default_parser():
help='Do not act on the specified package(s)')
parser.add_option('-v', '--verbose', action='store_true',
default=False, help='Turn on verbose mode')
parser.add_option('-s', '--setuptools', action='store_true',
parser.add_option('-s', '--setuptools', action='callback',
dest='setuptools',
callback=_set_setuptools,
default=False, help='Use Setuptools instead of Distribute')
parser.add_option('--extra-index-url', action='append', metavar='URL',
help='Extra index URL(s) to pass to pip.',
Expand Down Expand Up @@ -124,7 +148,9 @@ def get_default_parser():
callback=_check_for_deprecated_options)
parser.add_option('--python', metavar='EXECUTABLE',
help='The Python command to use')
parser.add_option('--builtin-venv', action='store_true',
parser.add_option('--builtin-venv', action='callback',
dest='builtin_venv',
callback=_set_builtin_venv,
help='Use the built-in venv module. Only works on '
'Python 3.4 and later.')
parser.add_option('-D', '--sourcedirectory', dest='sourcedirectory',
Expand Down
8 changes: 4 additions & 4 deletions dh_virtualenv/deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,11 @@ def create_virtualenv(self):
if self.python:
virtualenv.extend(('--python', self.python))

if self.setuptools:
virtualenv.append('--setuptools')
if self.setuptools:
virtualenv.append('--setuptools')

if self.verbose:
virtualenv.append('--verbose')
if self.verbose:
virtualenv.append('--verbose')

# Add in any user supplied virtualenv args
if self.extra_virtualenv_arg:
Expand Down
5 changes: 5 additions & 0 deletions doc/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ For a full list, consult the `git history`_ of the project.
.. _`git history`: https://github.com/spotify/dh-virtualenv/commits/master


Unreleased
==========

* Fix --verbose and --setuptools command line argument usage together with --builtin-venv

1.2.2
=====

Expand Down
1 change: 1 addition & 0 deletions doc/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ few command line options:
.. option:: --setuptools

Use setuptools instead of distribute in the virtualenv.
Not supported when using builtin `venv` module with :option:`--builtin-venv`.

.. option:: --setuptools-test

Expand Down
16 changes: 16 additions & 0 deletions test/test_cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,19 @@ def test_that_use_system_packages_option_can_be_true():
parser = cmdline.get_default_parser()
opts, args = parser.parse_args(['--use-system-packages'])
eq_(True, opts.use_system_packages)


def test_builtin_venv_and_setuptools_conflict():
error_message = '--setuptools flag is not supported by builtin venv module'
args_list = [
['--builtin-venv', '--setuptools'],
['--setuptools', '--builtin-venv'],
]

for args in args_list:
f = get_mocked_stderr()
with patch('sys.stderr', f), patch('sys.exit') as sysexit:
parser = cmdline.get_default_parser()
parser.parse_args(args)
ok_(error_message in f.getvalue())
sysexit.assert_called_once_with(2)
14 changes: 14 additions & 0 deletions test/test_deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,20 @@ def test_venv_with_custom_python(callmock):
eq_(['install', LOG_ARG], d.pip_args)


@patch('tempfile.NamedTemporaryFile', FakeTemporaryFile)
@patch('subprocess.check_call')
def test_create_builtin_venv_with_unsupported_options(callmock):
d = Deployment(
'test', python='python_interpreter',
builtin_venv=True, setuptools=True, verbose=True
)
d.create_virtualenv()
eq_(TEST_VENV_PATH, d.package_dir)
callmock.assert_called_with(
['python_interpreter', '-m', 'venv', TEST_VENV_PATH]
)


@patch('tempfile.NamedTemporaryFile', FakeTemporaryFile)
@patch('subprocess.check_call')
def test_install_package(callmock):
Expand Down

0 comments on commit f57cd42

Please sign in to comment.