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 contrib ports (Contrib ports part 2) #21244

Merged
merged 35 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
19f0583
added PORTS new settings
ypujante Jan 30, 2024
d135cd4
check for valid port name
ypujante Jan 30, 2024
27db0bd
Merge remote-tracking branch 'origin/main' into contrib-ports-1
ypujante Jan 30, 2024
831c523
use --use-port instead of -sPORTS
ypujante Jan 31, 2024
0bfdef3
Merge remote-tracking branch 'origin/main' into contrib-ports-1
ypujante Jan 31, 2024
177d603
removed PORTS from settings.py
ypujante Jan 31, 2024
7d43831
reverted comma
ypujante Jan 31, 2024
5d0afbb
changes from code review
ypujante Jan 31, 2024
f6694fe
Merge remote-tracking branch 'origin/main' into contrib-ports-1
ypujante Jan 31, 2024
ba2a1ff
removed print statement
ypujante Jan 31, 2024
b88e332
brute force documentation update
ypujante Jan 31, 2024
aeb2d0a
use compile+link
ypujante Jan 31, 2024
9facf38
fixed sanity checks
ypujante Feb 1, 2024
6717313
Merge remote-tracking branch 'origin/main' into contrib-ports-1
ypujante Feb 1, 2024
0344a99
documentation changes
ypujante Feb 1, 2024
2be2672
first pass at adding contrib ports
ypujante Feb 1, 2024
d7a3b45
added new documentation
ypujante Feb 2, 2024
dc92707
Merge remote-tracking branch 'origin/main' into contrib-ports-2
ypujante Feb 2, 2024
b028587
Code review
ypujante Feb 3, 2024
4f9e599
reverting automatic documentation
ypujante Feb 3, 2024
51ae73d
Merge remote-tracking branch 'origin/main' into contrib-ports-2
ypujante Feb 3, 2024
ec050d1
80 columns
ypujante Feb 3, 2024
033e0ba
removed comment
ypujante Feb 3, 2024
c5d2f99
Fixed documentation
ypujante Feb 3, 2024
db88e52
fix documentation
ypujante Feb 3, 2024
3d727b9
Added changelog + tweaks
ypujante Feb 4, 2024
5a0b668
updated readme accordingly
ypujante Feb 4, 2024
124d52b
removed blank line
ypujante Feb 4, 2024
22a392a
code review
ypujante Feb 5, 2024
93c5ea7
removed unnecessary comment
ypujante Feb 5, 2024
0953955
2 lines...
ypujante Feb 5, 2024
21a2839
removed key variable
ypujante Feb 5, 2024
44758b0
Merge remote-tracking branch 'origin/main' into contrib-ports-2
ypujante Feb 5, 2024
3df103f
code review
ypujante Feb 5, 2024
a1512be
Merge remote-tracking branch 'origin/main' into contrib-ports-2
ypujante Feb 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ See docs/process.md for more on how version tagging works.
the top of the JS file. This is useful as it allows things like `{{{
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
available via `--use-port=contrib.glfw3`: an emscripten port of glfw written
in C++ with many features like support for multiple windows. (#21244)


3.1.53 - 01/29/24
-----------------
Expand Down
3 changes: 2 additions & 1 deletion embuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ def get_help():
return '''
Available targets:

build / clear %s
build / clear
%s

Issuing 'embuilder build ALL' causes each task to be built.
''' % '\n '.join(all_tasks)
Expand Down
14 changes: 10 additions & 4 deletions site/source/docs/compiling/Building-Projects.rst
Original file line number Diff line number Diff line change
Expand Up @@ -241,15 +241,21 @@ To see a list of all available ports, run ``emcc --show-ports``.
.. 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
-------------

Contrib ports are contributed by the wider community and supported on a
"best effort" basis. Since they are not run as part of emscripten CI they are
not always guaranteed to build or function.
See :ref:`Contrib Ports <Contrib-Ports>` for more information.

Adding more ports
-----------------

Adding more ports is fairly easy. Basically, the steps are
The simplest way to add a new port is to put it under the ``contrib`` directory. Basically, the steps are:

* Make sure the port is open source and has a suitable license.
* Add it to emscripten-ports on github. The ports maintainers can create the repo and add the relevant developers to a team for that repo, so they have write access.
* Add a script to handle it under ``tools/ports/`` (see existing code for examples) and use it in ``tools/ports/__init__.py``.
* Add testing in the test suite.
* Read the ``README.md`` file under ``tools/ports/contrib`` which contains more information.


Build system issues
Expand Down
24 changes: 24 additions & 0 deletions site/source/docs/compiling/Contrib-Ports.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
.. _Contrib-Ports:

========================
Emscripten Contrib Ports
========================

Contrib ports are contributed by the wider community and
supported on a "best effort" basis. Since they are not run as part
of emscripten CI they are not always guaranteed to build or function.

The following is the complete list of the contrib ports that are
available in emscripten. In order to use a contrib port you use the
``--use-port=<port_name>`` option (:ref:`emcc <emcc-use-port>`).

.. _contrib.glfw3:

contrib.glfw3
=============

This project is an emscripten port of glfw written in C++ for the web/webassembly platform

`Project information <https://github.com/pongasoft/emscripten-glfw>`_

License: Apache 2.0 license
2 changes: 2 additions & 0 deletions site/source/docs/compiling/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ This section contains topics about building projects and running the output.
- :ref:`Running-html-files-with-emrun` explains how to use *emrun* to run generated HTML pages in a locally launched web server.
- :ref:`Deploying-Pages` covers topics related to hosting Emscripten compiled web pages on a CDN.
- :ref:`GitLab` explains how to build and test projects on GitLab.
- :ref:`Contrib-Ports` contains information about contrib ports.


.. toctree::
Expand All @@ -22,3 +23,4 @@ This section contains topics about building projects and running the output.
Running-html-files-with-emrun
Deploying-Pages
GitLab
Contrib-Ports
25 changes: 25 additions & 0 deletions test/other/test_contrib_ports.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright 2024 The Emscripten Authors. All rights reserved.
* Emscripten is available under two separate licenses, the MIT license and the
* University of Illinois/NCSA Open Source License. Both these licenses can be
* found in the LICENSE file.
*/

#include <GLFW/glfw3.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this header any different the emscirpten one?

If not perhaps we could just install emscripten_glfw3.h and rely on the existing header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this it is the same or not. But the idea is that the port is self contained and brings its own dependencies. If it were to diverge then that would become an issue. I would rather keep it that way. Note that this test_contrib_ports.cpp will be replaced with another port once there is another one (when/if I work on libarchive I will switch it to this port)

#include <GLFW/emscripten_glfw3.h>
#include <assert.h>

// cpp otherwise it fails to link
int main() {

assert(glfwInit() == GLFW_TRUE);

GLFWwindow* window = glfwCreateWindow(320, 200, "test_glfw3_port", 0, 0);
assert(window != 0);
// this call ensures that it uses the right port
assert(emscripten_glfw_is_window_fullscreen(window) == EM_FALSE);
glfwTerminate();


return 0;
}
5 changes: 5 additions & 0 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -2369,6 +2369,11 @@ def test_sdl2_ttf(self):
self.emcc(test_file('browser/test_sdl2_ttf.c'), args=['-sUSE_SDL=2', '-sUSE_SDL_TTF=2'], output_filename='a.out.js')
self.emcc(test_file('browser/test_sdl2_ttf.c'), args=['--use-port=sdl2', '--use-port=sdl2_ttf'], output_filename='a.out.js')

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'])

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
create_file('main.c', r'''
Expand Down
2 changes: 1 addition & 1 deletion test/test_sanity.py
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ def test_emcc_ports(self):

# listing ports
out = self.do([EMCC, '--show-ports'])
self.assertContained('Available ports:', out)
self.assertContained('Available official ports:', out)
self.assertContained('sdl2', out)
self.assertContained('sdl2_image', out)
self.assertContained('sdl2_net', out)
Expand Down
4 changes: 2 additions & 2 deletions tools/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -2941,6 +2941,8 @@ def run(linker_inputs, options, state, newargs):
logger.debug('stopping after linking to object file')
return 0

phase_calculate_system_libraries(state, linker_arguments, newargs)

js_syms = {}
if (not settings.SIDE_MODULE or settings.ASYNCIFY) and not shared.SKIP_SUBPROCS:
js_info = get_js_sym_info()
Expand All @@ -2963,8 +2965,6 @@ def add_js_deps(sym):
settings.ASYNCIFY_IMPORTS_EXCEPT_JS_LIBS = settings.ASYNCIFY_IMPORTS[:]
settings.ASYNCIFY_IMPORTS += ['*.' + x for x in js_info['asyncFuncs']]

phase_calculate_system_libraries(state, linker_arguments, newargs)

phase_link(linker_arguments, wasm_target, js_syms)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can revert this file since this will be part of the "port settings" PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this has nothing to do with port settings. It is a change that is required for using js_libraries in ports


# Special handling for when the user passed '-Wl,--version'. In this case the linker
Expand Down
48 changes: 37 additions & 11 deletions tools/ports/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,11 @@


def load_port(name):
expected_attrs = ['get', 'clear', 'show']
port = __import__(name, globals(), level=1)
port = __import__(name, globals(), level=1, fromlist=[None])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does does the addition of fromlist=[None] here do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will double check if I can get away without it, but I copied this from your own PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I double checked and without fromlist=[None] it doesn't work. Like I said I had copied this bit from your PR and I assumed you had done it for a good reason. Without it, the modules under contrib are not loaded properly.

Displaying port.__name__ which is the name of the module, shows that for modules under ports it works (I get for example tools.ports.sdl2) but for ports under contribs it doesn't as the name is tools.ports.contrib.

If I use fromlist=[None], then the name is now correct tools.ports.contrib.glfw3

Asking Jetbrains AI give this answer (which I clipped because it is quite long but quite informative):

In conclusion, fromlist=[None] is a way to tell __import__ to treat the last part of the module path as the actual module that we want to import. If the list is empty or not provided __import__ will instead return the first module in the provided path.

Note that documentation for __import__ recommends: Because this function is meant for use by the Python interpreter and not for general use, it is better to use importlib.import_module() to programmatically import a module.

ports.append(port)
port.is_contrib = name.startswith('contrib.')
port.name = name
ports_by_name[port.name] = port
for a in expected_attrs:
assert hasattr(port, a), 'port %s is missing %s' % (port, a)
if not hasattr(port, 'needed'):
port.needed = lambda s: name in ports_needed
else:
Expand All @@ -57,13 +55,31 @@ def load_port(name):
if not hasattr(port, 'variants'):
# port variants (default: no variants)
port.variants = {}
if not hasattr(port, 'show'):
port.show = lambda: f'{port.name} (--use-port={port.name}; {port.LICENSE})'

for variant, extra_settings in port.variants.items():
if variant in port_variants:
utils.exit_with_error('duplicate port variant: %s' % variant)
port_variants[variant] = (port.name, extra_settings)


def validate_port(port):
expected_attrs = ['get', 'clear', 'show']
if port.is_contrib:
expected_attrs += ['URL', 'DESCRIPTION', 'LICENSE']
for a in expected_attrs:
assert hasattr(port, a), 'port %s is missing %s' % (port, a)


def validate_ports():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One reason I thought we might want to do this at runtime is that we might want to allow folks to drop in new ports (or maybe as you suggest using URL of a port on the command line). In that case it might be good to validate each time you use a port. However, we can address that once we get to that point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course if/when ports are added dynamically or loaded on demand this would have to change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean technically you can do today by simply dropping a new file into tools/ports.. but I'm not sure anyone is doing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted the change and now validate_ports() is called every time ports are read (line 99) for the potential cases where somebody might drop a file in tools/ports

for port in ports:
validate_port(port)
for dep in port.deps:
if dep not in ports_by_name:
utils.exit_with_error('unknown dependency in port: %s' % dep)


@ToolchainProfiler.profile()
def read_ports():
for filename in os.listdir(ports_dir):
Expand All @@ -72,10 +88,14 @@ def read_ports():
filename = os.path.splitext(filename)[0]
load_port(filename)

for port in ports:
for dep in port.deps:
if dep not in ports_by_name:
utils.exit_with_error('unknown dependency in port: %s' % dep)
contrib_dir = os.path.join(ports_dir, 'contrib')
for filename in os.listdir(contrib_dir):
if not filename.endswith('.py') or filename == '__init__.py':
continue
filename = os.path.splitext(filename)[0]
load_port('contrib.' + filename)

validate_ports()


def get_all_files_under(dirname):
Expand Down Expand Up @@ -441,9 +461,15 @@ def add_cflags(args, settings): # noqa: U100


def show_ports():
print('Available ports:')
for port in sorted(ports, key=lambda p: p.name):
print(' ', port.show())
sorted_ports = sorted(ports, key=lambda p: p.name)
print('Available official ports:')
for port in sorted_ports:
if not port.is_contrib:
print(' ', port.show())
print('Available contrib ports:')
for port in sorted_ports:
if port.is_contrib:
print(' ', port.show())


read_ports()
19 changes: 19 additions & 0 deletions tools/ports/contrib/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Emscripten "Contrib" Ports
==========================

Ports in this directory are contributed by the wider community and are
supported on a "best effort" basis. Since they are not run as part of
emscripten CI they are not always guaranteed to build or function.

If you want to add a contrib port, please use another contrib port as
an example. In particular, each contrib port must provide 3 extra piece
of information:

* `URL`: the url where the user can find more information about
the project/port
* `DESCRIPTION`: a (short) description of what the project/port
is about
* `LICENSE`: the license used by the project/port

After adding a contrib port, you should consider modifying the documentation
under `site/source/docs/compiling/Contrib-Ports.rst`.
Empty file added tools/ports/contrib/__init__.py
Empty file.
54 changes: 54 additions & 0 deletions tools/ports/contrib/glfw3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Copyright 2024 The Emscripten Authors. All rights reserved.
# Emscripten is available under two separate licenses, the MIT license and the
# University of Illinois/NCSA Open Source License. Both these licenses can be
# found in the LICENSE file.

import os

TAG = '1.0.4'
HASH = 'c3c96718e5d2b37df434a46c4a93ddfd9a768330d33f0d6ce2d08c139752894c2421cdd0fefb800fe41fafc2bbe58c8f22b8aa2849dc4fc6dde686037215cfad'

# 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'


def get_lib_name(settings):
return 'lib_contrib.glfw3.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)

def create(final):
root_path = os.path.join(ports.get_dir(), 'contrib.glfw3')
source_path = os.path.join(root_path, 'src', 'cpp')
source_include_paths = [os.path.join(root_path, 'external', 'GLFW'), os.path.join(root_path, 'include', 'GLFW')]
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']

ports.build_port(source_path, final, 'contrib.glfw3', includes=source_include_paths, flags=flags)

return [shared.cache.get_lib(get_lib_name(settings), create, what='port')]


def clear(ports, settings, shared):
shared.cache.erase_lib(get_lib_name(settings))


def linker_setup(ports, settings):
root_path = os.path.join(ports.get_dir(), 'contrib.glfw3')
source_js_path = os.path.join(root_path, 'src', 'js', 'lib_emscripten_glfw3.js')
settings.JS_LIBRARIES += [source_js_path]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is never called as far as I can see?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is used. My port has a javascript implementation. If you remove this then link fails because it cannot find all the functions implemented in js



# Using contrib.glfw3 to avoid installing headers into top level include path
# so that we don't conflict with the builtin GLFW headers that emscripten
# includes
def process_args(ports):
return ['-isystem', ports.get_include_dir('contrib.glfw3')]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we want the include path to include contrib. like this? Maybe its fine and it avoid conflict the the builtin headers? For ports that don't conflict I would hope we can avoid this.

Perhaps add a comment such as:

Avoid installing headers into top level include path so that we don't conflict with the builtin GLFW headers that emscripten includes
?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok that makes sense