Enable real pthreads for emscripten-wasm32 builds

vinca/distro.py

@@ -543,12 +543,26 @@ def _construct_raw_url_github(self, pkg_info):
         # Extract owner/repo
         owner_repo = raw_url_base.split("github.com/")[-1]
         # Use rev if available, otherwise fallback to tag
-        ref = pkg_info.get("rev") or pkg_info.get("tag")
+        rev = pkg_info.get("rev")
+        tag = pkg_info.get("tag")
         xml_name = pkg_info.get("package_xml_name", "package.xml")
         additional_folder = pkg_info.get("additional_folder", "")
         if additional_folder != "":
             additional_folder = additional_folder + "/"
-        raw_url = f"https://raw.githubusercontent.com/{owner_repo}/{ref}/{additional_folder}{xml_name}"
+        if rev:
+            # A commit hash is unambiguous as-is.
+            ref_path = rev
+        else:
+            # ros2-gbp release tags look like "release/jazzy/foo_pkg/1.2.3-1" --
+            # raw.githubusercontent.com's short <owner>/<repo>/<ref>/<path> form
+            # has to guess where a slash-containing ref ends and the path
+            # begins, and that guess is inconsistently cached across CDN edges:
+            # the same URL can 404 from some vantage points (including GitHub
+            # Actions runners) while resolving fine from others. The explicit
+            # refs/tags/<name> form removes the ambiguity and resolves
+            # reliably everywhere.
+            ref_path = f"refs/tags/{tag}"
+        raw_url = f"https://raw.githubusercontent.com/{owner_repo}/{ref_path}/{additional_folder}{xml_name}"
         return raw_url
 
     # format (checked against GitLab 19.x): https://gitlab.com/<NAMESPACE>/-/raw/<REV>/<PATH>

vinca/pinning.py

@@ -219,11 +219,69 @@ def _migration_name(name: str) -> str:
     return name
 
 
+def _existing_eol_comment_text(source: Any, index: int) -> Optional[str]:
+    """Return the plain text of a CommentedSeq item's trailing EOL comment, if any."""
+    ca = getattr(source, "ca", None)
+    if ca is None:
+        return None
+    entry = ca.items.get(index)
+    if not entry:
+        return None
+    token = entry[0]
+    if token is None:
+        return None
+    return str(token.value).lstrip("#").strip()
+
+
+def _flatten_v1_selectors(value: Any) -> Any:
+    """Convert v1-style `- if: COND then: [...]` list entries into the legacy
+    `- VALUE  # [COND]` comment-annotated form that rattler-build's variant
+    config loader actually evaluates lazily per target_platform (unlike the
+    v1 if/then/else mapping form, which it treats as an opaque literal value
+    rather than a selector -- confirmed via `Could not parse version spec
+    for variant key ...: invalid channel` / `multiple bracket sections not
+    allowed` errors when left unconverted).
+
+    Passthrough items (plain scalars, possibly already carrying their own
+    `# [selector]` EOL comment) must have that existing comment re-attached
+    at their new index -- ruamel stores comments keyed by list position on
+    the *source* CommentedSeq, not on the item itself, so a naive
+    `result.append(item)` into a freshly created CommentedSeq silently
+    drops it, turning a platform-scoped entry into an unconditional one.
+    """
+    if not isinstance(value, list):
+        return value
+    import ruamel.yaml.comments as _rc
+
+    result = _rc.CommentedSeq()
+    for old_index, item in enumerate(value):
+        if isinstance(item, Mapping) and "if" in item and "then" in item:
+            cond = str(item["if"])
+            for entry in item["then"]:
+                idx = len(result)
+                result.append(entry)
+                result.yaml_add_eol_comment(f"[{cond}]", idx)
+            else_branch = item.get("else")
+            if else_branch is not None:
+                not_cond = f"not ({cond})"
+                for entry in else_branch:
+                    idx = len(result)
+                    result.append(entry)
+                    result.yaml_add_eol_comment(f"[{not_cond}]", idx)
+        else:
+            idx = len(result)
+            result.append(item)
+            comment_text = _existing_eol_comment_text(value, old_index)
+            if comment_text:
+                result.yaml_add_eol_comment(comment_text, idx)
+    return result
+
+
 def _overlay(target: Any, source: Any) -> None:
     for key, value in source.items():
         if key == "migrator_ts" or str(key).startswith("__"):
             continue
-        target[key] = value
+        target[key] = _flatten_v1_selectors(value)
 
 
 def _migration_timestamp(payload: bytes) -> float:

vinca/templates/build_ament_cmake.sh.in

@@ -69,20 +69,81 @@ if [[ $target_platform =~ emscripten.* ]]; then
   echo "set(CMAKE_STRIP FALSE)  # used by default in pybind11 on .so modules">> $SRC_DIR/__vinca_shared_lib_patch.cmake
   echo "set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE BOTH)  # fixes an error where numpy header files are not found correctly">> $SRC_DIR/__vinca_shared_lib_patch.cmake
 
-  # if [ "${PKG_NAME}" == "ros-humble-examples-rclcpp-minimal-publisher" ] || [ "${PKG_NAME}" == "ros-humble-examples-rclcpp-minimal-subscriber" ] || [ "${PKG_NAME}" == "ros-humble-rclcpp-components" ]; then
-  #   echo "set(CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS \"-s ASSERTIONS=1 -s SIDE_MODULE=1 -sWASM_BIGINT -s USE_PTHREADS=0 -s DEMANGLE_SUPPORT=1 -s ALLOW_MEMORY_GROWTH=1 \")">> $SRC_DIR/__vinca_shared_lib_patch.cmake
-  #   echo "set(CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS \"-s ASSERTIONS=1 -s SIDE_MODULE=1 -sWASM_BIGINT -s USE_PTHREADS=0 -s DEMANGLE_SUPPORT=1 -s ALLOW_MEMORY_GROWTH=1 -sASYNCIFY -O3 -s ASYNCIFY_STACK_SIZE=24576 \")">> $SRC_DIR/__vinca_shared_lib_patch.cmake
-  #   echo "set(CMAKE_EXE_LINKER_FLAGS \"-sMAIN_MODULE=1 -sASSERTIONS=1 -fexceptions -lembind -sWASM_BIGINT -s USE_PTHREADS=0 -s DEMANGLE_SUPPORT=1 -sALLOW_MEMORY_GROWTH=1 -sASYNCIFY -O3 -s ASYNCIFY_STACK_SIZE=24576 -L$SRC_DIR/build -L$PREFIX/lib\")  # remove SIDE_MODULE from exe linker flags">> $SRC_DIR/__vinca_shared_lib_patch.cmake
-  # else
-    echo "set(CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS \"-s ASSERTIONS=1 -s SIDE_MODULE=1 -sWASM_BIGINT -s USE_PTHREADS=0 -s ALLOW_MEMORY_GROWTH=1 -s DEMANGLE_SUPPORT=1 \")">> $SRC_DIR/__vinca_shared_lib_patch.cmake
-    echo "set(CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS \"-s ASSERTIONS=1 -s SIDE_MODULE=1 -sWASM_BIGINT -s USE_PTHREADS=0 -s ALLOW_MEMORY_GROWTH=1 -s DEMANGLE_SUPPORT=1 \")">> $SRC_DIR/__vinca_shared_lib_patch.cmake
-    echo "set(CMAKE_EXE_LINKER_FLAGS \"-sMAIN_MODULE=1 -sASSERTIONS=1 -fexceptions -lembind -sWASM_BIGINT -s USE_PTHREADS=0 -sALLOW_MEMORY_GROWTH=1 -s DEMANGLE_SUPPORT=1 -L$SRC_DIR/build -L$PREFIX/lib\")  # remove SIDE_MODULE from exe linker flags">> $SRC_DIR/__vinca_shared_lib_patch.cmake
-  # fi
+  # No real pthreads here (deliberately -- see history below). Instead,
+  # Asyncify lets the one genuinely blocking call in this stack (rmw_wait,
+  # which polls zenoh-pico's WS transport) cooperatively yield back to the
+  # browser's JS event loop instead of really blocking -- that's enough for
+  # wall-clock time (and incoming WebSocket messages) to advance while a
+  # "blocking" wait is outstanding, without needing an OS-level thread.
+  #
+  # Real pthreads (USE_PTHREADS=1) were tried instead of this and reverted.
+  # They do make a blocking std::condition_variable/rmw wait set actually
+  # wake up, but they require wasm --shared-memory, which is viral: every
+  # module dlopen'd into an eagerly-linked host must also be pthreads/
+  # shared-memory or linking fails ("mismatch in shared state of memory"),
+  # and worse, a genuinely blocking wait on a thread that also needs to
+  # service a message loop (e.g. JupyterLite's xeus-python kernel) just
+  # deadlocks outright -- there's nothing left free to deliver the wakeup.
+  # Asyncify avoids needing real concurrency at all for this.
+  echo "set(CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS \"-s ASSERTIONS=1 -s SIDE_MODULE=1 -sWASM_BIGINT -s ALLOW_MEMORY_GROWTH=1 -sASYNCIFY -s ASYNCIFY_STACK_SIZE=24576 \")">> $SRC_DIR/__vinca_shared_lib_patch.cmake
+  echo "set(CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS \"-s ASSERTIONS=1 -s SIDE_MODULE=1 -sWASM_BIGINT -s ALLOW_MEMORY_GROWTH=1 -sASYNCIFY -s ASYNCIFY_STACK_SIZE=24576 \")">> $SRC_DIR/__vinca_shared_lib_patch.cmake
+  # CMake's MODULE library type (add_library(... MODULE), what
+  # pybind11_add_module() uses for Python C extensions e.g. rclpy's
+  # _rclpy_pybind11) is a distinct target type from SHARED and reads its
+  # own CMAKE_SHARED_MODULE_CREATE_*_FLAGS variables -- keep it consistent
+  # with the SHARED flags above.
+  echo "set(CMAKE_SHARED_MODULE_CREATE_C_FLAGS \"-s ASSERTIONS=1 -s SIDE_MODULE=1 -sWASM_BIGINT -s ALLOW_MEMORY_GROWTH=1 -sASYNCIFY -s ASYNCIFY_STACK_SIZE=24576 \")">> $SRC_DIR/__vinca_shared_lib_patch.cmake
+  echo "set(CMAKE_SHARED_MODULE_CREATE_CXX_FLAGS \"-s ASSERTIONS=1 -s SIDE_MODULE=1 -sWASM_BIGINT -s ALLOW_MEMORY_GROWTH=1 -sASYNCIFY -s ASYNCIFY_STACK_SIZE=24576 \")">> $SRC_DIR/__vinca_shared_lib_patch.cmake
+  echo "set(CMAKE_EXE_LINKER_FLAGS \"-sMAIN_MODULE=1 -sASSERTIONS=1 -fexceptions -lembind -sWASM_BIGINT -sALLOW_MEMORY_GROWTH=1 -sASYNCIFY -s ASYNCIFY_STACK_SIZE=24576 -L$SRC_DIR/build -L$PREFIX/lib\")  # remove SIDE_MODULE from exe linker flags">> $SRC_DIR/__vinca_shared_lib_patch.cmake
+
+  # A message package's *Config.cmake only exports find_dependency() calls
+  # for what its own package.xml/CMakeLists.txt actually declares -- it has
+  # no idea VINCA_EMSCRIPTEN_STATIC_TYPESUPPORT_C/_CPP named an extra
+  # typesupport backend, so it never re-exports *that* dependency to ITS
+  # OWN consumers. A package that only uses one message package at a time
+  # never notices (it already found the backend itself while configuring
+  # its own rosidl_generate_interfaces() call), but one that find_package()s
+  # several message packages together hits "the target was not found ...
+  # A find_package call is missing for an IMPORTED target" the first time a
+  # downstream *Export.cmake references
+  # rosidl_typesupport_microxrcedds_c(pp)::rosidl_typesupport_microxrcedds_c(pp)
+  # without anyone upstream having found it first. Pre-finding it here (via
+  # CMAKE_PROJECT_INCLUDE, so it's already in every target's CMake
+  # namespace before that project's own find_package() calls run) covers
+  # every consumer uniformly instead of patching each one individually.
+  # A package that calls rosidl_generate_interfaces() itself (i.e. defines
+  # its own messages/services/actions) must NOT get this pre-find: that
+  # macro discovers available typesupport implementations itself and
+  # registers each one's ament_export_targets() call in a fixed relative
+  # order (each backend's generator target before its own typesupport
+  # target). Pre-finding the override backend here makes it "already a
+  # target" before that macro runs, which -- empirically confirmed by
+  # inspecting the resulting package's own ament_cmake_export_targets-extras.cmake
+  # -- causes THIS package's typesupport entry to jump to the front of its
+  # own _exported_targets list, ahead of the generator target its own
+  # Export.cmake requires (INTERFACE_LINK_LIBRARIES references
+  # <pkg>::<pkg>__rosidl_generator_c(pp)). That makes every downstream
+  # find_package(<this package>) fail with "referenced, but are missing:
+  # <pkg>::<pkg>__rosidl_generator_c(pp)" -- reproducible regardless of
+  # whether the C or C++ (or both) override is set. Without any pre-find,
+  # rosidl_generate_interfaces() discovers the same override backend on its
+  # own (via STATIC_ROSIDL_TYPESUPPORT_C/_CPP below) in the correct order,
+  # so skipping it here loses nothing for this package's own typesupport
+  # selection -- it only loses the (here unneeded) benefit described above
+  # of pre-registering the backend for *consumers* of this package.
+  if ! grep -q "rosidl_generate_interfaces(" "$SRC_DIR/$PKG_NAME"/src/work/CMakeLists.txt 2>/dev/null; then
+    if [ -n "${VINCA_EMSCRIPTEN_STATIC_TYPESUPPORT_C:-}" ]; then
+      echo "find_package(${VINCA_EMSCRIPTEN_STATIC_TYPESUPPORT_C} QUIET)">> $SRC_DIR/__vinca_shared_lib_patch.cmake
+    fi
+    if [ -n "${VINCA_EMSCRIPTEN_STATIC_TYPESUPPORT_CPP:-}" ]; then
+      echo "find_package(${VINCA_EMSCRIPTEN_STATIC_TYPESUPPORT_CPP} QUIET)">> $SRC_DIR/__vinca_shared_lib_patch.cmake
+    fi
+  fi
 
   export BUILD_TYPE="Debug"
   export EXTRA_CMAKE_ARGS=" \
       -DPYTHON_SOABI="cpython-${ROS_PYTHON_VERSION//./}-wasm32-emscripten" \
-      -DRMW_IMPLEMENTATION=rmw_wasm_cpp \
+      -DRMW_IMPLEMENTATION=${VINCA_EMSCRIPTEN_RMW_IMPLEMENTATION:-rmw_wasm_cpp} \
       -DCMAKE_FIND_ROOT_PATH=$PREFIX \
       -DCMAKE_POSITION_INDEPENDENT_CODE=TRUE \
       -DCMAKE_PROJECT_INCLUDE=$SRC_DIR/__vinca_shared_lib_patch.cmake \
@@ -92,8 +153,8 @@ if [[ $target_platform =~ emscripten.* ]]; then
   export CMAKE_GEN="emcmake cmake"
   export CMAKE_BLD="cmake"
 
-  export STATIC_ROSIDL_TYPESUPPORT_C=rosidl_typesupport_introspection_c
-  export STATIC_ROSIDL_TYPESUPPORT_CPP=rosidl_typesupport_introspection_cpp
+  export STATIC_ROSIDL_TYPESUPPORT_C=${VINCA_EMSCRIPTEN_STATIC_TYPESUPPORT_C:-rosidl_typesupport_introspection_c}
+  export STATIC_ROSIDL_TYPESUPPORT_CPP=${VINCA_EMSCRIPTEN_STATIC_TYPESUPPORT_CPP:-rosidl_typesupport_introspection_cpp}
 else
   export BUILD_TYPE="Release"
   export CMAKE_GEN="cmake"

vinca/test_github_raw_url.py

@@ -0,0 +1,59 @@
+from typing import Any
+
+from vinca.distro import Distro
+
+
+def _distro() -> Any:
+    return Distro.__new__(Distro)
+
+
+def test_tag_ref_uses_explicit_refs_tags_prefix():
+    # ros2-gbp release tags look like "release/jazzy/foo_pkg/1.2.3-1" -- the
+    # short <owner>/<repo>/<ref>/<path> raw.githubusercontent.com form has to
+    # guess where a slash-containing ref ends and the path begins, and that
+    # guess is inconsistently cached across CDN edges (the same URL 404s from
+    # some vantage points, including GitHub Actions runners, while resolving
+    # fine from others). The explicit refs/tags/<name> form is unambiguous.
+    pkg_info = {
+        "url": "https://github.com/ros2-gbp/ros2_control-release.git",
+        "tag": "release/jazzy/controller_interface/4.47.0-1",
+    }
+
+    url = _distro()._construct_raw_url_github(pkg_info)
+
+    assert url == (
+        "https://raw.githubusercontent.com/ros2-gbp/ros2_control-release/"
+        "refs/tags/release/jazzy/controller_interface/4.47.0-1/package.xml"
+    )
+
+
+def test_rev_ref_is_used_as_is():
+    # A commit hash is already unambiguous -- it must not get the refs/tags/
+    # prefix, since it isn't a tag name.
+    pkg_info = {
+        "url": "https://github.com/ros2-gbp/ros2_control-release.git",
+        "rev": "abc123def456",
+    }
+
+    url = _distro()._construct_raw_url_github(pkg_info)
+
+    assert url == (
+        "https://raw.githubusercontent.com/ros2-gbp/ros2_control-release/"
+        "abc123def456/package.xml"
+    )
+
+
+def test_tag_ref_with_additional_folder_and_custom_xml_name():
+    pkg_info = {
+        "url": "https://github.com/example/some-release.git",
+        "tag": "release/rolling/some_pkg/1.0.0-1",
+        "additional_folder": "some_pkg",
+        "package_xml_name": "package.xml",
+    }
+
+    url = _distro()._construct_raw_url_github(pkg_info)
+
+    assert url == (
+        "https://raw.githubusercontent.com/example/some-release/"
+        "refs/tags/release/rolling/some_pkg/1.0.0-1/some_pkg/package.xml"
+    )

vinca/test_snapshot_metadata.py

@@ -61,9 +61,11 @@ def make_snapshot_distro(monkeypatch):
     distro._distro = Mock()
     snapshot_xml_by_url = {
         "https://raw.githubusercontent.com/example/snapshot-package-release/"
-        "release/rolling/snapshot_package/1.0.0-1/package.xml": (SNAPSHOT_PACKAGE_XML),
+        "refs/tags/release/rolling/snapshot_package/1.0.0-1/package.xml": (
+            SNAPSHOT_PACKAGE_XML
+        ),
         "https://raw.githubusercontent.com/example/snapshot-dependency-release/"
-        "release/rolling/snapshot_dependency/1.0.0-1/package.xml": (
+        "refs/tags/release/rolling/snapshot_dependency/1.0.0-1/package.xml": (
             SNAPSHOT_DEPENDENCY_XML
         ),
     }