From 8e2ce1881d68465a3dde23884e2dc35c4b797f04 Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Tue, 6 Feb 2024 09:30:05 -0800 Subject: [PATCH 01/11] add options support for contrib ports --- test/test_browser.py | 2 +- test/test_other.py | 2 +- tools/ports/__init__.py | 10 +++++- tools/ports/contrib/glfw3.py | 67 ++++++++++++++++++++++++++++++++---- tools/ports/sdl2_image.py | 35 ++++++++++++++----- 5 files changed, 98 insertions(+), 18 deletions(-) diff --git a/test/test_browser.py b/test/test_browser.py index 5e3f6e93bf8fc..3bdd3b947399d 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -3025,7 +3025,7 @@ def test_sdl2_image_formats(self): self.btest_exit('test_sdl2_image.c', 600, args=[ '--preload-file', 'screenshot.jpg', '-DSCREENSHOT_DIRNAME="/"', '-DSCREENSHOT_BASENAME="screenshot.jpg"', '-DBITSPERPIXEL=24', '-DNO_PRELOADED', - '-sUSE_SDL=2', '-sUSE_SDL_IMAGE=2', '-sSDL2_IMAGE_FORMATS=["jpg"]' + '--use-port=sdl2', '--use-port=sdl2_image?formats=["jpg"]' ]) @no_wasm64('SDL2 + wasm64') diff --git a/test/test_other.py b/test/test_other.py index aba5e0e557c4a..978e53860c969 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -2372,7 +2372,7 @@ def test_sdl2_ttf(self): def test_contrib_ports(self): # Verify that contrib ports can be used (using the only contrib port available ATM, but can be replaced # with a different contrib port when there is another one - self.emcc(test_file('other/test_contrib_ports.cpp'), ['--use-port=contrib.glfw3']) + self.emcc(test_file('other/test_contrib_ports.cpp'), ['--use-port=contrib.glfw3?disableWarning=true']) 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 diff --git a/tools/ports/__init__.py b/tools/ports/__init__.py index 9ceb3b63e37e6..fdcc1d1d2458b 100644 --- a/tools/ports/__init__.py +++ b/tools/ports/__init__.py @@ -393,10 +393,18 @@ def add_deps(node): add_deps(port) -def handle_use_port_arg(settings, name): +def handle_use_port_arg(settings, arg): + args = arg.split('?', 1) + name, options = args[0], args[1] if len(args) == 2 else None if name not in ports_by_name: utils.exit_with_error(f'Invalid port name: {name} used with --use-port') ports_needed.add(name) + if options: + port = ports_by_name[name] + if not hasattr(port, 'handle_options'): + utils.exit_with_error(f'Invalid options for port {name}: No options available') + elif (error := port.handle_options(options)) is not None: + utils.exit_with_error(f'Invalid options for port {name}: {error}') def get_needed_ports(settings): diff --git a/tools/ports/contrib/glfw3.py b/tools/ports/contrib/glfw3.py index 60caa4231620d..3a657a214312c 100644 --- a/tools/ports/contrib/glfw3.py +++ b/tools/ports/contrib/glfw3.py @@ -4,23 +4,44 @@ # found in the LICENSE file. import os - -TAG = '1.0.4' -HASH = 'c3c96718e5d2b37df434a46c4a93ddfd9a768330d33f0d6ce2d08c139752894c2421cdd0fefb800fe41fafc2bbe58c8f22b8aa2849dc4fc6dde686037215cfad' +from urllib.parse import parse_qs # contrib port information (required) URL = 'https://github.com/pongasoft/emscripten-glfw' DESCRIPTION = 'This project is an emscripten port of GLFW written in C++ for the web/webassembly platform' LICENSE = 'Apache 2.0 license' +OPTIONS = { + 'tag': 'The git tag to use a different release', + 'hash': 'The sha512 of the release associated to the tag (can be omitted)', + 'disableWarning': 'Boolean to disable warnings emitted by the library', + 'disableJoystick': 'Boolean to disable support for joystick entirely', + 'disableMultiWindow': 'Boolean to disable multi window support which makes the code smaller and faster' +} + +# user options (from --use-port) +opts = { + 'tag': '1.0.4', + 'hash': 'c3c96718e5d2b37df434a46c4a93ddfd9a768330d33f0d6ce2d08c139752894c2421cdd0fefb800fe41fafc2bbe58c8f22b8aa2849dc4fc6dde686037215cfad', + 'disableWarning': False, + 'disableJoystick': False, + 'disableMultiWindow': False +} + def get_lib_name(settings): - return 'lib_contrib.glfw3.a' + return (f'lib_contrib.glfw3_{opts["tag"]}' + + ('-nw' if opts['disableWarning'] else '') + + ('-nj' if opts['disableJoystick'] else '') + + ('-sw' if opts['disableMultiWindow'] else '') + + '.a') def get(ports, settings, shared): # get the port - ports.fetch_project('contrib.glfw3', f'https://github.com/pongasoft/emscripten-glfw/releases/download/v{TAG}/emscripten-glfw3-{TAG}.zip', sha512hash=HASH) + ports.fetch_project('contrib.glfw3', + f'https://github.com/pongasoft/emscripten-glfw/releases/download/v{opts["tag"]}/emscripten-glfw3-{opts["tag"]}.zip', + sha512hash=opts['hash']) def create(final): root_path = os.path.join(ports.get_dir(), 'contrib.glfw3') @@ -29,8 +50,16 @@ def create(final): for source_include_path in source_include_paths: ports.install_headers(source_include_path, target='GLFW') - # this should be an option but better to disable for now... - flags = ['-DEMSCRIPTEN_GLFW3_DISABLE_WARNING'] + flags = [] + + if opts['disableWarning']: + flags += ['-DEMSCRIPTEN_GLFW3_DISABLE_WARNING'] + + if opts['disableJoystick']: + flags += ['-DEMSCRIPTEN_GLFW3_DISABLE_JOYSTICK'] + + if opts['disableMultiWindow']: + flags += ['-DEMSCRIPTEN_GLFW3_DISABLE_MULTI_WINDOW_SUPPORT'] ports.build_port(source_path, final, 'contrib.glfw3', includes=source_include_paths, flags=flags) @@ -52,3 +81,27 @@ def linker_setup(ports, settings): # includes def process_args(ports): return ['-isystem', ports.get_include_dir('contrib.glfw3')] + + +def handle_options(options): + try: + oqs = parse_qs(options, strict_parsing=True) + except ValueError as error: + return f'{options} is not valid: {error}. Available options are {OPTIONS}.' + + if not set(oqs.keys()).issubset(OPTIONS.keys()): + return f'{options} is not valid. Available options are {OPTIONS}.' + + for option, values in oqs.items(): + value = values[-1] # ignore multiple definitions (last one wins) + if isinstance(opts[option], bool): + if value.lower() in {'true', 'false'}: + opts[option] = value.lower() == 'true' + else: + return f'{option} is expecting a boolean, got {value}' + else: + opts[option] = value + + # in the event that only 'tag' is provided, clear 'hash' + if 'tag' in oqs and not 'hash' in oqs: + opts['hash'] = None diff --git a/tools/ports/sdl2_image.py b/tools/ports/sdl2_image.py index f6029de06ad67..405b71bdd1686 100644 --- a/tools/ports/sdl2_image.py +++ b/tools/ports/sdl2_image.py @@ -3,7 +3,7 @@ # University of Illinois/NCSA Open Source License. Both these licenses can be # found in the LICENSE file. -import os +import os, re TAG = 'release-2.6.0' HASH = '2175d11a90211871f2289c8d57b31fe830e4b46af7361925c2c30cd521c1c677d2ee244feb682b6d3909cf085129255934751848fc81b480ea410952d990ffe0' @@ -14,14 +14,21 @@ 'sdl2_image_png': {'SDL2_IMAGE_FORMATS': ["png"]}, } +# user options (from --use-port) +opts = { + 'formats': set() +} def needed(settings): return settings.USE_SDL_IMAGE == 2 +def get_formats(settings): + return set(settings.SDL2_IMAGE_FORMATS).union(opts['formats']) + + def get_lib_name(settings): - settings.SDL2_IMAGE_FORMATS.sort() - formats = '-'.join(settings.SDL2_IMAGE_FORMATS) + formats = '-'.join(sorted(get_formats(settings))) libname = 'libSDL2_image' if formats != '': @@ -44,13 +51,15 @@ def create(final): defs = ['-O2', '-sUSE_SDL=2', '-Wno-format-security'] - for fmt in settings.SDL2_IMAGE_FORMATS: + formats = get_formats(settings) + + for fmt in formats: defs.append('-DLOAD_' + fmt.upper()) - if 'png' in settings.SDL2_IMAGE_FORMATS: + if 'png' in formats: defs += ['-sUSE_LIBPNG'] - if 'jpg' in settings.SDL2_IMAGE_FORMATS: + if 'jpg' in formats: defs += ['-sUSE_LIBJPEG'] ports.build_port(src_dir, final, 'sdl2_image', flags=defs, srcs=srcs) @@ -64,13 +73,23 @@ def clear(ports, settings, shared): def process_dependencies(settings): settings.USE_SDL = 2 - if 'png' in settings.SDL2_IMAGE_FORMATS: + formats = get_formats(settings) + if 'png' in formats: deps.append('libpng') settings.USE_LIBPNG = 1 - if 'jpg' in settings.SDL2_IMAGE_FORMATS: + if 'jpg' in formats: deps.append('libjpeg') settings.USE_LIBJPEG = 1 +def handle_options(options): + if options.startswith('formats='): + options = options.split('=', 1)[1] + opts['formats'] = {format.lower() for format in re.findall(r'\b\w+\b', options)} + else: + return f'{options} is not supported (syntax is --use-port=sdl2_image?formats=[x,y,z])' + return None + + def show(): return 'sdl2_image (-sUSE_SDL_IMAGE=2 or --use-port=sdl2_image; zlib license)' From 0e7d07c854739c38375f3b16662a0d0e7c155613 Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Tue, 6 Feb 2024 10:16:59 -0800 Subject: [PATCH 02/11] fixed flake 8 errors (unclear about syntax error) --- tools/ports/__init__.py | 6 ++++-- tools/ports/contrib/glfw3.py | 2 +- tools/ports/sdl2_image.py | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tools/ports/__init__.py b/tools/ports/__init__.py index fdcc1d1d2458b..10e8c24a7e74d 100644 --- a/tools/ports/__init__.py +++ b/tools/ports/__init__.py @@ -403,8 +403,10 @@ def handle_use_port_arg(settings, arg): port = ports_by_name[name] if not hasattr(port, 'handle_options'): utils.exit_with_error(f'Invalid options for port {name}: No options available') - elif (error := port.handle_options(options)) is not None: - utils.exit_with_error(f'Invalid options for port {name}: {error}') + else: + error = port.handle_options(options) + if error is not None: + utils.exit_with_error(f'Invalid options for port {name}: {error}') def get_needed_ports(settings): diff --git a/tools/ports/contrib/glfw3.py b/tools/ports/contrib/glfw3.py index 3a657a214312c..3793c4f2e94b0 100644 --- a/tools/ports/contrib/glfw3.py +++ b/tools/ports/contrib/glfw3.py @@ -103,5 +103,5 @@ def handle_options(options): opts[option] = value # in the event that only 'tag' is provided, clear 'hash' - if 'tag' in oqs and not 'hash' in oqs: + if 'tag' in oqs and 'hash' not in oqs: opts['hash'] = None diff --git a/tools/ports/sdl2_image.py b/tools/ports/sdl2_image.py index 405b71bdd1686..ba7eea8ee061b 100644 --- a/tools/ports/sdl2_image.py +++ b/tools/ports/sdl2_image.py @@ -3,7 +3,8 @@ # University of Illinois/NCSA Open Source License. Both these licenses can be # found in the LICENSE file. -import os, re +import os +import re TAG = 'release-2.6.0' HASH = '2175d11a90211871f2289c8d57b31fe830e4b46af7361925c2c30cd521c1c677d2ee244feb682b6d3909cf085129255934751848fc81b480ea410952d990ffe0' @@ -19,6 +20,7 @@ 'formats': set() } + def needed(settings): return settings.USE_SDL_IMAGE == 2 From 96e569eecb64f640a67210f0e4d34f837806d67e Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Tue, 6 Feb 2024 11:39:03 -0800 Subject: [PATCH 03/11] code review --- test/test_browser.py | 2 +- tools/ports/__init__.py | 13 +++++++++--- tools/ports/contrib/glfw3.py | 41 ++++++++++++------------------------ tools/ports/sdl2_image.py | 16 ++++++++------ 4 files changed, 33 insertions(+), 39 deletions(-) diff --git a/test/test_browser.py b/test/test_browser.py index 3bdd3b947399d..f626b7bd41173 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -3025,7 +3025,7 @@ def test_sdl2_image_formats(self): self.btest_exit('test_sdl2_image.c', 600, args=[ '--preload-file', 'screenshot.jpg', '-DSCREENSHOT_DIRNAME="/"', '-DSCREENSHOT_BASENAME="screenshot.jpg"', '-DBITSPERPIXEL=24', '-DNO_PRELOADED', - '--use-port=sdl2', '--use-port=sdl2_image?formats=["jpg"]' + '--use-port=sdl2', '--use-port=sdl2_image?formats=jpg' ]) @no_wasm64('SDL2 + wasm64') diff --git a/tools/ports/__init__.py b/tools/ports/__init__.py index 10e8c24a7e74d..8f43ab3b6a6e3 100644 --- a/tools/ports/__init__.py +++ b/tools/ports/__init__.py @@ -15,6 +15,7 @@ from tools import system_libs from tools import utils from tools.settings import settings +from urllib.parse import parse_qs from tools.toolchain_profiler import ToolchainProfiler @@ -68,6 +69,8 @@ def validate_port(port): expected_attrs = ['get', 'clear', 'show'] if port.is_contrib: expected_attrs += ['URL', 'DESCRIPTION', 'LICENSE'] + if hasattr(port, 'handle_options'): + expected_attrs += ['OPTIONS'] for a in expected_attrs: assert hasattr(port, a), 'port %s is missing %s' % (port, a) @@ -404,9 +407,13 @@ def handle_use_port_arg(settings, arg): if not hasattr(port, 'handle_options'): utils.exit_with_error(f'Invalid options for port {name}: No options available') else: - error = port.handle_options(options) - if error is not None: - utils.exit_with_error(f'Invalid options for port {name}: {error}') + try: + options_qs = parse_qs(options, strict_parsing=True) + except ValueError as error: + utils.exit_with_error(f'{options} is not valid: {error}. Available options are {port.OPTIONS}.') + if not set(options_qs.keys()).issubset(port.OPTIONS.keys()): + utils.exit_with_error(f'{options} is not valid. Available options are {port.OPTIONS}.') + port.handle_options(options_qs) def get_needed_ports(settings): diff --git a/tools/ports/contrib/glfw3.py b/tools/ports/contrib/glfw3.py index 3793c4f2e94b0..4addf2e7f8c03 100644 --- a/tools/ports/contrib/glfw3.py +++ b/tools/ports/contrib/glfw3.py @@ -4,7 +4,11 @@ # found in the LICENSE file. import os -from urllib.parse import parse_qs +from tools import utils +from typing import Dict + +TAG = '1.0.4' +HASH = 'c3c96718e5d2b37df434a46c4a93ddfd9a768330d33f0d6ce2d08c139752894c2421cdd0fefb800fe41fafc2bbe58c8f22b8aa2849dc4fc6dde686037215cfad' # contrib port information (required) URL = 'https://github.com/pongasoft/emscripten-glfw' @@ -12,17 +16,13 @@ LICENSE = 'Apache 2.0 license' OPTIONS = { - 'tag': 'The git tag to use a different release', - 'hash': 'The sha512 of the release associated to the tag (can be omitted)', 'disableWarning': 'Boolean to disable warnings emitted by the library', 'disableJoystick': 'Boolean to disable support for joystick entirely', 'disableMultiWindow': 'Boolean to disable multi window support which makes the code smaller and faster' } # user options (from --use-port) -opts = { - 'tag': '1.0.4', - 'hash': 'c3c96718e5d2b37df434a46c4a93ddfd9a768330d33f0d6ce2d08c139752894c2421cdd0fefb800fe41fafc2bbe58c8f22b8aa2849dc4fc6dde686037215cfad', +opts: Dict[str, bool] = { 'disableWarning': False, 'disableJoystick': False, 'disableMultiWindow': False @@ -30,7 +30,7 @@ def get_lib_name(settings): - return (f'lib_contrib.glfw3_{opts["tag"]}' + + return (f'lib_contrib.glfw3' + ('-nw' if opts['disableWarning'] else '') + ('-nj' if opts['disableJoystick'] else '') + ('-sw' if opts['disableMultiWindow'] else '') + @@ -40,8 +40,8 @@ def get_lib_name(settings): def get(ports, settings, shared): # get the port ports.fetch_project('contrib.glfw3', - f'https://github.com/pongasoft/emscripten-glfw/releases/download/v{opts["tag"]}/emscripten-glfw3-{opts["tag"]}.zip', - sha512hash=opts['hash']) + f'https://github.com/pongasoft/emscripten-glfw/releases/download/v{TAG}/emscripten-glfw3-{TAG}.zip', + sha512hash=HASH) def create(final): root_path = os.path.join(ports.get_dir(), 'contrib.glfw3') @@ -84,24 +84,9 @@ def process_args(ports): def handle_options(options): - try: - oqs = parse_qs(options, strict_parsing=True) - except ValueError as error: - return f'{options} is not valid: {error}. Available options are {OPTIONS}.' - - if not set(oqs.keys()).issubset(OPTIONS.keys()): - return f'{options} is not valid. Available options are {OPTIONS}.' - - for option, values in oqs.items(): + for option, values in options.items(): value = values[-1] # ignore multiple definitions (last one wins) - if isinstance(opts[option], bool): - if value.lower() in {'true', 'false'}: - opts[option] = value.lower() == 'true' - else: - return f'{option} is expecting a boolean, got {value}' + if value.lower() in {'true', 'false'}: + opts[option] = value.lower() == 'true' else: - opts[option] = value - - # in the event that only 'tag' is provided, clear 'hash' - if 'tag' in oqs and 'hash' not in oqs: - opts['hash'] = None + utils.exit_with_error(f'{option} is expecting a boolean, got {value}') diff --git a/tools/ports/sdl2_image.py b/tools/ports/sdl2_image.py index ba7eea8ee061b..992a41adecf14 100644 --- a/tools/ports/sdl2_image.py +++ b/tools/ports/sdl2_image.py @@ -5,6 +5,7 @@ import os import re +from typing import Dict, Set TAG = 'release-2.6.0' HASH = '2175d11a90211871f2289c8d57b31fe830e4b46af7361925c2c30cd521c1c677d2ee244feb682b6d3909cf085129255934751848fc81b480ea410952d990ffe0' @@ -15,8 +16,12 @@ 'sdl2_image_png': {'SDL2_IMAGE_FORMATS': ["png"]}, } +OPTIONS = { + 'formats': 'A comma separated list of formats (ex: png,jpg)' +} + # user options (from --use-port) -opts = { +opts: Dict[str, Set] = { 'formats': set() } @@ -85,12 +90,9 @@ def process_dependencies(settings): def handle_options(options): - if options.startswith('formats='): - options = options.split('=', 1)[1] - opts['formats'] = {format.lower() for format in re.findall(r'\b\w+\b', options)} - else: - return f'{options} is not supported (syntax is --use-port=sdl2_image?formats=[x,y,z])' - return None + # options has been parsed from a query string + for fmts in options['formats']: + opts['formats'].update({format.lower() for format in re.findall(r'\b\w+\b', fmts)}) def show(): From 26860a11942bd6547134babc2455e2fe93bdfd2b Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Tue, 6 Feb 2024 11:42:33 -0800 Subject: [PATCH 04/11] flake8 --- tools/ports/contrib/glfw3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ports/contrib/glfw3.py b/tools/ports/contrib/glfw3.py index 4addf2e7f8c03..1ffafa9e58884 100644 --- a/tools/ports/contrib/glfw3.py +++ b/tools/ports/contrib/glfw3.py @@ -30,7 +30,7 @@ def get_lib_name(settings): - return (f'lib_contrib.glfw3' + + return ('lib_contrib.glfw3' + ('-nw' if opts['disableWarning'] else '') + ('-nj' if opts['disableJoystick'] else '') + ('-sw' if opts['disableMultiWindow'] else '') + From 2e192d4c21cb4eb536ca8406e0d23bde1425bf47 Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Tue, 6 Feb 2024 13:01:40 -0800 Subject: [PATCH 05/11] trying to fix "FROZEN_CACHE is set" error... --- test/test_other.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_other.py b/test/test_other.py index 978e53860c969..9ce0f192b42b9 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -2372,7 +2372,7 @@ def test_sdl2_ttf(self): def test_contrib_ports(self): # Verify that contrib ports can be used (using the only contrib port available ATM, but can be replaced # with a different contrib port when there is another one - self.emcc(test_file('other/test_contrib_ports.cpp'), ['--use-port=contrib.glfw3?disableWarning=true']) + self.emcc(test_file('other/test_contrib_ports.cpp'), ['--use-port=contrib.glfw3?disableWarning=true'], output_filename='a.out.js') 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 From ea4ea494c19ac2437ffeefd4f851397f0747aaf6 Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Tue, 6 Feb 2024 14:19:22 -0800 Subject: [PATCH 06/11] more code review --- test/test_other.py | 2 +- tools/ports/__init__.py | 4 +++- tools/ports/sdl2_image.py | 5 ++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index 9ce0f192b42b9..aba5e0e557c4a 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -2372,7 +2372,7 @@ def test_sdl2_ttf(self): def test_contrib_ports(self): # Verify that contrib ports can be used (using the only contrib port available ATM, but can be replaced # with a different contrib port when there is another one - self.emcc(test_file('other/test_contrib_ports.cpp'), ['--use-port=contrib.glfw3?disableWarning=true'], output_filename='a.out.js') + self.emcc(test_file('other/test_contrib_ports.cpp'), ['--use-port=contrib.glfw3']) 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 diff --git a/tools/ports/__init__.py b/tools/ports/__init__.py index 8f43ab3b6a6e3..660a60c727f6f 100644 --- a/tools/ports/__init__.py +++ b/tools/ports/__init__.py @@ -398,7 +398,9 @@ def add_deps(node): def handle_use_port_arg(settings, arg): args = arg.split('?', 1) - name, options = args[0], args[1] if len(args) == 2 else None + name, options = args[0], None + if len(args) == 2: + options = args[1] if name not in ports_by_name: utils.exit_with_error(f'Invalid port name: {name} used with --use-port') ports_needed.add(name) diff --git a/tools/ports/sdl2_image.py b/tools/ports/sdl2_image.py index 992a41adecf14..f16342189e7f2 100644 --- a/tools/ports/sdl2_image.py +++ b/tools/ports/sdl2_image.py @@ -4,7 +4,6 @@ # found in the LICENSE file. import os -import re from typing import Dict, Set TAG = 'release-2.6.0' @@ -17,7 +16,7 @@ } OPTIONS = { - 'formats': 'A comma separated list of formats (ex: png,jpg)' + 'formats': 'A comma separated list of formats (ex: --use-port=sdl2_image?formats=png,jpg)' } # user options (from --use-port) @@ -92,7 +91,7 @@ def process_dependencies(settings): def handle_options(options): # options has been parsed from a query string for fmts in options['formats']: - opts['formats'].update({format.lower() for format in re.findall(r'\b\w+\b', fmts)}) + opts['formats'].update({format.lower().strip() for format in fmts.split(',')}) def show(): From c9d15e69558e2f8091817982bf42adfeff12962b Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Wed, 7 Feb 2024 12:57:43 -0800 Subject: [PATCH 07/11] changed syntax to avoid shell character conflicts --- test/test_browser.py | 2 +- tools/ports/__init__.py | 4 ++-- tools/ports/sdl2_image.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test_browser.py b/test/test_browser.py index f626b7bd41173..73f08e1b4184d 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -3025,7 +3025,7 @@ def test_sdl2_image_formats(self): self.btest_exit('test_sdl2_image.c', 600, args=[ '--preload-file', 'screenshot.jpg', '-DSCREENSHOT_DIRNAME="/"', '-DSCREENSHOT_BASENAME="screenshot.jpg"', '-DBITSPERPIXEL=24', '-DNO_PRELOADED', - '--use-port=sdl2', '--use-port=sdl2_image?formats=jpg' + '--use-port=sdl2', '--use-port=sdl2_image@formats=jpg' ]) @no_wasm64('SDL2 + wasm64') diff --git a/tools/ports/__init__.py b/tools/ports/__init__.py index 660a60c727f6f..18fe97e5502d0 100644 --- a/tools/ports/__init__.py +++ b/tools/ports/__init__.py @@ -397,7 +397,7 @@ def add_deps(node): def handle_use_port_arg(settings, arg): - args = arg.split('?', 1) + args = arg.split('@', 1) name, options = args[0], None if len(args) == 2: options = args[1] @@ -410,7 +410,7 @@ def handle_use_port_arg(settings, arg): utils.exit_with_error(f'Invalid options for port {name}: No options available') else: try: - options_qs = parse_qs(options, strict_parsing=True) + options_qs = parse_qs(options, strict_parsing=True, separator=':') except ValueError as error: utils.exit_with_error(f'{options} is not valid: {error}. Available options are {port.OPTIONS}.') if not set(options_qs.keys()).issubset(port.OPTIONS.keys()): diff --git a/tools/ports/sdl2_image.py b/tools/ports/sdl2_image.py index f16342189e7f2..14f55c346936c 100644 --- a/tools/ports/sdl2_image.py +++ b/tools/ports/sdl2_image.py @@ -16,7 +16,7 @@ } OPTIONS = { - 'formats': 'A comma separated list of formats (ex: --use-port=sdl2_image?formats=png,jpg)' + 'formats': 'A comma separated list of formats (ex: --use-port=sdl2_image@formats=png,jpg)' } # user options (from --use-port) From 4a8edc46a2f0ab8e464f34c5d20fd988c0f92629 Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Wed, 7 Feb 2024 13:19:50 -0800 Subject: [PATCH 08/11] changed syntax --- test/test_browser.py | 2 +- tools/ports/__init__.py | 2 +- tools/ports/sdl2_image.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_browser.py b/test/test_browser.py index 4fb8c7ed28679..3b69a42e762bc 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -3024,7 +3024,7 @@ def test_sdl2_image_formats(self): self.btest_exit('test_sdl2_image.c', 600, args=[ '--preload-file', 'screenshot.jpg', '-DSCREENSHOT_DIRNAME="/"', '-DSCREENSHOT_BASENAME="screenshot.jpg"', '-DBITSPERPIXEL=24', '-DNO_PRELOADED', - '--use-port=sdl2', '--use-port=sdl2_image@formats=jpg' + '--use-port=sdl2', '--use-port=sdl2_image:formats=jpg' ]) @no_wasm64('SDL2 + wasm64') diff --git a/tools/ports/__init__.py b/tools/ports/__init__.py index 18fe97e5502d0..cef497fa67c6e 100644 --- a/tools/ports/__init__.py +++ b/tools/ports/__init__.py @@ -397,7 +397,7 @@ def add_deps(node): def handle_use_port_arg(settings, arg): - args = arg.split('@', 1) + args = arg.split(':', 1) name, options = args[0], None if len(args) == 2: options = args[1] diff --git a/tools/ports/sdl2_image.py b/tools/ports/sdl2_image.py index 14f55c346936c..db341a8c33567 100644 --- a/tools/ports/sdl2_image.py +++ b/tools/ports/sdl2_image.py @@ -16,7 +16,7 @@ } OPTIONS = { - 'formats': 'A comma separated list of formats (ex: --use-port=sdl2_image@formats=png,jpg)' + 'formats': 'A comma separated list of formats (ex: --use-port=sdl2_image:formats=png,jpg)' } # user options (from --use-port) From d0a9375b0f685cc5521dc57821b87092a2f311dc Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Thu, 8 Feb 2024 07:17:46 -0800 Subject: [PATCH 09/11] Updated documentation and changelog --- ChangeLog.md | 6 ++-- .../docs/compiling/Building-Projects.rst | 27 +++++++++++++-- tools/ports/contrib/README.md | 34 +++++++++++++++++++ 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index d7637df843cd1..8ce86f8021de0 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -37,9 +37,11 @@ See docs/process.md for more on how version tagging works. POINTER_SIZE }}}` and `{{{ makeGetValue(..) }}}` to be used in pre/post JS files, just like they can be in JS library files. (#21227) - Added concept of contrib ports which are ports contributed by the wider - community and supported on a "best effort" basis. A first contrib port is + community and supported on a "best effort" basis. See + `tools/ports/contrib/README.md` for details.A first contrib port is available via `--use-port=contrib.glfw3`: an emscripten port of glfw written - in C++ with many features like support for multiple windows. (#21244) + in C++ with many features like support for multiple windows. (#21244 and + #21276) 3.1.53 - 01/29/24 diff --git a/site/source/docs/compiling/Building-Projects.rst b/site/source/docs/compiling/Building-Projects.rst index 46a306f14d613..eddd406aae396 100644 --- a/site/source/docs/compiling/Building-Projects.rst +++ b/site/source/docs/compiling/Building-Projects.rst @@ -225,11 +225,30 @@ You should see some notifications about SDL2 being used, and built if it wasn't To see a list of all available ports, run ``emcc --show-ports``. -.. note:: *SDL_image* has also been added to ports, use it with ``--use-port=sdl2_image``. For ``sdl2_image`` to be useful, you generally need to specify the image formats you are planning on using with e.g. ``-sSDL2_IMAGE_FORMATS='["bmp","png","xpm","jpg"]'``. This will also ensure that ``IMG_Init`` works properly when you specify those formats. Alternatively, you can use ``emcc --use-preload-plugins`` and ``--preload-file`` your images, so the browser codecs decode them (see :ref:`preloading-files`). A code path in the ``sdl2_image`` port will load through :c:func:`emscripten_get_preloaded_image_data`, but then your calls to ``IMG_Init`` with those image formats will fail (as while the images will work through preloading, IMG_Init reports no support for those formats, as it doesn't have support compiled in - in other words, IMG_Init does not report support for formats that only work through preloading).``` +.. note:: *SDL_image* has also been added to ports, use it with + ``--use-port=sdl2_image``. For ``sdl2_image`` to be useful, you generally + need to specify the image formats you are planning on using with e.g. + ``--use-port=sdl2_image:formats=bmp,png,xpm,jpg``. This will also ensure that + ``IMG_Init`` works properly when you specify those formats. Alternatively, + you can use ``emcc --use-preload-plugins`` and ``--preload-file`` your + images, so the browser codecs decode them (see :ref:`preloading-files`). + A code path in the ``sdl2_image`` port will load through + :c:func:`emscripten_get_preloaded_image_data`, but then your calls to + ``IMG_Init`` with those image formats will fail (as while the images will + work through preloading, IMG_Init reports no support for those formats, as + it doesn't have support compiled in - in other words, ``IMG_Init`` does not + report support for formats that only work through preloading). .. note:: *SDL_net* has also been added to ports, use it with ``--use-port=sdl2_net``. -.. note:: Emscripten also has support for older SDL1, which is built-in. If you do not specify SDL2 as in the command above, then SDL1 is linked in and the SDL1 include paths are used. SDL1 has support for *sdl-config*, which is present in `system/bin `_. Using the native *sdl-config* may result in compilation or missing-symbol errors. You will need to modify the build system to look for files in **emscripten/system** or **emscripten/system/bin** in order to use the Emscripten *sdl-config*. +.. note:: Emscripten also has support for older SDL1, which is built-in. + If you do not specify SDL2 as in the command above, then SDL1 is linked in + and the SDL1 include paths are used. SDL1 has support for *sdl-config*, + which is present in `system/bin `_. + Using the native *sdl-config* may result in compilation or missing-symbol errors. + You will need to modify the build system to look for files in + **emscripten/system** or **emscripten/system/bin** in order to use the + Emscripten *sdl-config*. .. note:: You can also build a library from ports in a manual way if you prefer that, but then you will need to also apply the python logic that ports does. @@ -238,7 +257,9 @@ To see a list of all available ports, run ``emcc --show-ports``. it's better to use the ports version as it is what is tested and known to work. -.. note:: Since emscripten 3.1.54, ``--use-port`` is the preferred syntax to use a port in your project. The legacy syntax (for example ``-sUSE_SDL2``, ``-sUSE_SDL_IMAGE=2``) remains available. +.. note:: Since emscripten 3.1.54, ``--use-port`` is the preferred syntax to + use a port in your project. The legacy syntax (for example ``-sUSE_SDL2``, + ``-sUSE_SDL_IMAGE=2``) remains available. Contrib ports diff --git a/tools/ports/contrib/README.md b/tools/ports/contrib/README.md index f5b3b40d3a2a3..03f1143265d25 100644 --- a/tools/ports/contrib/README.md +++ b/tools/ports/contrib/README.md @@ -15,5 +15,39 @@ of information: is about * `LICENSE`: the license used by the project/port +A contrib port can have options using the syntax +`--use-port=name:opt1=v1:opt2=v2`. + +If you want to support options, then your port needs to provide 2 +additional components: + +1. A handler function defined this way: +```python +def handle_options(options): + # options is of type Dict[str, List[str]] (result of calling urllib.parse.parse_qs) + # in case of error, use utils.exit_with_error('error message') +``` +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 +failure. + +Options are interpreted as a query string, but with a distinction: the `:` +character is used as the separator instead of the more commonly used `&` (so +that it doesn't interfere when used in a shell environment). +This process automatically manages escape sequences and repeated options: + +Example: `--use-port=contrib.foo:bar=1:bar=2%263` => calls +`tools.ports.contrib.foo.handle_options({'bar': ['1', '2&3']})` + +> ### Note +> If the options influence the way the library produced by the port is built, +> you must ensure that the library name accounts for these options. Check +> `glfw3.py` for an example of ports with options. + After adding a contrib port, you should consider modifying the documentation under `site/source/docs/compiling/Contrib-Ports.rst`. \ No newline at end of file From 118922fa270a630d96fbb2abbf666574a5f66ee9 Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Thu, 8 Feb 2024 10:34:20 -0800 Subject: [PATCH 10/11] simplified port option syntax (not a query string) --- test/test_other.py | 18 ++++++++++++++++++ tools/ports/__init__.py | 27 +++++++++++++++++---------- tools/ports/contrib/README.md | 10 +--------- tools/ports/contrib/glfw3.py | 3 +-- tools/ports/sdl2_image.py | 4 +--- 5 files changed, 38 insertions(+), 24 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index 2492ea605da4c..80a6abfcc0f93 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -14513,3 +14513,21 @@ def test_js_preprocess_pre_post(self): self.do_runf(test_file('hello_world.c'), 'assertions enabled\n4', emcc_args=['-sASSERTIONS=1']) self.do_runf(test_file('hello_world.c'), 'assertions disabled\n4', emcc_args=['-sASSERTIONS=0']) self.assertNotContained('#preprocess', read_file('hello_world.js')) + + @with_both_compilers + 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) + 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) + 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) + 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) + 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) diff --git a/tools/ports/__init__.py b/tools/ports/__init__.py index cef497fa67c6e..dd84aebb3cdc0 100644 --- a/tools/ports/__init__.py +++ b/tools/ports/__init__.py @@ -15,7 +15,6 @@ from tools import system_libs from tools import utils from tools.settings import settings -from urllib.parse import parse_qs from tools.toolchain_profiler import ToolchainProfiler @@ -396,26 +395,34 @@ def add_deps(node): add_deps(port) +def handle_use_port_error(arg, message): + utils.exit_with_error(f'Error with --use-port={arg} | {message}') + + def handle_use_port_arg(settings, arg): args = arg.split(':', 1) name, options = args[0], None if len(args) == 2: options = args[1] if name not in ports_by_name: - utils.exit_with_error(f'Invalid port name: {name} used with --use-port') + handle_use_port_error(arg, f'invalid port name: {name}') ports_needed.add(name) if options: port = ports_by_name[name] if not hasattr(port, 'handle_options'): - utils.exit_with_error(f'Invalid options for port {name}: No options available') + handle_use_port_error(arg, f'no options available for port {name}') else: - try: - options_qs = parse_qs(options, strict_parsing=True, separator=':') - except ValueError as error: - utils.exit_with_error(f'{options} is not valid: {error}. Available options are {port.OPTIONS}.') - if not set(options_qs.keys()).issubset(port.OPTIONS.keys()): - utils.exit_with_error(f'{options} is not valid. Available options are {port.OPTIONS}.') - port.handle_options(options_qs) + 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') + if nv[0] not in port.OPTIONS: + handle_use_port_error(arg, 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]}') + options_dict[nv[0]] = nv[1] + port.handle_options(options_dict) def get_needed_ports(settings): diff --git a/tools/ports/contrib/README.md b/tools/ports/contrib/README.md index 03f1143265d25..670b53cbd90ab 100644 --- a/tools/ports/contrib/README.md +++ b/tools/ports/contrib/README.md @@ -24,7 +24,7 @@ additional components: 1. A handler function defined this way: ```python def handle_options(options): - # options is of type Dict[str, List[str]] (result of calling urllib.parse.parse_qs) + # options is of type Dict[str, str] # in case of error, use utils.exit_with_error('error message') ``` 2. A dictionary called `OPTIONS` (type `Dict[str, str]`) where each key is the @@ -36,14 +36,6 @@ 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 failure. -Options are interpreted as a query string, but with a distinction: the `:` -character is used as the separator instead of the more commonly used `&` (so -that it doesn't interfere when used in a shell environment). -This process automatically manages escape sequences and repeated options: - -Example: `--use-port=contrib.foo:bar=1:bar=2%263` => calls -`tools.ports.contrib.foo.handle_options({'bar': ['1', '2&3']})` - > ### Note > If the options influence the way the library produced by the port is built, > you must ensure that the library name accounts for these options. Check diff --git a/tools/ports/contrib/glfw3.py b/tools/ports/contrib/glfw3.py index 1ffafa9e58884..7e83fdd301280 100644 --- a/tools/ports/contrib/glfw3.py +++ b/tools/ports/contrib/glfw3.py @@ -84,8 +84,7 @@ def process_args(ports): def handle_options(options): - for option, values in options.items(): - value = values[-1] # ignore multiple definitions (last one wins) + for option, value in options.items(): if value.lower() in {'true', 'false'}: opts[option] = value.lower() == 'true' else: diff --git a/tools/ports/sdl2_image.py b/tools/ports/sdl2_image.py index db341a8c33567..0adf74a0e5660 100644 --- a/tools/ports/sdl2_image.py +++ b/tools/ports/sdl2_image.py @@ -89,9 +89,7 @@ def process_dependencies(settings): def handle_options(options): - # options has been parsed from a query string - for fmts in options['formats']: - opts['formats'].update({format.lower().strip() for format in fmts.split(',')}) + opts['formats'].update({format.lower().strip() for format in options['formats'].split(',')}) def show(): From 29cf1882c869eae03182d1b018ac60ee7880782b Mon Sep 17 00:00:00 2001 From: Yan Pujante Date: Thu, 8 Feb 2024 10:59:07 -0800 Subject: [PATCH 11/11] removed unnecessary [] --- test/test_other.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/test_other.py b/test/test_other.py index 16dabe3487969..3eca9b8dd5bc2 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -14520,16 +14520,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)