Skip to content

Commit 48e1df7

Browse files
author
Nicole Thomas
committed
Merge pull request saltstack#18776 from jfindlay/quote_virtualenv
unquote venv mod commands
2 parents 8a8d888 + b7467f5 commit 48e1df7

File tree

2 files changed

+67
-50
lines changed

2 files changed

+67
-50
lines changed

‎salt/modules/virtualenv_mod.py

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@
99
import logging
1010
import os
1111
import os.path
12-
try:
13-
from shlex import quote as _cmd_quote # pylint: disable=E0611
14-
except ImportError:
15-
from pipes import quote as _cmd_quote
1612

1713
# Import salt libs
1814
import salt.utils
@@ -126,7 +122,7 @@ def create(path,
126122
elif runas is not None and not user:
127123
user = str(runas)
128124

129-
cmd = [_cmd_quote(venv_bin)]
125+
cmd = [venv_bin]
130126

131127
if 'pyvenv' not in venv_bin:
132128
# ----- Stop the user if pyvenv only options are used --------------->
@@ -154,8 +150,10 @@ def create(path,
154150
)
155151
except ImportError:
156152
# Unable to import?? Let's parse the version from the console
157-
version_cmd = '{0} --version'.format(_cmd_quote(venv_bin))
158-
ret = __salt__['cmd.run_all'](version_cmd, runas=user)
153+
version_cmd = '{0} --version'.format(venv_bin)
154+
ret = __salt__['cmd.run_all'](
155+
version_cmd, runas=user, python_shell=False
156+
)
159157
if ret['retcode'] > 0 or not ret['stdout'].strip():
160158
raise salt.exceptions.CommandExecutionError(
161159
'Unable to get the virtualenv version output using {0!r}. '
@@ -183,15 +181,15 @@ def create(path,
183181
'Requested python ({0}) does not appear '
184182
'executable.'.format(python)
185183
)
186-
cmd.append('--python={0}'.format(_cmd_quote(python)))
184+
cmd.append('--python={0}'.format(python))
187185
if extra_search_dir is not None:
188186
if isinstance(extra_search_dir, string_types) and \
189187
extra_search_dir.strip() != '':
190188
extra_search_dir = [
191189
e.strip() for e in extra_search_dir.split(',')
192190
]
193191
for entry in extra_search_dir:
194-
cmd.append('--extra-search-dir={0}'.format(_cmd_quote(entry)))
192+
cmd.append('--extra-search-dir={0}'.format(entry))
195193
if never_download is True:
196194
if virtualenv_version_info >= (1, 10):
197195
log.info(
@@ -203,7 +201,7 @@ def create(path,
203201
else:
204202
cmd.append('--never-download')
205203
if prompt is not None and prompt.strip() != '':
206-
cmd.append('--prompt={0!r}'.format(_cmd_quote(prompt)))
204+
cmd.append('--prompt={0!r}'.format(prompt))
207205
else:
208206
# venv module from the Python >= 3.3 standard library
209207

@@ -244,10 +242,10 @@ def create(path,
244242
cmd.append('--system-site-packages')
245243

246244
# Finally the virtualenv path
247-
cmd.append(_cmd_quote(path))
245+
cmd.append(path)
248246

249247
# Let's create the virtualenv
250-
ret = __salt__['cmd.run_all'](' '.join(cmd), runas=user)
248+
ret = __salt__['cmd.run_all'](cmd, runas=user, python_shell=False)
251249
if ret['retcode'] > 0:
252250
# Something went wrong. Let's bail out now!
253251
return ret
@@ -310,7 +308,7 @@ def get_site_packages(venv):
310308
raise salt.exceptions.CommandExecutionError(
311309
"Path does not appear to be a virtualenv: '{0}'".format(bin_path))
312310

313-
return __salt__['cmd.exec_code'](_cmd_quote(bin_path),
311+
return __salt__['cmd.exec_code'](bin_path,
314312
'from distutils import sysconfig; print sysconfig.get_python_lib()')
315313

316314

@@ -327,10 +325,11 @@ def _install_script(source, cwd, python, user, saltenv='base'):
327325
os.chown(tmppath, __salt__['file.user_to_uid'](user), -1)
328326
try:
329327
return __salt__['cmd.run_all'](
330-
'{0} {1}'.format(_cmd_quote(python), _cmd_quote(tmppath)),
328+
'{0} {1}'.format(python, tmppath),
331329
runas=user,
332330
cwd=cwd,
333-
env={'VIRTUAL_ENV': cwd}
331+
env={'VIRTUAL_ENV': cwd},
332+
python_shell=False
334333
)
335334
finally:
336335
os.remove(tmppath)

‎tests/unit/modules/virtualenv_test.py

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,9 @@ def test_issue_6029_deprecated_distribute(self):
5050
'/tmp/foo', system_site_packages=True, distribute=True
5151
)
5252
mock.assert_called_once_with(
53-
'virtualenv --distribute --system-site-packages /tmp/foo',
54-
runas=None
53+
['virtualenv', '--distribute', '--system-site-packages', '/tmp/foo'],
54+
runas=None,
55+
python_shell=False
5556
)
5657

5758
with TestsLoggingHandler() as handler:
@@ -66,8 +67,9 @@ def test_issue_6029_deprecated_distribute(self):
6667
'/tmp/foo', system_site_packages=True, distribute=True
6768
)
6869
mock.assert_called_once_with(
69-
'virtualenv --system-site-packages /tmp/foo',
70-
runas=None
70+
['virtualenv', '--system-site-packages', '/tmp/foo'],
71+
runas=None,
72+
python_shell=False
7173
)
7274

7375
# Are we logging the deprecation information?
@@ -87,8 +89,9 @@ def test_issue_6030_deprecated_never_download(self):
8789
'/tmp/foo', never_download=True
8890
)
8991
mock.assert_called_once_with(
90-
'virtualenv --never-download /tmp/foo',
91-
runas=None
92+
['virtualenv', '--never-download', '/tmp/foo'],
93+
runas=None,
94+
python_shell=False
9295
)
9396

9497
with TestsLoggingHandler() as handler:
@@ -102,8 +105,9 @@ def test_issue_6030_deprecated_never_download(self):
102105
virtualenv_mod.create(
103106
'/tmp/foo', never_download=True
104107
)
105-
mock.assert_called_once_with('virtualenv /tmp/foo',
106-
runas=None)
108+
mock.assert_called_once_with(['virtualenv', '/tmp/foo'],
109+
runas=None,
110+
python_shell=False)
107111

108112
# Are we logging the deprecation information?
109113
self.assertIn(
@@ -128,12 +132,13 @@ def test_issue_6031_multiple_extra_search_dirs(self):
128132
'/tmp/foo', extra_search_dir=extra_search_dirs
129133
)
130134
mock.assert_called_once_with(
131-
'virtualenv '
132-
'--extra-search-dir=/tmp/bar-1 '
133-
'--extra-search-dir=/tmp/bar-2 '
134-
'--extra-search-dir=/tmp/bar-3 '
135-
'/tmp/foo',
136-
runas=None
135+
['virtualenv',
136+
'--extra-search-dir=/tmp/bar-1',
137+
'--extra-search-dir=/tmp/bar-2',
138+
'--extra-search-dir=/tmp/bar-3',
139+
'/tmp/foo'],
140+
runas=None,
141+
python_shell=False
137142
)
138143

139144
# Passing extra_search_dirs as comma separated list
@@ -143,12 +148,13 @@ def test_issue_6031_multiple_extra_search_dirs(self):
143148
'/tmp/foo', extra_search_dir=','.join(extra_search_dirs)
144149
)
145150
mock.assert_called_once_with(
146-
'virtualenv '
147-
'--extra-search-dir=/tmp/bar-1 '
148-
'--extra-search-dir=/tmp/bar-2 '
149-
'--extra-search-dir=/tmp/bar-3 '
150-
'/tmp/foo',
151-
runas=None
151+
['virtualenv',
152+
'--extra-search-dir=/tmp/bar-1',
153+
'--extra-search-dir=/tmp/bar-2',
154+
'--extra-search-dir=/tmp/bar-3',
155+
'/tmp/foo'],
156+
runas=None,
157+
python_shell=False
152158
)
153159

154160
def test_unapplicable_options(self):
@@ -253,8 +259,9 @@ def test_get_virtualenv_version_from_shell(self):
253259
'/tmp/foo', never_download=True
254260
)
255261
mock.assert_called_with(
256-
'virtualenv --never-download /tmp/foo',
257-
runas=None
262+
['virtualenv', '--never-download', '/tmp/foo'],
263+
runas=None,
264+
python_shell=False
258265
)
259266
# <---- virtualenv binary returns 1.9.1 as its version ----------
260267

@@ -268,8 +275,9 @@ def test_get_virtualenv_version_from_shell(self):
268275
'/tmp/foo', never_download=True
269276
)
270277
mock.assert_called_with(
271-
'virtualenv /tmp/foo',
272-
runas=None
278+
['virtualenv', '/tmp/foo'],
279+
runas=None,
280+
python_shell=False
273281
)
274282
# <---- virtualenv binary returns 1.10rc1 as its version --------
275283

@@ -281,42 +289,48 @@ def test_python_argument(self):
281289
'/tmp/foo', python=sys.executable,
282290
)
283291
mock.assert_called_once_with(
284-
'virtualenv --python={0} /tmp/foo'.format(sys.executable),
285-
runas=None
292+
['virtualenv', '--python={0}'.format(sys.executable), '/tmp/foo'],
293+
runas=None,
294+
python_shell=False
286295
)
287296

288297
def test_prompt_argument(self):
289298
mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
290299
with patch.dict(virtualenv_mod.__salt__, {'cmd.run_all': mock}):
291300
virtualenv_mod.create('/tmp/foo', prompt='PY Prompt')
292301
mock.assert_called_once_with(
293-
'virtualenv --prompt=\'PY Prompt\' /tmp/foo',
294-
runas=None
302+
['virtualenv', '--prompt=\'PY Prompt\'', '/tmp/foo'],
303+
runas=None,
304+
python_shell=False
295305
)
296306

297307
# Now with some quotes on the mix
298308
mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
299309
with patch.dict(virtualenv_mod.__salt__, {'cmd.run_all': mock}):
300310
virtualenv_mod.create('/tmp/foo', prompt='\'PY\' Prompt')
301311
mock.assert_called_once_with(
302-
'virtualenv --prompt="\'PY\' Prompt" /tmp/foo',
303-
runas=None
312+
['virtualenv', '--prompt="\'PY\' Prompt"', '/tmp/foo'],
313+
runas=None,
314+
python_shell=False
304315
)
305316

306317
mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
307318
with patch.dict(virtualenv_mod.__salt__, {'cmd.run_all': mock}):
308319
virtualenv_mod.create('/tmp/foo', prompt='"PY" Prompt')
309320
mock.assert_called_once_with(
310-
'virtualenv --prompt=\'"PY" Prompt\' /tmp/foo',
311-
runas=None
321+
['virtualenv', '--prompt=\'"PY" Prompt\'', '/tmp/foo'],
322+
runas=None,
323+
python_shell=False
312324
)
313325

314326
def test_clear_argument(self):
315327
mock = MagicMock(return_value={'retcode': 0, 'stdout': ''})
316328
with patch.dict(virtualenv_mod.__salt__, {'cmd.run_all': mock}):
317329
virtualenv_mod.create('/tmp/foo', clear=True)
318330
mock.assert_called_once_with(
319-
'virtualenv --clear /tmp/foo', runas=None
331+
['virtualenv', '--clear', '/tmp/foo'],
332+
runas=None,
333+
python_shell=False
320334
)
321335

322336
def test_upgrade_argument(self):
@@ -326,7 +340,9 @@ def test_upgrade_argument(self):
326340
with patch.dict(virtualenv_mod.__salt__, {'cmd.run_all': mock}):
327341
virtualenv_mod.create('/tmp/foo', venv_bin='pyvenv', upgrade=True)
328342
mock.assert_called_once_with(
329-
'pyvenv --upgrade /tmp/foo', runas=None
343+
['pyvenv', '--upgrade', '/tmp/foo'],
344+
runas=None,
345+
python_shell=False
330346
)
331347

332348
def test_symlinks_argument(self):
@@ -336,7 +352,9 @@ def test_symlinks_argument(self):
336352
with patch.dict(virtualenv_mod.__salt__, {'cmd.run_all': mock}):
337353
virtualenv_mod.create('/tmp/foo', venv_bin='pyvenv', symlinks=True)
338354
mock.assert_called_once_with(
339-
'pyvenv --symlinks /tmp/foo', runas=None
355+
['pyvenv', '--symlinks', '/tmp/foo'],
356+
runas=None,
357+
python_shell=False
340358
)
341359

342360

0 commit comments

Comments
 (0)