From ca206d5eb95921f7a77d03af17a6dee8e98660ac Mon Sep 17 00:00:00 2001
From: Julian-o <github@somethinkodd.com>
Date: Wed, 30 Aug 2023 14:16:38 +1000
Subject: [PATCH 1/2] Migrate to use cmd, cmd_expect and file_extract from
 Buildops.

* Remove buildozer's cmd, cmd_expect and file_extract functions, and tests.

* Migrate clients to use buildops.cmd()
  * Note that the implementation is completely different.
    * Particularly, it doesn't depend on fcntl, so it runs cross-platform.
	* It is much more explicit about the parameters it accepts.
	* It no longer accepts `sensible` param; `quiet` has the same effect.
	* It returns a named tuple, rather than a tuple; took advantage of replacing "magic number" indices with meaningful names.
    * The treatment of environment variables has changed.
      * Old system:
        * buildozer.environ contained a list of deltas - overrides to the os.environ variables.
        * buildozer.cmd()'s env parameter was bigger override.
        * buildozer.cmd() would use the env parameter (and only the env parameter), unless it were None, in which case it would grab os.environ and apply buildozer.environ on top.
      * New system:
        * buildozer.environ isn't available to the library function buildops.cmd().
        * So, now the environment must be passed. It becomes the client's task to pass it.
        * Rather than having every client have to merge the os.environ and buildozer.environ together, the definition of buildozer.environ has been changed. It now is not the delta, but the complete set of env vars. (It now defaults to os.environ, rather than [].)
        * It may be the case that many commands do not care about env vars, and the clients would rather omit the parameter. `buildops.cmd()` could be made to default to os.environ. I did not do this, because I can't tell which need special env vars. I wanted to ensure that every call was updated to include the env. Once that migration is done, it *could* be modified to accept a default. Leaving that for future refactors.
	* Most commands being run, at first inspection at least, look like they should work on darwin, linux and win32. Added assert statements where the command is clearly not going to run cross-platform.
	* ios.py contained a complicated pipe command that required a shell. Rather than supporting shells in `cmd`, I ported the client code to Python - which is a better solution for maintenance.

* Migrate clients to use buildops.cmd_expect()
   * This remains platform-dependent, but is only used to automate accepting Android SDK licenses.
   * Making this platform-independent is left for future refactors.
   * Also now requires env param.

* Migrate clients to use buildops.file_extract()
   * The implementation has been substantially rewritten (platform-independent and should be faster because it doesn't normally spawn shells.)
   * It still, rather unexpectedly, will *run* a .bin file if passed. I do not know if that feature is ever used.
      * Running a .bin depends on `cmd()` and also now needs an environ parameter. [This is the reason I had to bundle it with this larger PR.]
      * Running a .bin is not platform independent, but this is likely to never come up in practice - Windows versions of SDKs/NDKs etc don't require .bin files to be executed.
   * Fixed one call from `osx.py` to use extract_file instead of cmd('unzip')

* Last minute: Rolled back changes to extracting zipfiles.
   * Buildops.extract_file now uses the `unzip` shell command (which is not platform dependent).
   * Python's zipfile doesn't support extracting file permissions. I modified extract_file to chmod as appropriate.
   * That wasn't enough. `pythonforandroid.toolchain create` was failing with the config script complaining about an error in `/home/runner/work/buildozer/buildozer/.buildozer/android/platform/build-arm64-v8a_armeabi-v7a/build/other_builds/libffi/armeabi-v7a__ndk_target_21/libffi` and hence `C compiler cannot create executables`, but only on the CI machine; it works fine on my VirtualBox test machine. Still figuring out what might be different. Hints welcome!
   * Changed tests to match. (Needed more from the logger mock, and behaviour when the file is missing is less specific. In practice, this doesn't occur.)

* Trivial changes
	* Corrected the grammar of a few comments.
	* Improved the indenting of some of Buildops code.
---
 buildozer/__init__.py         | 182 ++--------------------------------
 buildozer/buildops.py         |  41 ++++----
 buildozer/target.py           |  16 ++-
 buildozer/targets/android.py  | 138 +++++++++++++++++---------
 buildozer/targets/ios.py      |  74 +++++++++-----
 buildozer/targets/osx.py      |   3 +-
 tests/targets/test_android.py |  51 +++++-----
 tests/targets/test_ios.py     |  19 ++--
 tests/targets/utils.py        |   8 +-
 tests/test_buildops.py        |   9 +-
 tests/test_buildozer.py       |  42 +-------
 11 files changed, 238 insertions(+), 345 deletions(-)

diff --git a/buildozer/__init__.py b/buildozer/__init__.py
index e3cdf49d5..37ac57f43 100644
--- a/buildozer/__init__.py
+++ b/buildozer/__init__.py
@@ -8,32 +8,18 @@
 
 __version__ = '1.5.1.dev0'
 
-import codecs
-from copy import copy
 from fnmatch import fnmatch
 import os
 from os import environ, walk, sep, listdir
 from os.path import join, exists, dirname, realpath, splitext, expanduser
 import re
 from re import search
-import select
-from subprocess import Popen, PIPE, TimeoutExpired
 import sys
-from sys import stdout, stderr, exit
+from sys import exit
 import textwrap
 import warnings
 
-import shlex
-import pexpect
-
-try:
-    import fcntl
-except ImportError:
-    # on windows, no fcntl
-    fcntl = None
-
 import buildozer.buildops as buildops
-from buildozer.exceptions import BuildozerCommandException
 from buildozer.jsonstore import JsonStore
 from buildozer.logger import Logger
 from buildozer.specparser import SpecParser
@@ -47,7 +33,7 @@ class Buildozer:
                      'deploy', 'run', 'serve')
 
     def __init__(self, filename='buildozer.spec', target=None):
-        self.environ = {}
+        self.environ = environ.copy()
         self.specfilename = filename
         self.state = None
         self.build_id = None
@@ -138,133 +124,6 @@ def build(self):
         # flag to prevent multiple build
         self.target._build_done = True
 
-    def cmd(self, command, **kwargs):
-        # prepare the environ, based on the system + our own env
-        env = environ.copy()
-        env.update(self.environ)
-
-        # prepare the process
-        kwargs.setdefault('env', env)
-        kwargs.setdefault('stdout', PIPE)
-        kwargs.setdefault('stderr', PIPE)
-        kwargs.setdefault('close_fds', True)
-        kwargs.setdefault('show_output', self.logger.log_level > 1)
-
-        show_output = kwargs.pop('show_output')
-        get_stdout = kwargs.pop('get_stdout', False)
-        get_stderr = kwargs.pop('get_stderr', False)
-        break_on_error = kwargs.pop('break_on_error', True)
-        sensible = kwargs.pop('sensible', False)
-        run_condition = kwargs.pop('run_condition', None)
-        quiet = kwargs.pop('quiet', False)
-
-        if not quiet:
-            if not sensible:
-                self.logger.debug('Run {0!r}'.format(command))
-            else:
-                if isinstance(command, (list, tuple)):
-                    self.logger.debug('Run {0!r} ...'.format(command[0]))
-                else:
-                    self.logger.debug('Run {0!r} ...'.format(command.split()[0]))
-            self.logger.debug('Cwd {}'.format(kwargs.get('cwd')))
-
-        # open the process
-        if sys.platform == 'win32':
-            kwargs.pop('close_fds', None)
-        process = Popen(command, **kwargs)
-
-        # prepare fds
-        fd_stdout = process.stdout.fileno()
-        fd_stderr = process.stderr.fileno()
-        if fcntl:
-            fcntl.fcntl(
-                fd_stdout, fcntl.F_SETFL,
-                fcntl.fcntl(fd_stdout, fcntl.F_GETFL) | os.O_NONBLOCK)
-            fcntl.fcntl(
-                fd_stderr, fcntl.F_SETFL,
-                fcntl.fcntl(fd_stderr, fcntl.F_GETFL) | os.O_NONBLOCK)
-
-        ret_stdout = [] if get_stdout else None
-        ret_stderr = [] if get_stderr else None
-        while not run_condition or run_condition():
-            try:
-                readx = select.select([fd_stdout, fd_stderr], [], [], 1)[0]
-            except select.error:
-                break
-            if fd_stdout in readx:
-                chunk = process.stdout.read()
-                if not chunk:
-                    break
-                if get_stdout:
-                    ret_stdout.append(chunk)
-                if show_output:
-                    stdout.write(chunk.decode('utf-8', 'replace'))
-            if fd_stderr in readx:
-                chunk = process.stderr.read()
-                if not chunk:
-                    break
-                if get_stderr:
-                    ret_stderr.append(chunk)
-                if show_output:
-                    stderr.write(chunk.decode('utf-8', 'replace'))
-
-            stdout.flush()
-            stderr.flush()
-
-        try:
-            process.communicate(
-                timeout=(1 if run_condition and not run_condition() else None)
-            )
-        except TimeoutExpired:
-            pass
-
-        if process.returncode != 0 and break_on_error:
-            self.logger.error('Command failed: {0}'.format(command))
-            self.logger.log_env(self.logger.ERROR, kwargs['env'])
-            self.logger.error('')
-            self.logger.error('Buildozer failed to execute the last command')
-            if self.logger.log_level <= self.logger.INFO:
-                self.logger.error('If the error is not obvious, please raise the log_level to 2')
-                self.logger.error('and retry the latest command.')
-            else:
-                self.logger.error('The error might be hidden in the log above this error')
-                self.logger.error('Please read the full log, and search for it before')
-                self.logger.error('raising an issue with buildozer itself.')
-            self.logger.error('In case of a bug report, please add a full log with log_level = 2')
-            raise BuildozerCommandException()
-
-        if ret_stdout:
-            ret_stdout = b''.join(ret_stdout)
-        if ret_stderr:
-            ret_stderr = b''.join(ret_stderr)
-
-        return (ret_stdout.decode('utf-8', 'ignore') if ret_stdout else None,
-                ret_stderr.decode('utf-8') if ret_stderr else None,
-                process.returncode)
-
-    def cmd_expect(self, command, **kwargs):
-
-        # prepare the environ, based on the system + our own env
-        env = environ.copy()
-        env.update(self.environ)
-
-        # prepare the process
-        kwargs.setdefault('env', env)
-        kwargs.setdefault('show_output', self.logger.log_level > 1)
-        sensible = kwargs.pop('sensible', False)
-        show_output = kwargs.pop('show_output')
-
-        if show_output:
-            kwargs['logfile'] = codecs.getwriter('utf8')(stdout.buffer)
-
-        if not sensible:
-            self.logger.debug('Run (expect) {0!r}'.format(command))
-        else:
-            self.logger.debug('Run (expect) {0!r} ...'.format(command.split()[0]))
-
-        self.logger.debug('Cwd {}'.format(kwargs.get('cwd')))
-        return pexpect.spawnu(shlex.join(command), **kwargs)
-
     def check_configuration_tokens(self):
         '''Ensure the spec file is 'correct'.
         '''
@@ -400,7 +259,7 @@ def check_application_requirements(self):
     def _install_application_requirement(self, module):
         self._ensure_virtualenv()
         self.logger.debug('Install requirement {} in virtualenv'.format(module))
-        self.cmd(
+        buildops.cmd(
             ["pip", "install", f"--target={self.applibs_dir}", module],
             env=self.env_venv,
             cwd=self.buildozer_dir,
@@ -417,16 +276,19 @@ def _ensure_virtualenv(self):
             return
         self.venv = join(self.buildozer_dir, 'venv')
         if not buildops.file_exists(self.venv):
-            self.cmd(["python3", "-m", "venv", "./venv"],
-                    cwd=self.buildozer_dir)
+            buildops.cmd(
+                ["python3", "-m", "venv", "./venv"],
+                cwd=self.buildozer_dir,
+                env=self.environ)
 
         # read virtualenv output and parse it
-        output = self.cmd(
+        assert sys.platform != "win32", "Can't call bash on Windows"
+        output = buildops.cmd(
             ["bash", "-c", "source venv/bin/activate && env"],
             get_stdout=True,
             cwd=self.buildozer_dir,
-        )
-        self.env_venv = copy(self.environ)
+            env=self.environ)
+        self.env_venv = self.environ.copy()
         for line in output[0].splitlines():
             args = line.split('=', 1)
             if len(args) != 2:
@@ -441,28 +303,6 @@ def _ensure_virtualenv(self):
         self.env_venv['CC'] = '/bin/false'
         self.env_venv['CXX'] = '/bin/false'
 
-    def file_extract(self, archive, cwd=None):
-        if archive.endswith('.tgz') or archive.endswith('.tar.gz'):
-            self.cmd(["tar", "xzf", archive], cwd=cwd)
-            return
-
-        if archive.endswith('.tbz2') or archive.endswith('.tar.bz2'):
-            # XXX same as before
-            self.cmd(["tar", "xjf", archive], cwd=cwd)
-            return
-
-        if archive.endswith('.bin'):
-            # To process the bin files for linux and darwin systems
-            self.cmd(["chmod", "a+x", archive], cwd=cwd)
-            self.cmd([f"./{archive}"], cwd=cwd)
-            return
-
-        if archive.endswith('.zip'):
-            self.cmd(["unzip", "-q", join(cwd, archive)], cwd=cwd)
-            return
-
-        raise Exception('Unhandled extraction for type {0}'.format(archive))
-
     def clean_platform(self):
         self.logger.info('Clean the platform build directory')
         if not exists(self.platform_dir):
diff --git a/buildozer/buildops.py b/buildozer/buildops.py
index caa40ec46..bd05b57e9 100644
--- a/buildozer/buildops.py
+++ b/buildozer/buildops.py
@@ -22,7 +22,6 @@
 import tarfile
 from threading import Thread
 from urllib.request import Request, urlopen
-from zipfile import ZipFile
 
 from buildozer.exceptions import BuildozerCommandException
 from buildozer.logger import Logger
@@ -121,15 +120,24 @@ def file_extract(archive, env, cwd="."):
         str(archive).endswith(extension)
         for extension in (".tgz", ".tar.gz", ".tbz2", ".tar.bz2")
     ):
-        LOGGER.debug("Extracting {0}".format(archive))
+        LOGGER.debug("Extracting {0} to {1}".format(archive, cwd))
         with tarfile.open(path, "r") as compressed_file:
             compressed_file.extractall(cwd)
         return
 
     if path.suffix == ".zip":
-        LOGGER.debug("Extracting {0}".format(archive))
-        with ZipFile(path, "r") as compressed_file:
-            compressed_file.extractall(cwd)
+        LOGGER.debug("Extracting {0} to {1}".format(archive, cwd))
+        assert platform != "win32", "unzip unavailable on Windows"
+        # This should use python's zipfile library, with suitable handling of
+        # Unix permissions.
+        # However, this lead to unexpected config script issues, so sticking to
+        # unzip for now.
+        return_code = cmd(
+            ["unzip", "-q", join(cwd, archive)], cwd=cwd, env=env
+        ).return_code
+        if return_code != 0:
+            raise BuildozerCommandException(
+                "Unzip gave bad return code: {}".format(return_code))
         return
 
     if path.suffix == ".bin":
@@ -263,7 +271,8 @@ def cmd(
     termination due to run_condition returning False will result in an
     error code.
 
-    quiet parameter reduces logging.
+    quiet parameter reduces logging; useful to keep passwords in command lines
+    out of the log.
 
     The env parameter is deliberately not optional, to ensure it is considered
     during the migration to use this library. Once completed, it can return
@@ -321,12 +330,8 @@ def cmd(
     if process.returncode != 0 and break_on_error:
         _command_fail(command, env, process.returncode)
 
-    ret_stdout = (
-        b"".join(ret_stdout).decode("utf-8", "ignore") if ret_stdout else None
-    )
-    ret_stderr = (
-        b"".join(ret_stderr).decode("utf-8", "ignore") if ret_stderr else None
-    )
+    ret_stdout = b"".join(ret_stdout).decode("utf-8", "ignore") if ret_stdout else None
+    ret_stderr = b"".join(ret_stderr).decode("utf-8", "ignore") if ret_stderr else None
 
     return CommandResult(ret_stdout, ret_stderr, process.returncode)
 
@@ -338,17 +343,13 @@ def _command_fail(command, env, returncode):
     LOGGER.error("")
     LOGGER.error("Buildozer failed to execute the last command")
     if LOGGER.log_level <= LOGGER.INFO:
-        LOGGER.error(
-            "If the error is not obvious, please raise the log_level to 2"
-        )
+        LOGGER.error("If the error is not obvious, please raise the log_level to 2")
         LOGGER.error("and retry the latest command.")
     else:
         LOGGER.error("The error might be hidden in the log above this error")
         LOGGER.error("Please read the full log, and search for it before")
         LOGGER.error("raising an issue with buildozer itself.")
-    LOGGER.error(
-        "In case of a bug report, please add a full log with log_level = 2"
-    )
+    LOGGER.error("In case of a bug report, please add a full log with log_level = 2")
     raise BuildozerCommandException()
 
 
@@ -373,9 +374,7 @@ def cmd_expect(command, env, **kwargs):
     LOGGER.debug("Cwd {}".format(kwargs.get("cwd")))
 
     assert platform != "win32", "pexpect.spawn is not available on Windows."
-    return pexpect.spawn(
-        shlex.join(command), env=env, encoding="utf-8", **kwargs
-    )
+    return pexpect.spawn(shlex.join(command), env=env, encoding="utf-8", **kwargs)
 
 
 def _report_download_progress(bytes_read, total_size):
diff --git a/buildozer/target.py b/buildozer/target.py
index 616835b13..75a378363 100644
--- a/buildozer/target.py
+++ b/buildozer/target.py
@@ -249,7 +249,6 @@ def install_or_update_repo(self, repo, **kwargs):
         :Returns:
             fully qualified path to updated git repo
         """
-        cmd = self.buildozer.cmd
         install_dir = join(self.buildozer.platform_dir, repo)
         custom_dir, clone_url, clone_branch = self.path_or_git_url(repo, **kwargs)
         if not buildops.file_exists(install_dir):
@@ -257,11 +256,20 @@ def install_or_update_repo(self, repo, **kwargs):
                 buildops.mkdir(install_dir)
                 buildops.file_copytree(custom_dir, install_dir)
             else:
-                cmd(["git", "clone", "--branch", clone_branch, clone_url], cwd=self.buildozer.platform_dir)
+                buildops.cmd(
+                    ["git", "clone", "--branch", clone_branch, clone_url],
+                    cwd=self.buildozer.platform_dir,
+                    env=self.buildozer.environ)
         elif self.platform_update:
             if custom_dir:
                 buildops.file_copytree(custom_dir, install_dir)
             else:
-                cmd(["git", "clean", "-dxf"], cwd=install_dir)
-                cmd(["git", "pull", "origin", clone_branch], cwd=install_dir)
+                buildops.cmd(
+                    ["git", "clean", "-dxf"],
+                    cwd=install_dir,
+                    env=self.buildozer.environ)
+                buildops.cmd(
+                    ["git", "pull", "origin", clone_branch],
+                    cwd=install_dir,
+                    env=self.buildozer.environ)
         return install_dir
diff --git a/buildozer/targets/android.py b/buildozer/targets/android.py
index 829d463bb..b77cb279e 100644
--- a/buildozer/targets/android.py
+++ b/buildozer/targets/android.py
@@ -132,9 +132,12 @@ def warn_on_deprecated_tokens(self):
                          'but you set value {}').format(section, token, value)
                 self.logger.error(error)
 
-    def _p4a(self, cmd, **kwargs):
+    def _p4a(self, cmd, env, **kwargs):
         kwargs.setdefault('cwd', self.p4a_dir)
-        return self.buildozer.cmd([*self._p4a_cmd, *cmd, *self.extra_p4a_args], **kwargs)
+        return buildops.cmd(
+            [*self._p4a_cmd, *cmd, *self.extra_p4a_args],
+            env=env,
+            **kwargs)
 
     @property
     def p4a_dir(self):
@@ -194,10 +197,11 @@ def _sdkmanager(self, *args, **kwargs):
         command = [self.sdkmanager_path, f"--sdk_root={android_sdk_dir}", *args]
 
         if kwargs.pop('return_child', False):
-            return self.buildozer.cmd_expect(command, **kwargs)
+            return buildops.cmd_expect(
+                command, env=self.buildozer.environ, **kwargs)
         else:
             kwargs['get_stdout'] = kwargs.get('get_stdout', True)
-            return self.buildozer.cmd(command, **kwargs)
+            return buildops.cmd(command, env=self.buildozer.environ, **kwargs)
 
     @property
     def android_ndk_version(self):
@@ -312,7 +316,10 @@ def check_requirements(self):
         buildops.checkbin('Java keytool (keytool)', self.keytool_cmd)
 
     def _p4a_have_aab_support(self):
-        returncode = self._p4a(["aab", "-h"], break_on_error=False)[2]
+        returncode = self._p4a(
+            ["aab", "-h"],
+            break_on_error=False,
+            env=self.buildozer.environ).return_code
         if returncode == 0:
             return True
         else:
@@ -357,8 +364,10 @@ def _install_apache_ant(self):
             url,
             archive,
             cwd=ant_dir)
-        self.buildozer.file_extract(archive,
-                                    cwd=ant_dir)
+        buildops.file_extract(
+            archive,
+            cwd=ant_dir,
+            env=self.buildozer.environ)
         self.logger.info('Apache ANT installation done.')
         return ant_dir
 
@@ -388,8 +397,10 @@ def _install_android_sdk(self):
             cwd=sdk_dir)
 
         self.logger.info('Unpacking Android SDK')
-        self.buildozer.file_extract(archive,
-                                    cwd=sdk_dir)
+        buildops.file_extract(
+            archive,
+            cwd=sdk_dir,
+            env=self.buildozer.environ)
 
         self.logger.info('Android SDK tools base installation done.')
 
@@ -450,8 +461,10 @@ def _install_android_ndk(self):
                                 cwd=self.buildozer.global_platform_dir)
 
         self.logger.info('Unpacking Android NDK')
-        self.buildozer.file_extract(archive,
-                                    cwd=self.buildozer.global_platform_dir)
+        buildops.file_extract(
+            archive,
+            cwd=self.buildozer.global_platform_dir,
+            env=self.buildozer.environ)
         buildops.rename(
             unpacked,
             ndk_dir,
@@ -530,7 +543,7 @@ def _find_latest_package(self, packages, key):
 
     def _install_android_packages(self):
 
-        # if any of theses value change into the buildozer.spec, retry the
+        # if any of these values change into the buildozer.spec, retry the
         # update
         cache_key = 'android:sdk_installation'
         cache_value = [
@@ -600,9 +613,11 @@ def _check_aidl(self, v_build_tools):
         aidl_cmd = join(self.android_sdk_dir, 'build-tools',
                         str(v_build_tools), 'aidl')
         buildops.checkbin('Aidl', aidl_cmd)
-        _, _, returncode = self.buildozer.cmd(aidl_cmd,
-                                              break_on_error=False,
-                                              show_output=False)
+        returncode = buildops.cmd(
+            [aidl_cmd],
+            break_on_error=False,
+            show_output=False,
+            env=self.buildozer.environ).return_code
         if returncode != 1:
             self.logger.error('Aidl cannot be executed')
             if architecture()[0] == '64bit':
@@ -627,7 +642,7 @@ def install_platform(self):
         self._install_android_packages()
 
         # ultimate configuration check.
-        # some of our configuration cannot be check without platform.
+        # some of our configuration cannot be checked without platform.
         self.check_configuration_tokens()
         if not self._p4a_have_aab_support():
             self.logger.error(
@@ -645,7 +660,6 @@ def install_platform(self):
         })
 
     def _install_p4a(self):
-        cmd = self.buildozer.cmd
         p4a_fork = self.buildozer.config.getdefault(
             'app', 'p4a.fork', self.p4a_fork
         )
@@ -672,14 +686,18 @@ def _install_p4a(self):
         else:
             # check that url/branch has not been changed
             if buildops.file_exists(p4a_dir):
-                cur_url = cmd(
+                cur_url = buildops.cmd(
                     ["git", "config", "--get", "remote.origin.url"],
                     get_stdout=True,
                     cwd=p4a_dir,
-                )[0].strip()
-                cur_branch = cmd(
-                    ["git", "branch", "-vv"], get_stdout=True, cwd=p4a_dir
-                )[0].split()[1]
+                    env=self.buildozer.environ
+                ).stdout.strip()
+                cur_branch = buildops.cmd(
+                    ["git", "branch", "-vv"],
+                    get_stdout=True,
+                    cwd=p4a_dir,
+                    env=self.buildozer.environ
+                ).stdout.split()[1]
                 if any([cur_url != p4a_url, cur_branch != p4a_branch]):
                     self.logger.info(
                         f"Detected old url/branch ({cur_url}/{cur_branch}), deleting..."
@@ -687,7 +705,7 @@ def _install_p4a(self):
                     buildops.rmdir(p4a_dir)
 
             if not buildops.file_exists(p4a_dir):
-                cmd(
+                buildops.cmd(
                     [
                         "git",
                         "clone",
@@ -698,19 +716,37 @@ def _install_p4a(self):
                         self.p4a_directory_name,
                     ],
                     cwd=self.buildozer.platform_dir,
+                    env=self.buildozer.environ
                 )
             elif self.platform_update:
-                cmd(["git", "clean", "-dxf"], cwd=p4a_dir)
-                current_branch = cmd(["git", "rev-parse", "--abbrev-ref", "HEAD"],
-                                     get_stdout=True, cwd=p4a_dir)[0].strip()
+                buildops.cmd(
+                    ["git", "clean", "-dxf"],
+                    cwd=p4a_dir,
+                    env=self.buildozer.environ)
+                current_branch = buildops.cmd(
+                    ["git", "rev-parse", "--abbrev-ref", "HEAD"],
+                    get_stdout=True,
+                    cwd=p4a_dir,
+                    env=self.buildozer.environ).stdout.strip()
                 if current_branch == p4a_branch:
-                    cmd(["git", "pull"], cwd=p4a_dir)
+                    buildops.cmd(
+                        ["git", "pull"],
+                        cwd=p4a_dir,
+                        env=self.buildozer.environ)
                 else:
-                    cmd(["git", "fetch", "--tags", "origin", "{0}:{0}".format(p4a_branch)],
-                        cwd=p4a_dir)
-                    cmd(["git", "checkout", p4a_branch], cwd=p4a_dir)
+                    buildops.cmd(
+                        ["git", "fetch", "--tags", "origin", "{0}:{0}".format(p4a_branch)],
+                        cwd=p4a_dir,
+                        env=self.buildozer.environ)
+                    buildops.cmd(
+                        ["git", "checkout", p4a_branch],
+                        cwd=p4a_dir,
+                        env=self.buildozer.environ)
             if p4a_commit != 'HEAD':
-                cmd(["git", "reset", "--hard", p4a_commit], cwd=p4a_dir)
+                buildops.cmd(
+                    ["git", "reset", "--hard", p4a_commit],
+                    cwd=p4a_dir,
+                    env=self.buildozer.environ)
 
         # also install dependencies (currently, only setup.py knows about it)
         # let's extract them.
@@ -728,7 +764,9 @@ def _install_p4a(self):
         options = ["--user"]
         if "VIRTUAL_ENV" in os.environ or "CONDA_PREFIX" in os.environ:
             options = []
-        cmd([executable, "-m", "pip", "install", "-q", *options, *deps])
+        buildops.cmd(
+            [executable, "-m", "pip", "install", "-q", *options, *deps],
+            env=self.buildozer.environ)
 
     def compile_platform(self):
         app_requirements = self.buildozer.config.getlist(
@@ -763,7 +801,7 @@ def compile_platform(self):
 
         p4a_create.extend(options)
 
-        self._p4a(p4a_create, get_stdout=True)[0]
+        self._p4a(p4a_create, get_stdout=True, env=self.buildozer.environ)
 
     def get_available_packages(self):
         return True
@@ -930,7 +968,7 @@ def execute_build_package(self, build_cmd):
             cmd.append('--arch')
             cmd.append(arch)
 
-        self._p4a(cmd)
+        self._p4a(cmd, env=self.buildozer.environ)
 
     def get_release_mode(self):
         # aab, also if unsigned is named as *-release
@@ -968,7 +1006,7 @@ def cmd_run(self, *args):
         for serial in self.serials:
             self.buildozer.environ['ANDROID_SERIAL'] = serial
             self.logger.info('Run on {}'.format(serial))
-            self.buildozer.cmd(
+            buildops.cmd(
                 [
                     self.adb_executable,
                     *self.adb_args,
@@ -981,6 +1019,7 @@ def cmd_run(self, *args):
                     entrypoint,
                 ],
                 cwd=self.buildozer.global_platform_dir,
+                env=self.buildozer.environ
             )
         self.buildozer.environ.pop('ANDROID_SERIAL', None)
 
@@ -1006,14 +1045,14 @@ def cmd_p4a(self, *args):
                   .format(self.targetname))
             sys.stderr.write('PYTHONPATH={} {}\n'.format(self.p4a_dir, self._p4a_cmd))
         else:
-            self._p4a(args)
+            self._p4a(args, env=self.buildozer.environ)
 
     def cmd_clean(self, *args):
         '''
         Clean the build and distribution
         '''
-        self._p4a(["clean_builds"])
-        self._p4a(["clean_dists"])
+        self._p4a(["clean_builds"], env=self.buildozer.environ)
+        self._p4a(["clean_dists"], env=self.buildozer.environ)
 
     def _get_package(self):
         config = self.buildozer.config
@@ -1360,9 +1399,11 @@ def serials(self):
         serial = environ.get('ANDROID_SERIAL')
         if serial:
             return serial.split(',')
-        lines = self.buildozer.cmd(
-            [self.adb_executable, *self.adb_args, "devices"], get_stdout=True
-        )[0].splitlines()
+        lines = buildops.cmd(
+            [self.adb_executable, *self.adb_args, "devices"],
+            get_stdout=True,
+            env=self.buildozer.environ
+        ).stdout.splitlines()
         serials = []
         for serial in lines:
             if not serial:
@@ -1388,7 +1429,9 @@ def cmd_adb(self, *args):
                   .format(self.targetname))
             sys.stderr.write(self.adb_executable + '\n')
         else:
-            self.buildozer.cmd([self.adb_executable, *self.adb_args, *args])
+            buildops.cmd(
+                [self.adb_executable, *self.adb_args, *args],
+                env=self.buildozer.environ)
 
     def cmd_deploy(self, *args):
         super().cmd_deploy(*args)
@@ -1411,16 +1454,17 @@ def cmd_deploy(self, *args):
         for serial in self.serials:
             self.buildozer.environ['ANDROID_SERIAL'] = serial
             self.logger.info('Deploy on {}'.format(serial))
-            self.buildozer.cmd(
+            buildops.cmd(
                 [self.adb_executable, *self.adb_args, "install", "-r", full_apk],
                 cwd=self.buildozer.global_platform_dir,
+                env=self.buildozer.environ
             )
         self.buildozer.environ.pop('ANDROID_SERIAL', None)
 
         self.logger.info('Application pushed.')
 
     def _get_pid(self):
-        pid, *_ = self.buildozer.cmd(
+        pid = buildops.cmd(
             [
                 self.adb_executable,
                 *self.adb_args,
@@ -1432,7 +1476,8 @@ def _get_pid(self):
             show_output=False,
             break_on_error=False,
             quiet=True,
-        )
+            env=self.buildozer.environ
+        ).stdout
         if pid:
             return pid.strip()
         return False
@@ -1459,12 +1504,13 @@ def cmd_logcat(self, *args):
             if pid:
                 extra_args.extend(('--pid', pid))
 
-        self.buildozer.cmd(
+        buildops.cmd(
             [self.adb_executable, *self.adb_args, "logcat", filters, *extra_args],
             cwd=self.buildozer.global_platform_dir,
             show_output=True,
             run_condition=self._get_pid if pid else None,
             break_on_error=False,
+            env=self.buildozer.environ
         )
 
         self.logger.info(f"{self._get_package()} terminated")
diff --git a/buildozer/targets/ios.py b/buildozer/targets/ios.py
index ca5f2dc3b..ee213dcbf 100644
--- a/buildozer/targets/ios.py
+++ b/buildozer/targets/ios.py
@@ -75,7 +75,6 @@ def __init__(self, buildozer):
     def check_requirements(self):
         if sys.platform != "darwin":
             raise NotImplementedError("Only macOS is supported for iOS target")
-        cmd = self.buildozer.cmd
 
         buildops.checkbin('Xcode xcodebuild', 'xcodebuild')
         buildops.checkbin('Xcode xcode-select', 'xcode-select')
@@ -87,9 +86,20 @@ def check_requirements(self):
         buildops.checkbin('libtool', 'libtool')
 
         self.logger.debug('Check availability of a iPhone SDK')
-        sdk = cmd('xcodebuild -showsdks | fgrep "iphoneos" |'
-                'tail -n 1 | awk \'{print $2}\'',
-                get_stdout=True, shell=True)[0]
+        sdk_list = buildops.cmd(
+            ['xcodebuild', '-showsdks'],
+            get_stdout=True,
+            env=self.buildozer.environ).stdout
+        iphoneos_lines = [
+            line
+            for line in sdk_list.split("\n")
+            if "iphoneos" in line]
+        if not iphoneos_lines:
+            sdk = None
+        else:
+            last_iphoneos_line = iphoneos_lines[-1]
+            sdk = last_iphoneos_line.split()[1]  # Second column
+
         if not sdk:
             raise Exception(
                 'No iPhone SDK found. Please install at least one iOS SDK.')
@@ -97,7 +107,10 @@ def check_requirements(self):
             self.logger.debug(' -> found %r' % sdk)
 
         self.logger.debug('Check Xcode path')
-        xcode = cmd(["xcode-select", "-print-path"], get_stdout=True)[0]
+        xcode = buildops.cmd(
+            ["xcode-select", "-print-path"],
+            get_stdout=True,
+            env=self.buildozer.environ).stdout
         if not xcode:
             raise Exception('Unable to get xcode path')
         self.logger.debug(' -> found {0}'.format(xcode))
@@ -115,11 +128,17 @@ def install_platform(self):
 
     def toolchain(self, cmd, **kwargs):
         kwargs.setdefault('cwd', self.ios_dir)
-        return self.buildozer.cmd([*self._toolchain_cmd, *cmd], **kwargs)
+        return buildops.cmd(
+            [*self._toolchain_cmd, *cmd],
+            env=self.buildozer.environ,
+            **kwargs)
 
     def xcodebuild(self, *args, **kwargs):
         filtered_args = [arg for arg in args if arg is not None]
-        return self.buildozer.cmd([*self._xcodebuild_cmd, *filtered_args], **kwargs)
+        return buildops.cmd(
+            [*self._xcodebuild_cmd, *filtered_args],
+            env=self.buildozer.environ,
+            **kwargs)
 
     @property
     def code_signing_allowed(self):
@@ -333,9 +352,11 @@ def cmd_xcode(self, *args):
             'package.name'))
         app_name = app_name.lower()
 
-        ios_dir = ios_dir = join(self.buildozer.platform_dir, 'kivy-ios')
-        self.buildozer.cmd(
-            ["open", f"{app_name}.xcodeproj"], cwd=join(ios_dir, f"{app_name}-ios")
+        ios_dir = join(self.buildozer.platform_dir, 'kivy-ios')
+        buildops.cmd(
+            ["open", f"{app_name}.xcodeproj"],
+            cwd=join(ios_dir, f"{app_name}-ios"),
+            env=self.buildozer.environ
         )
 
     def _run_ios_deploy(self, lldb=False):
@@ -353,11 +374,11 @@ def _run_ios_deploy(self, lldb=False):
             debug_mode = ''
             self.logger.info('Deploy the application')
 
-        self.buildozer.cmd(
+        buildops.cmd(
             [join(self.ios_deploy_dir, "ios-deploy"), debug_mode, "-b", ios_app_dir],
             cwd=self.ios_dir,
             show_output=True,
-        )
+            env=self.buildozer.environ)
 
     def _create_icons(self):
         icon = self.buildozer.config.getdefault('app', 'icon.filename', '')
@@ -405,9 +426,11 @@ def cmd_list_identities(self, *args):
             print('  - {}'.format(x))
 
     def _get_available_identities(self):
-        output = self.buildozer.cmd(
-            ["security", "find-identity", "-v", "-p", "codesigning"], get_stdout=True
-        )[0]
+        output = buildops.cmd(
+            ["security", "find-identity", "-v", "-p", "codesigning"],
+            get_stdout=True,
+            env=self.buildozer.environ
+        ).stdout
 
         lines = output.splitlines()[:-1]
         lines = [u'"{}"'.format(x.split('"')[1]) for x in lines]
@@ -422,17 +445,21 @@ def _unlock_keychain(self):
 
         if not password:
             # no password available, try to unlock anyway...
-            error = self.buildozer.cmd(["security", "unlock-keychain", "-u"],
-                    break_on_error=False)[2]
+            error = buildops.cmd(
+                ["security", "unlock-keychain", "-u"],
+                break_on_error=False,
+                quiet=True,  # Log doesn't need secure info
+                env=self.buildozer.environ).return_code
             if not error:
                 return
         else:
             # password available, try to unlock
-            error = self.buildozer.cmd(
+            error = buildops.cmd(
                 ["security", "unlock-keychain", "-p", password],
                 break_on_error=False,
-                sensible=True,
-            )[2]
+                quiet=True,  # Log doesn't need secure info
+                env=self.buildozer.environ
+            ).return_code
             if not error:
                 return
 
@@ -442,11 +469,12 @@ def _unlock_keychain(self):
         while attempt:
             attempt -= 1
             password = getpass('Password to unlock the default keychain:')
-            error = self.buildozer.cmd(
+            error = buildops.cmd(
                 ["security", "unlock-keychain", "-p", password],
+                quiet=True,  # Log doesn't need secure info
                 break_on_error=False,
-                sensible=True,
-            )[2]
+                env=self.buildozer.environ
+            ).return_code
             if not error:
                 correct = True
                 break
diff --git a/buildozer/targets/osx.py b/buildozer/targets/osx.py
index 059f521e9..6ceca122c 100644
--- a/buildozer/targets/osx.py
+++ b/buildozer/targets/osx.py
@@ -33,7 +33,8 @@ def ensure_sdk(self):
             'https://github.com/kivy/kivy-sdk-packager/archive/master.zip',
             'master.zip',
             cwd=platdir)
-        check_call(('unzip', 'master.zip'), cwd=platdir)
+        buildops.file_extract(
+            'master.zip', cwd=platdir, env=self.buildozer.environ)
         buildops.file_remove(join(platdir, 'master.zip'))
 
     def download_kivy(self, cwd):
diff --git a/tests/targets/test_android.py b/tests/targets/test_android.py
index 7809a35d3..a3db75825 100644
--- a/tests/targets/test_android.py
+++ b/tests/targets/test_android.py
@@ -9,23 +9,22 @@
 from buildozer.targets.android import TargetAndroid
 from tests.targets.utils import (
     init_buildozer,
-    patch_buildozer,
     patch_buildops_checkbin,
-    patch_buildozer_cmd,
+    patch_buildops_cmd,
     patch_buildops_file_exists,
 )
 
 
-def patch_buildozer_cmd_expect():
-    return patch_buildozer("cmd_expect")
+def patch_buildops_cmd_expect():
+    return mock.patch("buildozer.buildops.cmd_expect")
 
 
 def patch_buildops_download():
     return mock.patch("buildozer.buildops.download")
 
 
-def patch_buildozer_file_extract():
-    return patch_buildozer("file_extract")
+def patch_buildops_file_extract():
+    return mock.patch("buildozer.buildops.file_extract")
 
 
 def patch_os_isfile():
@@ -131,18 +130,18 @@ def test_sdkmanager(self):
         """Tests the _sdkmanager() method."""
         target_android = init_target(self.temp_dir)
         kwargs = {}
-        with patch_buildozer_cmd() as m_cmd, patch_buildozer_cmd_expect() as m_cmd_expect, patch_os_isfile() as m_isfile:
+        with patch_buildops_cmd() as m_cmd, patch_buildops_cmd_expect() as m_cmd_expect, patch_os_isfile() as m_isfile:
             m_isfile.return_value = True
-            assert m_cmd.return_value == target_android._sdkmanager(**kwargs)
+            m_cmd.return_value = "Some value"
+            assert target_android._sdkmanager(**kwargs) == "Some value"
         assert m_cmd.call_count == 1
         assert m_cmd_expect.call_count == 0
         assert m_isfile.call_count == 1
         kwargs = {"return_child": True}
-        with patch_buildozer_cmd() as m_cmd, patch_buildozer_cmd_expect() as m_cmd_expect, patch_os_isfile() as m_isfile:
+        with patch_buildops_cmd() as m_cmd, patch_buildops_cmd_expect() as m_cmd_expect, patch_os_isfile() as m_isfile:
             m_isfile.return_value = True
-            assert m_cmd_expect.return_value == target_android._sdkmanager(
-                **kwargs
-            )
+            m_cmd_expect.return_value = "Some value"
+            assert target_android._sdkmanager(**kwargs) == "Some value"
         assert m_cmd.call_count == 0
         assert m_cmd_expect.call_count == 1
         assert m_isfile.call_count == 1
@@ -154,7 +153,7 @@ def test_check_requirements(self):
         assert not hasattr(target_android, "adb_executable")
         assert not hasattr(target_android, "adb_args")
         assert not hasattr(target_android, "javac_cmd")
-        assert "PATH" not in buildozer.environ
+        del buildozer.environ["PATH"]
         with patch_buildops_checkbin() as m_checkbin:
             target_android.check_requirements()
         assert m_checkbin.call_args_list == [
@@ -192,7 +191,7 @@ def test_install_android_sdk(self, platform):
         assert sdk_dir.endswith(".buildozer/android/platform/android-sdk")
         with patch_buildops_file_exists() as m_file_exists, \
                 patch_buildops_download() as m_download, \
-                patch_buildozer_file_extract() as m_file_extract, \
+                patch_buildops_file_extract() as m_file_extract, \
                 patch_platform(platform):
             m_file_exists.return_value = False
             sdk_dir = target_android._install_android_sdk()
@@ -209,7 +208,8 @@ def test_install_android_sdk(self, platform):
                 cwd=mock.ANY,
             )
         ]
-        assert m_file_extract.call_args_list == [mock.call(archive, cwd=mock.ANY)]
+        assert m_file_extract.call_args_list == [
+            mock.call(archive, cwd=mock.ANY, env=mock.ANY)]
         assert sdk_dir.endswith(".buildozer/android/platform/android-sdk")
 
     def test_build_package(self):
@@ -255,7 +255,7 @@ def test_execute_build_package__debug__apk(self):
                 "arm64-v8a",
                 "--arch",
                 "armeabi-v7a"
-            ])
+            ], env=mock.ANY)
         ]
 
     def test_execute_build_package__release__apk(self):
@@ -278,7 +278,7 @@ def test_execute_build_package__release__apk(self):
                 "arm64-v8a",
                 "--arch",
                 "armeabi-v7a"
-            ])
+            ], env=mock.ANY)
         ]
 
     def test_execute_build_package__release__aab(self):
@@ -302,7 +302,7 @@ def test_execute_build_package__release__aab(self):
                 "arm64-v8a",
                 "--arch",
                 "armeabi-v7a",
-            ])
+            ], env=mock.ANY)
         ]
 
     def test_numeric_version(self):
@@ -429,13 +429,14 @@ def test_install_platform_p4a_clone_url(self):
             'p4a.fork': 'myfork',
         })
 
-        with patch_buildozer_cmd() as m_cmd, mock.patch('buildozer.targets.android.open') as m_open:
+        with patch_buildops_cmd() as m_cmd, mock.patch('buildozer.targets.android.open') as m_open:
             m_open.return_value = StringIO('install_reqs = []')  # to stub setup.py parsing
             target_android._install_p4a()
 
         assert mock.call(
             ["git", "clone", "-b", "master", "--single-branch", "https://custom-p4a-url/p4a.git", "python-for-android"],
-            cwd=mock.ANY) in m_cmd.call_args_list
+            cwd=mock.ANY,
+            env=mock.ANY) in m_cmd.call_args_list
 
     def test_install_platform_p4a_clone_fork(self):
         """The `p4a.fork` config should be used for cloning p4a."""
@@ -443,25 +444,27 @@ def test_install_platform_p4a_clone_fork(self):
             'p4a.fork': 'fork'
         })
 
-        with patch_buildozer_cmd() as m_cmd, mock.patch('buildozer.targets.android.open') as m_open:
+        with patch_buildops_cmd() as m_cmd, mock.patch('buildozer.targets.android.open') as m_open:
             m_open.return_value = StringIO('install_reqs = []')  # to stub setup.py parsing
             target_android._install_p4a()
 
         assert mock.call(
             ["git", "clone", "-b", "master", "--single-branch", "https://github.com/fork/python-for-android.git", "python-for-android"],
-            cwd=mock.ANY) in m_cmd.call_args_list
+            cwd=mock.ANY,
+            env=mock.ANY) in m_cmd.call_args_list
 
     def test_install_platform_p4a_clone_default(self):
         """The default URL should be used for cloning p4a if no config options `p4a.url` and `p4a.fork` are set."""
         target_android = init_target(self.temp_dir)
 
-        with patch_buildozer_cmd() as m_cmd, mock.patch('buildozer.targets.android.open') as m_open:
+        with patch_buildops_cmd() as m_cmd, mock.patch('buildozer.targets.android.open') as m_open:
             m_open.return_value = StringIO('install_reqs = []')  # to stub setup.py parsing
             target_android._install_p4a()
 
         assert mock.call(
             ["git", "clone", "-b", "master", "--single-branch", "https://github.com/kivy/python-for-android.git", "python-for-android"],
-            cwd=mock.ANY) in m_cmd.call_args_list
+            cwd=mock.ANY,
+            env=mock.ANY) in m_cmd.call_args_list
 
     def test_orientation(self):
         target_android = init_target(self.temp_dir, {
diff --git a/tests/targets/test_ios.py b/tests/targets/test_ios.py
index 071b72efe..fdf109f52 100644
--- a/tests/targets/test_ios.py
+++ b/tests/targets/test_ios.py
@@ -5,12 +5,13 @@
 
 import pytest
 
+from buildozer.buildops import CommandResult
 from buildozer.exceptions import BuildozerCommandException
 from buildozer.targets.ios import TargetIos
 from tests.targets.utils import (
     init_buildozer,
     patch_buildops_checkbin,
-    patch_buildozer_cmd,
+    patch_buildops_cmd,
     patch_buildops_file_exists,
     patch_logger_error,
 )
@@ -53,9 +54,7 @@ def test_init(self):
     def test_check_requirements(self):
         """Basic tests for the check_requirements() method."""
         target = init_target(self.temp_dir)
-        buildozer = target.buildozer
         assert not hasattr(target, "javac_cmd")
-        assert "PATH" not in buildozer.environ
         with patch_buildops_checkbin() as m_checkbin:
             target.check_requirements()
         assert m_checkbin.call_args_list == [
@@ -112,7 +111,7 @@ def test_install_platform(self):
         target = init_target(self.temp_dir)
         assert target.ios_dir is None
         assert target.ios_deploy_dir is None
-        with patch_buildozer_cmd() as m_cmd:
+        with patch_buildops_cmd() as m_cmd:
             target.install_platform()
         assert m_cmd.call_args_list == [
             mock.call(
@@ -124,6 +123,7 @@ def test_install_platform(self):
                     "https://github.com/kivy/kivy-ios",
                 ],
                 cwd=mock.ANY,
+                env=mock.ANY,
             ),
             mock.call(
                 [
@@ -134,6 +134,7 @@ def test_install_platform(self):
                     "https://github.com/phonegap/ios-deploy",
                 ],
                 cwd=mock.ANY,
+                env=mock.ANY,
             ),
         ]
         assert target.ios_dir.endswith(".buildozer/ios/platform/kivy-ios")
@@ -176,12 +177,12 @@ def test_unlock_keychain_wrong_password(self):
         target = init_target(self.temp_dir)
         # fmt: off
         with mock.patch("buildozer.targets.ios.getpass") as m_getpass, \
-             patch_buildozer_cmd() as m_cmd, \
+             patch_buildops_cmd() as m_cmd, \
              pytest.raises(BuildozerCommandException):
             m_getpass.return_value = "password"
             # the `security unlock-keychain` command returned an error
             # hence we'll get prompted to enter the password
-            m_cmd.return_value = (None, None, 123)
+            m_cmd.return_value = CommandResult(None, None, 123)
             target._unlock_keychain()
         # fmt: on
         assert m_getpass.call_args_list == [
@@ -199,7 +200,7 @@ def test_build_package_no_signature(self):
              patch_logger_error() as m_error, \
              mock.patch("buildozer.targets.ios.TargetIos.load_plist_from_file") as m_load_plist_from_file, \
              mock.patch("buildozer.targets.ios.TargetIos.dump_plist_to_file") as m_dump_plist_to_file, \
-             patch_buildozer_cmd() as m_cmd:
+             patch_buildops_cmd() as m_cmd:
             m_load_plist_from_file.return_value = {}
             target.build_package()
         # fmt: on
@@ -224,7 +225,7 @@ def test_build_package_no_signature(self):
             )
         ]
         assert m_cmd.call_args_list == [
-            mock.call(mock.ANY, cwd=target.ios_dir),
+            mock.call(mock.ANY, cwd=target.ios_dir, env=mock.ANY),
             mock.call([
                 "xcodebuild",
                 "-configuration",
@@ -235,6 +236,7 @@ def test_build_package_no_signature(self):
                 "clean",
                 "build"],
                 cwd="/ios/dir/myapp-ios",
+                env=mock.ANY,
             ),
             mock.call([
                 "xcodebuild",
@@ -251,5 +253,6 @@ def test_build_package_no_signature(self):
                 "ENABLE_BITCODE=NO",
                 "CODE_SIGNING_ALLOWED=NO"],
                 cwd="/ios/dir/myapp-ios",
+                env=mock.ANY,
             ),
         ]
diff --git a/tests/targets/utils.py b/tests/targets/utils.py
index e98940780..dd41c4fc8 100644
--- a/tests/targets/utils.py
+++ b/tests/targets/utils.py
@@ -6,12 +6,8 @@
 from buildozer import Buildozer
 
 
-def patch_buildozer(method):
-    return mock.patch("buildozer.Buildozer.{method}".format(method=method))
-
-
-def patch_buildozer_cmd():
-    return patch_buildozer("cmd")
+def patch_buildops_cmd():
+    return mock.patch("buildozer.buildops.cmd")
 
 
 def patch_buildops_checkbin():
diff --git a/tests/test_buildops.py b/tests/test_buildops.py
index 227b6c2ff..723d21edb 100644
--- a/tests/test_buildops.py
+++ b/tests/test_buildops.py
@@ -222,6 +222,8 @@ def test_extract_file(self):
         with mock.patch(
             "buildozer.buildops.LOGGER"
         ) as m_logger, TemporaryDirectory() as base_dir:
+            m_logger.log_level = 2
+            m_logger.INFO = 1
 
             # Test behaviour when the source doesn't exist
             nonexistent_path = Path(base_dir) / "wrongfiletype.txt"
@@ -239,10 +241,10 @@ def test_extract_file(self):
             m_logger.reset_mock()
 
             nonexistent_path = Path(base_dir) / "nonexistent.zip"
-            with self.assertRaises(FileNotFoundError):
+            with self.assertRaises(BuildozerCommandException):
                 buildops.file_extract(nonexistent_path, environ)
             m_logger.debug.assert_called()
-            m_logger.error.assert_not_called()
+            m_logger.error.assert_called()
             m_logger.reset_mock()
 
             # Create a zip file and unzip it.
@@ -260,6 +262,9 @@ def test_extract_file(self):
                 assert uncompressed_file.read() == "Text to zip"
             m_logger.reset_mock()
 
+            # Todo: Create a multi-file zip file with permissions.
+            # Show it unpacks.
+
             # Create a tgz file and untgz it.
             text_file_path = Path(base_dir) / "text_to_tgz.txt"
             with open(text_file_path, "w") as outfile:
diff --git a/tests/test_buildozer.py b/tests/test_buildozer.py
index 38829f682..6f1a8a451 100644
--- a/tests/test_buildozer.py
+++ b/tests/test_buildozer.py
@@ -126,10 +126,11 @@ def test_android_ant_path(self):
 
         # Mock first run
         with mock.patch('buildozer.buildops.download') as download, \
-                mock.patch('buildozer.Buildozer.file_extract') as m_file_extract, \
+                mock.patch('buildozer.buildops.file_extract') as m_file_extract, \
                 mock.patch('os.makedirs'):
             ant_path = target._install_apache_ant()
-        assert m_file_extract.call_args_list == [mock.call(mock.ANY, cwd='/my/ant/path')]
+        assert m_file_extract.call_args_list == [
+            mock.call(mock.ANY, cwd='/my/ant/path', env=mock.ANY)]
         assert ant_path == my_ant_path
         assert download.call_args_list == [
             mock.call("https://archive.apache.org/dist/ant/binaries/", mock.ANY, cwd=my_ant_path)]
@@ -138,43 +139,6 @@ def test_android_ant_path(self):
             ant_path = target._install_apache_ant()
         assert ant_path == my_ant_path
 
-    def test_cmd_unicode_decode(self):
-        """
-        Verifies Buildozer.cmd() can properly handle non-unicode outputs.
-        refs: https://github.com/kivy/buildozer/issues/857
-        """
-        buildozer = Buildozer()
-        command = 'uname'
-        kwargs = {
-            'show_output': True,
-            'get_stdout': True,
-            'get_stderr': True,
-        }
-        command_output = b'\x80 cannot decode \x80'
-        # showing the point that we can't decode it
-        with self.assertRaises(UnicodeDecodeError):
-            command_output.decode('utf-8')
-        with mock.patch('buildozer.Popen') as m_popen, \
-                mock.patch('buildozer.select') as m_select, \
-                mock.patch('buildozer.stdout') as m_stdout:
-            m_select.select().__getitem__.return_value = [0]
-            # makes sure fcntl.fcntl() gets what it expects so it doesn't crash
-            m_popen().stdout.fileno.return_value = 0
-            m_popen().stderr.fileno.return_value = 2
-            # Buildozer.cmd() is iterating through command output "chunk" until
-            # one chunk is None
-            m_popen().stdout.read.side_effect = [command_output, None]
-            m_popen().returncode = 0
-            stdout, stderr, returncode = buildozer.cmd(command, **kwargs)
-        # when get_stdout is True, the command output also gets returned
-        assert stdout == command_output.decode('utf-8', 'ignore')
-        assert stderr is None
-        assert returncode == 0
-        # Python2 and Python3 have different approaches for decoding the output
-        assert m_stdout.write.call_args_list == [
-            mock.call(command_output.decode('utf-8', 'replace'))
-        ]
-
     def test_p4a_recommended_ndk_version_default_value(self):
         self.set_specfile_log_level(self.specfile.name, 1)
         buildozer = Buildozer(self.specfile.name, 'android')

From a916e5494800cb4b9bd3377800bbeb780d2e65f7 Mon Sep 17 00:00:00 2001
From: Julian-o <github@somethinkodd.com>
Date: Sun, 10 Sep 2023 03:15:07 +1000
Subject: [PATCH 2/2] Simplify check_requirements

Assume env var has a path.
Use os.pathsep (not to be confused with os.path.sep) instead of magic string.

Update test to ensure assumption is correct, and check ANT path has been prepended to the path var.
---
 buildozer/targets/android.py  | 16 ++++++++--------
 tests/targets/test_android.py |  8 ++++++--
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/buildozer/targets/android.py b/buildozer/targets/android.py
index b77cb279e..932bb2fbb 100644
--- a/buildozer/targets/android.py
+++ b/buildozer/targets/android.py
@@ -302,14 +302,14 @@ def check_requirements(self):
             "app", "android.adb_args", "")
         self.adb_args = shlex.split(adb_args)
 
-        # Need to add internally installed ant to path for external tools
-        # like adb to use
-        path = [join(self.apache_ant_dir, 'bin')]
-        if 'PATH' in self.buildozer.environ:
-            path.append(self.buildozer.environ['PATH'])
-        else:
-            path.append(os.environ['PATH'])
-        self.buildozer.environ['PATH'] = ':'.join(path)
+        # Need to add internally-installed ant to path for external tools
+        # (like adb) to use
+        self.buildozer.environ['PATH'] = os.pathsep.join(
+            [
+                join(self.apache_ant_dir, 'bin'),
+                self.buildozer.environ['PATH'],
+            ])
+
         buildops.checkbin('Git (git)', 'git')
         buildops.checkbin('Cython (cython)', 'cython')
         buildops.checkbin('Java compiler (javac)', self.javac_cmd)
diff --git a/tests/targets/test_android.py b/tests/targets/test_android.py
index a3db75825..5b1546696 100644
--- a/tests/targets/test_android.py
+++ b/tests/targets/test_android.py
@@ -1,4 +1,5 @@
 import os
+import os.path
 import tempfile
 from io import StringIO
 from unittest import mock
@@ -153,7 +154,7 @@ def test_check_requirements(self):
         assert not hasattr(target_android, "adb_executable")
         assert not hasattr(target_android, "adb_args")
         assert not hasattr(target_android, "javac_cmd")
-        del buildozer.environ["PATH"]
+        assert "PATH" in buildozer.environ
         with patch_buildops_checkbin() as m_checkbin:
             target_android.check_requirements()
         assert m_checkbin.call_args_list == [
@@ -166,7 +167,10 @@ def test_check_requirements(self):
         assert target_android.adb_args == []
         assert target_android.javac_cmd == "javac"
         assert target_android.keytool_cmd == "keytool"
-        assert "PATH" in buildozer.environ
+        assert buildozer.environ["PATH"].startswith(
+            os.path.join(
+                target_android.apache_ant_dir, "bin")
+        )
 
     def test_check_configuration_tokens(self):
         """Basic tests for the check_configuration_tokens() method."""