Skip to content

Commit

Permalink
Migrate to use cmd, cmd_expect and file_extract from Buildops.
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@Julian-O
Julian-O committed Aug 30, 2023
1 parent 236b22a commit ca206d5
Show file tree
Hide file tree
Showing 11 changed files with 238 additions and 345 deletions.
182 changes: 11 additions & 171 deletions buildozer/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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'.
'''
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand All @@ -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):
Expand Down
41 changes: 20 additions & 21 deletions buildozer/buildops.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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()


Expand All @@ -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):
Expand Down
16 changes: 12 additions & 4 deletions buildozer/target.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,19 +249,27 @@ 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):
if custom_dir:
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
Loading

0 comments on commit ca206d5

Please sign in to comment.