Attention is currently required from: fixeria, neels, pespin.
15 comments:
Commit Message:
Patch Set #5, Line 7: Add Emscripten build support and JS callback logging backend
Why is this needed? How is this used? What's the context? I'm missing a good description here for a […]
Thanks for pointing this out.
The purpose of this patch is to make libosmocore usable in sandboxed, non-POSIX environments, specifically WebAssembly targets built with the Emscripten toolchain.
In such environments, a number of assumptions made by libosmocore (process model, availability of standard OS interfaces, and I/O mechanisms) do not hold, which currently prevents the library from being built and used at all. This patch introduces the minimal platform-specific adjustments required to enable successful builds and basic runtime integration in that context.
The intended use case is running Osmocom protocol and control components inside a WebAssembly runtime, where interaction with the outside world is mediated by the host environment rather than direct OS access.
All changes are limited to Emscripten-specific build paths and are inactive for native targets, leaving existing deployments and behavior unchanged.
I will update the commit message to better reflect this context.
File include/osmocom/core/logging.h:
Patch Set #5, Line 285: LOG_TGT_TYPE_WEB, /*!< Web logging */
AFAIU this is not "web generic" but "emscripten" specific, so let's please rename to something more […]
Thanks for the clarification. You are right — the current naming suggests a generic “web” abstraction, while in practice this mechanism is only enabled for Emscripten builds.
I will rename the API accordingly and update the commit message to better explain the intended scope and usage. I will send an updated patch shortly.
File src/core/netdev.c:
Patch Set #5, Line 67: #if (!EMBEDDED) && !defined(__EMSCRIPTEN__)
I'd say better do this logic inside configure. […]
Fully agree, that's cleaner and more in line with the rest of the library.
I went ahead and added a new configure option `ENABLE_NETDEV` in configure.ac.
For EMBEDDED and EMSCRIPTEN builds it is explicitly set to `false`
Right now it fails to compile
File src/core/osmo_io_internal.h:
Patch Set #5, Line 7: HAVE_LIBSCTP
Indeed config.h should be at the top.
Thanks for catching this!
You're right — I completely missed that config.h is included after this block
File src/core/serial.c:
Patch Set #5, Line 27: #if !defined(__EMSCRIPTEN__)
Again, I'd say better add a HAVE_SERIAL or alike in configure.ac. […]
Thanks for the note!
After taking a closer look at configure.ac, I noticed the project already uses `ENABLE_SERIAL` to conditionally skip compiling the serial.c file.
The file compiles without issues, but in emscripten/web environments there's simply no need for serial support at all.
File src/core/socket.c:
Patch Set #5, Line 2050: #ifdef HAVE_LIBSCTP
missing explanation for this change. It may be a separate patch.
Explained in the comment for the function `osmo_sock_multiaddr_get_ip_and_port`
Patch Set #5, Line 1928: #ifdef HAVE_LIBSCTP
Why this change? I'm lacking any rationale here. Also, looks like this may be a separate patch.
About functions `osmo_sock_multiaddr_get_ip_and_port`, `osmo_multiaddr_ip_and_port_snprintf` and `osmo_sock_multiaddr_get_name_buf`:
These functions were previously conditionally compiled only under `HAVE_LIBSCTP`. However, `libosmo-netif` uses some of them without the same `HAVE_LIBSCTP` guards, leading to link failures when libsctp is disabled.
Additionally, the implementations already included a fallback to legacy (single-address) behavior, so they do not strictly depend on SCTP/multi-homing features.
Refactoring done as follows:
File src/vty/Makefile.am:
Patch Set #5, Line 36: libosmovty_la_SOURCES += telnet_interface_dummy.c
This for sure also needs an explanation.
In Emscripten there are no sockets so `telnet_interface.c` fails to compile.
However, completely excluding the file from the build leads to link errors (undefined references to the telnet functions).
To resolve this, I added a new file `telnet_interface_dummy.c` with stub/empty implementations of the required functions.
I considered adding many `#ifdef __EMSCRIPTEN__` blocks directly inside telnet_interface.c, but decided that separate dummy stubs are the cleaner approach.
File src/vty/logging_vty.c:
Patch Set #5, Line 907: #if !defined(__EMSCRIPTEN__)
this also needs explanation.
The conditional compilation here was added by mistake — it's not needed in this place.
This spot was accidentally affected when I removed the file logging parts (Emscripten only emulates files via IndexedDB with no native operations, so file logging isn't usable anyway).
Patch Set #5, Line 1038: DEFUN(cfg_log_web, cfg_log_web_cmd,
#if defined(__EMSCRIPTEN__)
Thanks — added conditional compilation `#if defined(__EMSCRIPTEN__)`
Patch Set #5, Line 1047: vty_out(vty, "%% Unable to create WEB log for %s", VTY_NEWLINE);
this string end looks wrong
Thanks, fixed
Patch Set #5, Line 1067: vty_out(vty, "%% Unable to find WEB log target for %s", VTY_NEWLINE);
this string end looks wrong
Thanks, fixed
No need for a space here.
Thanks, fixed
install_lib_element(CONFIG_NODE, &cfg_log_web_cmd);
install_lib_element(CONFIG_NODE, &cfg_no_log_web_cmd);
I assume these commands are only relevant for emscripten? […]
Thanks — added conditional compilation `#if defined(__EMSCRIPTEN__)`
File src/vty/telnet_interface_dummy.c:
#include <sys/socket.h>
#include <netinet/in.h>
#include <errno.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#i
(Not critical) Most if the includes are not needed here and can be removed?
Thanks for pointing that out!
You're right — many of the includes here are indeed not needed anymore (leftover from copy-paste)
To view, visit change 41813. To unsubscribe, or for help writing mail filters, visit settings.