Merge pull request #989 from nsoranzo/shell_without_shell_True

Reduce use of `shell=True` in subprocesses

Author: nsoranzo

planemo/commands/cmd_travis_before_install.py

@@ -21,7 +21,7 @@
 def cli(ctx):
     """Internal command for GitHub/TravisCI testing.
 
-    This command is used internally by planemo to assist in contineous testing
+    This command is used internally by planemo to assist in continuous testing
     of tools with Travis CI (https://travis-ci.org/).
     """
     build_dir = os.environ.get("TRAVIS_BUILD_DIR", None)
@@ -58,7 +58,6 @@ def cli(ctx):
     shell(['sudo', 'dpkg', '-i', SAMTOOLS_DEB])
     setup_file = os.path.join(build_travis_dir, SETUP_FILE_NAME)
     if os.path.exists(setup_file):
-        shell(
-            ". %s && bash -x %s" % (build_env_path, setup_file),
-            env=template_vars
-        )
+        env = template_vars
+        env['PATH'] = build_bin_dir
+        shell(['bash', '-x', setup_file], env=env)

planemo/galaxy/config.py

@@ -29,6 +29,7 @@
     kill_pid_file,
     shell,
     shell_join,
+    untar_to,
     wait_on,
     warn,
     write_file,
@@ -185,11 +186,6 @@ class = StreamHandler
 DEFAULT_GALAXY_SOURCE = "https://github.com/galaxyproject/galaxy"
 CWL_GALAXY_SOURCE = "https://github.com/common-workflow-language/galaxy"
 
-# TODO: Mac-y curl variant of this.
-DOWNLOAD_GALAXY = (
-    "wget -q https://codeload.github.com/galaxyproject/galaxy/tar.gz/"
-)
-
 DATABASE_LOCATION_TEMPLATE = "sqlite:///%s?isolation_level=IMMEDIATE"
 
 COMMAND_STARTUP_COMMAND = "./scripts/common_startup.sh ${COMMON_STARTUP_ARGS}"
@@ -1082,18 +1078,17 @@ def _install_galaxy(ctx, galaxy_root, env, kwds):
 
 
 def _install_galaxy_via_download(ctx, galaxy_root, env, kwds):
-    tmpdir = mkdtemp()
     branch = _galaxy_branch(kwds)
-    command = DOWNLOAD_GALAXY + "%s -O - | tar -C '%s' -xvz | tail && mv '%s' '%s'" % \
-        (branch, tmpdir, os.path.join(tmpdir, 'galaxy-' + branch), galaxy_root)
-    _install_with_command(ctx, command, galaxy_root, env, kwds)
+    untar_to("https://codeload.github.com/galaxyproject/galaxy/tar.gz/" + branch, tar_args=['-xvzf', '-', 'galaxy-' + branch], dest_dir=galaxy_root)
+    _install_with_command(ctx, galaxy_root, env, kwds)
 
 
 def _install_galaxy_via_git(ctx, galaxy_root, env, kwds):
     gx_repo = _ensure_galaxy_repository_available(ctx, kwds)
     branch = _galaxy_branch(kwds)
     command = git.command_clone(ctx, gx_repo, galaxy_root, branch=branch)
-    _install_with_command(ctx, command, galaxy_root, env, kwds)
+    shell(command, env=env)
+    _install_with_command(ctx, galaxy_root, env, kwds)
 
 
 def _build_eggs_cache(ctx, env, kwds):
@@ -1128,17 +1123,15 @@ def _galaxy_source(kwds):
     return source
 
 
-def _install_with_command(ctx, command, galaxy_root, env, kwds):
+def _install_with_command(ctx, galaxy_root, env, kwds):
     setup_venv_command = setup_venv(ctx, kwds)
     env['__PYVENV_LAUNCHER__'] = ''
     install_cmd = shell_join(
-        command,
-        ['cd', galaxy_root],
         setup_venv_command,
         setup_common_startup_args(),
         COMMAND_STARTUP_COMMAND,
     )
-    shell(install_cmd, env=env)
+    shell(install_cmd, cwd=galaxy_root, env=env)
 
 
 def _ensure_galaxy_repository_available(ctx, kwds):

planemo/git.py

@@ -109,9 +109,7 @@ def rev(ctx, directory):
 
 def is_rev_dirty(ctx, directory):
     """Check if specified git repository has uncommitted changes."""
-    # TODO: Use ENV instead of cd.
-    cmd = "cd '%s' && git diff --quiet" % directory
-    return io.shell(cmd) != 0
+    return io.shell(['git', 'diff', '--quiet'], cwd=directory) != 0
 
 
 def rev_if_git(ctx, directory):

planemo/io.py

@@ -103,9 +103,9 @@ def untar_to(url, tar_args=None, path=None, dest_dir=None):
         if dest_dir:
             if not os.path.exists(dest_dir):
                 os.makedirs(dest_dir)
-            tar_args.extend(['-C', dest_dir])
+            tar_args[0:0] = ['-C', dest_dir]
         if path:
-            tar_args.append('-O')
+            tar_args.insert(0, '-O')
 
         download_cmd = download_command(url)
         download_p = commands.shell_process(download_cmd, stdout=subprocess.PIPE)

planemo/shed/__init__.py

@@ -415,10 +415,12 @@ def _diff_in(ctx, working, realized_repository, **kwds):
         else:
             xml_diff = diff_and_remove(working, label_a, label_b, sys.stdout)
 
-    cmd = 'cd "%s"; diff -r %s %s' % (working, label_a, label_b)
+    cmd = ['diff', '-r', label_a, label_b]
     if output:
-        cmd += " >> '%s'" % output
-    raw_diff = shell(cmd)
+        with open(output, 'ab') as fh:
+            raw_diff = shell(cmd, cwd=working, stdout=fh)
+    else:
+        raw_diff = shell(cmd, cwd=working)
     exit = raw_diff or xml_diff
     if not raw:
         if xml_diff: