Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add external ports + options support to embuilder #21345

Merged
merged 8 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions embuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from tools import shared
from tools import system_libs
from tools import ports
from tools import utils
from tools.settings import settings
from tools.system_libs import USE_NINJA

Expand Down Expand Up @@ -169,6 +170,10 @@ def get_all_tasks():
return get_system_tasks()[1] + PORTS


def handle_port_error(target, message):
utils.exit_with_error(f'error building port `{target}` | {message}')


def main():
all_build_start_time = time.time()

Expand Down Expand Up @@ -289,6 +294,12 @@ def main():
clear_port(what)
if do_build:
build_port(what)
elif ':' in what or what.endswith('.py'):
name = ports.handle_use_port_arg(settings, what, lambda message: handle_port_error(what, message))
if do_clear:
clear_port(name)
if do_build:
build_port(name)
else:
logger.error('unfamiliar build target: ' + what)
return 1
Expand Down
7 changes: 5 additions & 2 deletions test/other/ports/external.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@


def get_lib_name(settings):
return 'lib_external.a'
if opts['dependency']:
return f'lib_external-{opts["dependency"]}.a'
else:
return 'lib_external.a'


def get(ports, settings, shared):
Expand Down Expand Up @@ -58,5 +61,5 @@ def process_dependencies(settings):
deps.append(opts['dependency'])


def handle_options(options):
def handle_options(options, error_handler):
opts.update(options)
12 changes: 6 additions & 6 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -2416,7 +2416,7 @@ def test_external_ports(self):
# testing invalid dependency
stderr = self.expect_fail([EMCC, test_file('other/test_external_ports.c'), f'--use-port={external_port_path}:dependency=invalid', '-o', 'a4.out.js'])
self.assertFalse(os.path.exists('a4.out.js'))
self.assertContained('Unknown dependency `invalid` for port `external`', stderr)
self.assertContained('unknown dependency `invalid` for port `external`', stderr)

def test_link_memcpy(self):
# memcpy can show up *after* optimizations, so after our opportunity to link in libc, so it must be special-cased
Expand Down Expand Up @@ -14557,16 +14557,16 @@ def test_js_preprocess_pre_post(self):
def test_use_port_errors(self, compiler):
stderr = self.expect_fail([compiler, test_file('hello_world.c'), '--use-port=invalid', '-o', 'out.js'])
self.assertFalse(os.path.exists('out.js'))
self.assertContained('Error with `--use-port=invalid` | invalid port name: `invalid`', stderr)
self.assertContained('error with `--use-port=invalid` | invalid port name: `invalid`', stderr)
stderr = self.expect_fail([compiler, test_file('hello_world.c'), '--use-port=sdl2:opt1=v1', '-o', 'out.js'])
self.assertFalse(os.path.exists('out.js'))
self.assertContained('Error with `--use-port=sdl2:opt1=v1` | no options available for port `sdl2`', stderr)
self.assertContained('error with `--use-port=sdl2:opt1=v1` | no options available for port `sdl2`', stderr)
stderr = self.expect_fail([compiler, test_file('hello_world.c'), '--use-port=sdl2_image:format=jpg', '-o', 'out.js'])
self.assertFalse(os.path.exists('out.js'))
self.assertContained('Error with `--use-port=sdl2_image:format=jpg` | `format` is not supported', stderr)
self.assertContained('error with `--use-port=sdl2_image:format=jpg` | `format` is not supported', stderr)
stderr = self.expect_fail([compiler, test_file('hello_world.c'), '--use-port=sdl2_image:formats', '-o', 'out.js'])
self.assertFalse(os.path.exists('out.js'))
self.assertContained('Error with `--use-port=sdl2_image:formats` | `formats` is missing a value', stderr)
self.assertContained('error with `--use-port=sdl2_image:formats` | `formats` is missing a value', stderr)
stderr = self.expect_fail([compiler, test_file('hello_world.c'), '--use-port=sdl2_image:formats=jpg:formats=png', '-o', 'out.js'])
self.assertFalse(os.path.exists('out.js'))
self.assertContained('Error with `--use-port=sdl2_image:formats=jpg:formats=png` | duplicate option `formats`', stderr)
self.assertContained('error with `--use-port=sdl2_image:formats=jpg:formats=png` | duplicate option `formats`', stderr)
22 changes: 22 additions & 0 deletions test/test_sanity.py
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,28 @@ def test_embuilder_wildcards(self):
self.run_process([EMBUILDER, 'build', 'libwebgpu*'])
self.assertGreater(len(glob.glob(glob_match)), 3)

def test_embuilder_with_use_port_syntax(self):
restore_and_set_up()
self.run_process([EMBUILDER, 'build', 'sdl2_image:formats=png,jpg', '--force'])
self.assertExists(os.path.join(config.CACHE, 'sysroot', 'lib', 'wasm32-emscripten', 'libSDL2_image_jpg-png.a'))
self.assertContained('error building port `sdl2_image:formats=invalid` | invalid is not a supported format', self.do([EMBUILDER, 'build', 'sdl2_image:formats=invalid', '--force']))

def test_embuilder_external_ports(self):
restore_and_set_up()
simple_port_path = test_file("other/ports/simple.py")
# embuilder handles external port target that ends with .py
self.run_process([EMBUILDER, 'build', f'{simple_port_path}', '--force'])
self.assertExists(os.path.join(config.CACHE, 'sysroot', 'lib', 'wasm32-emscripten', 'lib_simple.a'))
# embuilder handles external port target that contains port options
external_port_path = test_file("other/ports/external.py")
self.run_process([EMBUILDER, 'build', f'{external_port_path}:value1=12:value2=36', '--force'])
self.assertExists(os.path.join(config.CACHE, 'sysroot', 'lib', 'wasm32-emscripten', 'lib_external.a'))
# embuilder handles external port target that contains port options (influences library name,
# like sdl2_image:formats=png)
external_port_path = test_file("other/ports/external.py")
self.run_process([EMBUILDER, 'build', f'{external_port_path}:dependency=sdl2', '--force'])
self.assertExists(os.path.join(config.CACHE, 'sysroot', 'lib', 'wasm32-emscripten', 'lib_external-sdl2.a'))

def test_binaryen_version(self):
restore_and_set_up()
with open(EM_CONFIG, 'a') as f:
Expand Down
24 changes: 14 additions & 10 deletions tools/ports/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ def add_deps(node):
node.process_dependencies(settings)
for d in node.deps:
if d not in ports_by_name:
utils.exit_with_error(f'Unknown dependency `{d}` for port `{node.name}`')
utils.exit_with_error(f'unknown dependency `{d}` for port `{node.name}`')
dep = ports_by_name[d]
if dep not in port_set:
port_set.add(dep)
Expand All @@ -409,10 +409,13 @@ def add_deps(node):


def handle_use_port_error(arg, message):
utils.exit_with_error(f'Error with `--use-port={arg}` | {message}')
utils.exit_with_error(f'error with `--use-port={arg}` | {message}')


def handle_use_port_arg(settings, arg):
def handle_use_port_arg(settings, arg, error_handler=None):
if not error_handler:
def error_handler(message):
ypujante marked this conversation as resolved.
Show resolved Hide resolved
handle_use_port_error(arg, message)
# Ignore ':' in first or second char of string since we could be dealing with a windows drive separator
pos = arg.find(':', 2)
if pos != -1:
Expand All @@ -422,27 +425,28 @@ def handle_use_port_arg(settings, arg):
if name.endswith('.py'):
port_file_path = name
if not os.path.isfile(port_file_path):
handle_use_port_error(arg, f'not a valid port path: {port_file_path}')
error_handler(f'not a valid port path: {port_file_path}')
name = load_port_by_path(port_file_path)
elif name not in ports_by_name:
handle_use_port_error(arg, f'invalid port name: `{name}`')
error_handler(f'invalid port name: `{name}`')
ports_needed.add(name)
if options:
port = ports_by_name[name]
if not hasattr(port, 'handle_options'):
handle_use_port_error(arg, f'no options available for port `{name}`')
error_handler(f'no options available for port `{name}`')
else:
options_dict = {}
for name_value in options.split(':'):
nv = name_value.split('=', 1)
if len(nv) != 2:
handle_use_port_error(arg, f'`{name_value}` is missing a value')
error_handler(f'`{name_value}` is missing a value')
if nv[0] not in port.OPTIONS:
handle_use_port_error(arg, f'`{nv[0]}` is not supported; available options are {port.OPTIONS}')
error_handler(f'`{nv[0]}` is not supported; available options are {port.OPTIONS}')
if nv[0] in options_dict:
handle_use_port_error(arg, f'duplicate option `{nv[0]}`')
error_handler(f'duplicate option `{nv[0]}`')
options_dict[nv[0]] = nv[1]
port.handle_options(options_dict)
port.handle_options(options_dict, error_handler)
return name


def get_needed_ports(settings):
Expand Down
7 changes: 4 additions & 3 deletions tools/ports/contrib/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,18 @@ additional components:

1. A handler function defined this way:
```python
def handle_options(options):
def handle_options(options, error_handler):
# options is of type Dict[str, str]
# in case of error, use utils.exit_with_error('error message')
# in case of error, use error_handler('error message')
ypujante marked this conversation as resolved.
Show resolved Hide resolved
# note that error_handler is guaranteed to never return
```
2. A dictionary called `OPTIONS` (type `Dict[str, str]`) where each key is the
name of the option and the value is a short description of what it does

When emscripten detects that options have been provided, it parses them and
check that they are valid option names for this port (using `OPTIONS`). It then
calls the handler function with these (valid) options. If you detect an error
with a value, you should use `tools.utils.exit_with_error` to report the
with a value, you should use the error handler provided to report the
failure.

> ### Note
Expand Down
5 changes: 2 additions & 3 deletions tools/ports/contrib/glfw3.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
# found in the LICENSE file.

import os
from tools import utils
from typing import Dict

TAG = '1.0.4'
Expand Down Expand Up @@ -83,9 +82,9 @@ def process_args(ports):
return ['-isystem', ports.get_include_dir('contrib.glfw3')]


def handle_options(options):
def handle_options(options, error_handler):
for option, value in options.items():
if value.lower() in {'true', 'false'}:
opts[option] = value.lower() == 'true'
else:
utils.exit_with_error(f'{option} is expecting a boolean, got {value}')
error_handler(f'{option} is expecting a boolean, got {value}')
13 changes: 11 additions & 2 deletions tools/ports/sdl2_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
'formats': 'A comma separated list of formats (ex: --use-port=sdl2_image:formats=png,jpg)'
}

SUPPORTED_FORMATS = {'avif', 'bmp', 'gif', 'jpg', 'jxl', 'lbm', 'pcx', 'png',
'pnm', 'qoi', 'svg', 'tga', 'tif', 'webp', 'xcf', 'xpm', 'xv'}

ypujante marked this conversation as resolved.
Show resolved Hide resolved
# user options (from --use-port)
opts: Dict[str, Set] = {
'formats': set()
Expand Down Expand Up @@ -88,8 +91,14 @@ def process_dependencies(settings):
settings.USE_LIBJPEG = 1


def handle_options(options):
opts['formats'].update({format.lower().strip() for format in options['formats'].split(',')})
def handle_options(options, error_handler):
formats = options['formats'].split(',')
for format in formats:
format = format.lower().strip()
if format not in SUPPORTED_FORMATS:
error_handler(f'{format} is not a supported format')
else:
opts['formats'].add(format)


def show():
Expand Down
Loading