[PATCH 2/2] lib/bb: Further shell=True cleanups

Go through the remaining shell=True references in bitbake and clean them up
to use lists where possible. By using existing helpers, chdir and PATH
settings can be dropped.

In toaster, the current branch can be obtained more easily with modern git
commands.

Signed-off-by: Richard Purdie <[email protected]>
---
 lib/bb/fetch2/crate.py                        | 21 ++++---------------
 lib/bb/fetch2/gomod.py                        |  7 ++-----
 lib/bb/ui/ncurses.py                          |  2 +-
 .../bldcontrol/localhostbecontroller.py       | 21 +++++++++++--------
 lib/toaster/toastermain/settings.py           |  4 ++--
 5 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/lib/bb/fetch2/crate.py b/lib/bb/fetch2/crate.py
index 8f928ea6b4d..f94e153ee3b 100644
--- a/lib/bb/fetch2/crate.py
+++ b/lib/bb/fetch2/crate.py
@@ -17,7 +17,7 @@ import subprocess
 import re
 from functools import cmp_to_key
 import bb
-from   bb.fetch2 import logger, subprocess_setup, UnpackError
+from   bb.fetch2 import logger, subprocess_setup, UnpackError, runfetchcmd
 from   bb.fetch2.wget import Wget
 
 
@@ -110,19 +110,15 @@ class Crate(Wget):
         # possible metadata we need to write out
         metadata = {}
 
-        # change to the rootdir to unpack but save the old working dir
-        save_cwd = os.getcwd()
-        os.chdir(rootdir)
-
         bp = d.getVar('BP')
         if bp == ud.parm.get('name'):
-            cmd = "tar -xz --no-same-owner -f %s" % thefile
+            cmd = ['tar', '-xz', '--no-same-owner', '-f', thefile]
             ud.unpack_tracer.unpack("crate-extract", rootdir)
         else:
             cargo_bitbake = self._cargo_bitbake_path(rootdir)
             ud.unpack_tracer.unpack("cargo-extract", cargo_bitbake)
 
-            cmd = "tar -xz --no-same-owner -f %s -C %s" % (thefile, cargo_bitbake)
+            cmd = ['tar', '-xz', '--no-same-owner', '-f', thefile, '-C', cargo_bitbake]
 
             # ensure we've got these paths made
             bb.utils.mkdirhier(cargo_bitbake)
@@ -135,17 +131,8 @@ class Crate(Wget):
             metadata['files'] = {}
             metadata['package'] = tarhash
 
-        path = d.getVar('PATH')
-        if path:
-            cmd = "PATH=\"%s\" %s" % (path, cmd)
         bb.note("Unpacking %s to %s/" % (thefile, os.getcwd()))
-
-        ret = subprocess.call(cmd, preexec_fn=subprocess_setup, shell=True)
-
-        os.chdir(save_cwd)
-
-        if ret != 0:
-            raise UnpackError("Unpack command %s failed with return value %s" % (cmd, ret), ud.url)
+        runfetchcmd(cmd, d, workdir=rootdir)
 
         # if we have metadata to write out..
         if len(metadata) > 0:
diff --git a/lib/bb/fetch2/gomod.py b/lib/bb/fetch2/gomod.py
index 8a64f644dfd..5cdf8f99814 100644
--- a/lib/bb/fetch2/gomod.py
+++ b/lib/bb/fetch2/gomod.py
@@ -135,13 +135,10 @@ class GoMod(Wget):
         unpackdir = os.path.dirname(unpackpath)
         bb.utils.mkdirhier(unpackdir)
         ud.unpack_tracer.unpack("file-copy", unpackdir)
-        cmd = f"cp {ud.localpath} {unpackpath}"
-        path = d.getVar('PATH')
-        if path:
-            cmd = f"PATH={path} {cmd}"
+        cmd = ['cp', ud.localpath, unpackpath]
         name = os.path.basename(unpackpath)
         bb.note(f"Unpacking {name} to {unpackdir}/")
-        subprocess.check_call(cmd, shell=True, preexec_fn=subprocess_setup)
+        runfetchcmd(cmd, d)
 
         if name.endswith('.zip'):
             # Unpack the go.mod file from the zip file
diff --git a/lib/bb/ui/ncurses.py b/lib/bb/ui/ncurses.py
index 18a706547aa..228fa9a6dc7 100644
--- a/lib/bb/ui/ncurses.py
+++ b/lib/bb/ui/ncurses.py
@@ -291,7 +291,7 @@ class NCursesUI:
 #                            bb.error("log data follows (%s)" % logfile)
 #                            number_of_lines = data.getVar("BBINCLUDELOGS_LINES", d)
 #                            if number_of_lines:
-#                                subprocess.check_call('tail -n%s %s' % (number_of_lines, logfile), shell=True)
+#                                subprocess.check_call(['tail', '-n' + number_of_lines, logfile])
 #                            else:
 #                                f = open(logfile, "r")
 #                                while True:
diff --git a/lib/toaster/bldcontrol/localhostbecontroller.py b/lib/toaster/bldcontrol/localhostbecontroller.py
index 50069d47a4c..dcbd9a8260d 100644
--- a/lib/toaster/bldcontrol/localhostbecontroller.py
+++ b/lib/toaster/bldcontrol/localhostbecontroller.py
@@ -51,7 +51,10 @@ class LocalhostBEController(BuildEnvironmentController):
             env=os.environ.copy()
 
         logger.debug("lbc_shellcmd: (%s) %s" % (cwd, command))
-        p = subprocess.Popen(command, cwd = cwd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
+        if isinstance(command, str):
+            p = subprocess.Popen(command, cwd = cwd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
+        else:
+            p = subprocess.Popen(command, cwd = cwd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
         if nowait:
             return
         (out,err) = p.communicate()
@@ -136,7 +139,7 @@ class LocalhostBEController(BuildEnvironmentController):
         cached_layers = {}
 
         try:
-            for remotes in self._shellcmd("git remote -v", self.be.sourcedir,env=git_env).split("\n"):
+            for remotes in self._shellcmd(['git', 'remote', '-v'], self.be.sourcedir, env=git_env).split("\n"):
                 try:
                     remote = remotes.split("\t")[1].split(" ")[0]
                     if remote not in cached_layers:
@@ -172,8 +175,7 @@ class LocalhostBEController(BuildEnvironmentController):
             # see if our directory is a git repository
             if os.path.exists(localdirname):
                 try:
-                    localremotes = self._shellcmd("git remote -v",
-                                                  localdirname,env=git_env)
+                    localremotes = self._shellcmd(['git' ,'remote', '-v'], localdirname, env=git_env)
                     # NOTE: this nice-to-have check breaks when using git remaping to get past firewall
                     #       Re-enable later with .gitconfig remapping checks
                     #if not giturl in localremotes and commit != 'HEAD':
@@ -186,12 +188,12 @@ class LocalhostBEController(BuildEnvironmentController):
             else:
                 if giturl in cached_layers:
                     logger.debug("localhostbecontroller git-copying %s to %s" % (cached_layers[giturl], localdirname))
-                    self._shellcmd("git clone \"%s\" \"%s\"" % (cached_layers[giturl], localdirname),env=git_env)
-                    self._shellcmd("git remote remove origin", localdirname,env=git_env)
-                    self._shellcmd("git remote add origin \"%s\"" % giturl, localdirname,env=git_env)
+                    self._shellcmd(['git', 'clone', cached_layers[giturl]], localdirname], env=git_env)
+                    self._shellcmd(['git', 'remote', 'remove', 'origin'], localdirname, env=git_env)
+                    self._shellcmd(['git', 'remote', 'add', 'origin', giturl], localdirname, env=git_env)
                 else:
                     logger.debug("localhostbecontroller: cloning %s in %s" % (giturl, localdirname))
-                    self._shellcmd('git clone "%s" "%s"' % (giturl, localdirname),env=git_env)
+                    self._shellcmd(['git', 'clone', giturl, localdirname], env=git_env)
 
             # branch magic name "HEAD" will inhibit checkout
             if commit != "HEAD":
@@ -199,7 +201,8 @@ class LocalhostBEController(BuildEnvironmentController):
                 ref = commit if re.match('^[a-fA-F0-9]+$', commit) else 'origin/%s' % commit
                 # DEBUGGING NOTE: this is the 'git fetch" to disable after the initial clone to
                 # prevent inserted debugging commands from being lost
-                self._shellcmd('git fetch && git reset --hard "%s"' % ref, localdirname,env=git_env)
+                self._shellcmd(['git', 'fetch'], localdirname, env=git_env)
+                self._shellcmd(['git', 'reset', '--hard', ref], localdirname, env=git_env)
 
             # verify our repositories
             for name, dirpath, index in gitrepos[(giturl, commit)]:
diff --git a/lib/toaster/toastermain/settings.py b/lib/toaster/toastermain/settings.py
index d2a449627f8..9d2dbc464ec 100644
--- a/lib/toaster/toastermain/settings.py
+++ b/lib/toaster/toastermain/settings.py
@@ -229,8 +229,8 @@ from os.path import dirname as DN
 SITE_ROOT=DN(DN(os.path.abspath(__file__)))
 
 import subprocess
-TOASTER_BRANCH = subprocess.Popen('git branch | grep "^* " | tr -d "* "', cwd = SITE_ROOT, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate()[0]
-TOASTER_REVISION = subprocess.Popen('git rev-parse HEAD ', cwd = SITE_ROOT, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate()[0]
+TOASTER_BRANCH = subprocess.check_output(['git', 'branch', '--show-current'], cwd=SITE_ROOT, text=True).strip()
+TOASTER_REVISION = subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=SITE_ROOT, text=True).strip()
 
 ROOT_URLCONF = 'toastermain.urls'