Skip to content

Share protobuf runtime and expose optional-wheel build capabilities - #1499

Merged
schaubh merged 3 commits into
developfrom
feature/protobuf_shared
Jul 27, 2026
Merged

Share protobuf runtime and expose optional-wheel build capabilities#1499
schaubh merged 3 commits into
developfrom
feature/protobuf_shared

Conversation

@schaubh

@schaubh schaubh commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

This PR prevents interpreter-shutdown crashes on macOS when OpenCV-backed OpNav modules and vizInterface are loaded in the same process. Protobuf is built as a shared Conan dependency on macOS so both consumers use one runtime.

It also extends Basilisk.hasBuildFeature() to aggregate capabilities provided by the core artifact and separately installed optional wheels. Optional wheels advertise their features through package entry-point metadata and must match the core Basilisk version.

The commits are organized as follows:

  1. Configure the shared protobuf runtime and add regression coverage for loading OpNav and vizInterface in either order.
  2. Add optional-wheel feature metadata, capability aggregation, compatibility validation, tests, and documentation.

Verification

  • Completed a clean macOS build with all build options enabled and ran the full test suite successfully.
  • The GitHub “Test Optional Wheels” workflow passed on Windows, macOS, Ubuntu x64, and Ubuntu ARM for the identical source tree.
  • A repeat workflow run against the rewritten branch tip is in progress: https://github.com/AVSLab/basilisk/actions/runs/30227387528
  • Added subprocess regression tests that instantiate both protobuf consumers in both import orders and verify clean interpreter shutdown.
  • Added tests for optional-wheel metadata generation, feature discovery, missing providers, incompatible provider versions, malformed build metadata, and unexpected metadata drift.
  • Verified installation, removal, and reinstallation of the bsk-opnav wheel updates the reported opNav capability as expected.
  • The focused build-information, wheel-builder, and protobuf-runtime suite passes with 19 tests.
  • Pre-commit, Python compilation, release-note compilation, and git diff --check pass.

No test baselines were changed.

Documentation

Updated the following documentation:

  • Build metadata documentation describing the distinction between core build provenance and aggregate installed capabilities.
  • Coding guidelines for guarding tests in monolithic and split-wheel installations.
  • Known issues documenting the macOS protobuf shutdown crash and optional-wheel capability reporting.
  • Release-note snippets for the shared protobuf runtime and optional-wheel feature aggregation.

Reviewers should verify the documented behavior of getBuildInfo() versus hasBuildFeature(), particularly that the former preserves core build provenance while the latter reports aggregate installed capabilities.

Future work

Future optional Basilisk wheels can use the same entry-point metadata mechanism to advertise their build capabilities. No additional wheel splits are included in this PR.

@schaubh schaubh self-assigned this Jul 27, 2026
@schaubh
schaubh requested a review from a team as a code owner July 27, 2026 00:30
@schaubh schaubh added documentation Improvements or additions to documentation enhancement New feature or request build Build system or compilation enhancement labels Jul 27, 2026
@schaubh schaubh added this to Basilisk Jul 27, 2026
@schaubh schaubh moved this to 👀 In review in Basilisk Jul 27, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e34880b46

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/cmake/bskBuildInfo.py Outdated
Comment on lines +30 to +31
@_lruCache(maxsize=1)
def _installedBuildFeatureProviders() -> dict[str, tuple[tuple[str, str], ...]]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Refresh providers after environment changes

When a long-running process changes its environment after the first feature query—for example, a notebook runs %pip install bsk-opnav after hasBuildFeature("opNav") returned False—this process-lifetime cache prevents the newly installed entry point from ever being discovered; uninstalling or upgrading a provider similarly leaves stale capabilities or version-mismatch errors. The install/uninstall checks use fresh subprocesses, and the unit test manually clears this cache, so they do not cover the persistent-process behavior. Avoid caching this metadata indefinitely or provide automatic invalidation so the query reflects the currently installed distributions.

Useful? React with 👍 / 👎.

schaubh added 3 commits July 26, 2026 18:42
Build protobuf as a shared Conan dependency so OpenCV-backed OpNav and
vizInterface bindings use one runtime without crashing at interpreter
shutdown. Add subprocess import-order coverage for source builds and
optional wheels, and document the resolved issue.
- let optional wheels declare build features through entry-point metadata
- preserve core build provenance while detecting installed feature providers
- reject optional providers that do not match the core Basilisk version
- validate the intentional build-metadata delta used to create extension wheels
- test feature discovery, metadata failures, and optional-wheel installation states
- document feature guards for monolithic and split-wheel installations
Remove process-lifetime caching from optional build feature discovery so
hasBuildFeature() reflects providers installed, upgraded, or removed while
Python is running. Add same-process coverage using real distribution metadata
and remove obsolete manual cache invalidation from the tests.
@schaubh
schaubh force-pushed the feature/protobuf_shared branch from 9e34880 to 822104c Compare July 27, 2026 00:42
@schaubh
schaubh merged commit 82af4f6 into develop Jul 27, 2026
13 checks passed
schaubh added a commit that referenced this pull request Jul 27, 2026
Build protobuf as a shared Conan dependency so OpenCV-backed OpNav and
vizInterface bindings use one runtime without crashing at interpreter
shutdown. Add subprocess import-order coverage for source builds and
optional wheels, and document the resolved issue.
schaubh added a commit that referenced this pull request Jul 27, 2026
- let optional wheels declare build features through entry-point metadata
- preserve core build provenance while detecting installed feature providers
- reject optional providers that do not match the core Basilisk version
- validate the intentional build-metadata delta used to create extension wheels
- test feature discovery, metadata failures, and optional-wheel installation states
- document feature guards for monolithic and split-wheel installations
@github-project-automation github-project-automation Bot moved this from 👀 In review to ✅ Done in Basilisk Jul 27, 2026
@schaubh
schaubh deleted the feature/protobuf_shared branch July 27, 2026 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Build system or compilation enhancement documentation Improvements or additions to documentation enhancement New feature or request

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant