Skip to content

Vendors: consume precompiled MafiaNet release archives - #261

Open
Segfaultd wants to merge 2 commits into
developfrom
vendors/mafianet-prebuilt
Open

Vendors: consume precompiled MafiaNet release archives#261
Segfaultd wants to merge 2 commits into
developfrom
vendors/mafianet-prebuilt

Conversation

@Segfaultd

@Segfaultd Segfaultd commented Aug 29, 2026

Copy link
Copy Markdown
Member

What

Replaces the FetchContent source build of MafiaNet with a configure-time download of the per-platform precompiled archive published by MafiaNet's release pipeline (linux-x86_64, windows-x86/x64, macos-x86_64/arm64 — static + shared libs, bundled opus/rnnoise, all headers, and the MafiaNetConfig.cmake package). The archive is extracted into the build tree and consumed via find_package(MafiaNet CONFIG REQUIRED); consumers keep linking MafiaNet::MafiaNetStatic unchanged.

Why

Building MafiaNet from source dragged ~85 MB of third-party source (plus its own Opus/RNNoise fetches) through every cold configure. Downloading one ~12 MB archive is faster and keeps the dependency boundary where it belongs.

Details

  • Pin: cmake/MafiaNetPin.cmake keeps its path (so bump_version.sh keeps its wire-format signal) but now names the release version plus the SHA-256 of each archive. Tags are mutable refs, so the hashes are what actually pin content — file(DOWNLOAD EXPECTED_HASH) fails the configure on any mismatch. Bumping MafiaNet = update the version and five hashes, nothing else.
  • Configs: the archives carry a single Release configuration compiled with /MD on Windows — exactly the CRT FrameworkSetup forces even in Debug — so Debug/RelWithDebInfo/MinSizeRel are mapped onto it via MAP_IMPORTED_CONFIG_*, and the old _DEBUG/CRT reconciliation block for the source build is deleted.
  • Scope: find_package imports are directory-scoped, unlike the old alias targets, so they are promoted IMPORTED_GLOBAL for the consumers under code/ (the GLOBAL keyword needs CMake 3.24; minimum here is 3.20).
  • OpenSSL still resolves through find_dependency(OpenSSL): the vendored 3.3.1 on Windows, the system one elsewhere — matching the OpenSSL 3 the archives were compiled against.

Verification

On macOS arm64: clean reconfigure downloads and hash-verifies MafiaNet-0.15.0-macos-arm64.tar.gz, FrameworkServer builds, and FrameworkTests links against the prebuilt static libs and passes 206/206 tests across 18 modules. Windows/Linux archives were inspected for correct arch and CRT (x86_64/i386 PE machine types, /MD Release objects; ELF glibc ≤ 2.34) but the CI run on this PR is their first real Framework build.

Summary by CodeRabbit

  • Build Improvements
    • MafiaNet is now retrieved as a precompiled, platform-specific release archive instead of being built from source.
    • Downloads are verified with SHA-256 checksums for improved integrity.
    • Added support for Linux, Windows, and Intel/Apple Silicon macOS builds.
    • Unsupported platform and architecture combinations now produce a clear configuration error.
    • Build configurations consistently use the packaged Release binaries.

Replace the FetchContent source build of MafiaNet with a download of the
per-platform precompiled archive published by MafiaNet's release pipeline
(static + shared libs, bundled opus/rnnoise, headers, CMake package). The
archive is fetched at configure time into the build tree and consumed via
find_package(MafiaNet CONFIG); consumers keep linking
MafiaNet::MafiaNetStatic unchanged.

The pin in cmake/MafiaNetPin.cmake (same path, so bump_version.sh keeps its
wire-format signal) now names the release version plus the SHA-256 of each
archive: tags are mutable refs, so the hashes are what actually pin content
(file(DOWNLOAD EXPECTED_HASH) fails the configure on any mismatch).

The archives carry a single Release configuration compiled with /MD on
Windows -- the CRT FrameworkSetup forces even in Debug -- so Debug/
RelWithDebInfo/MinSizeRel are mapped onto it via MAP_IMPORTED_CONFIG_*, and
the old _DEBUG/CRT reconciliation block for the source build is deleted.
find_package imports are directory-scoped, unlike the old alias targets, so
the imported targets are promoted IMPORTED_GLOBAL for the consumers under
code/ (the GLOBAL keyword needs CMake 3.24; minimum here is 3.20).
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 46 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 976a1ce6-94de-4b3b-ada6-e655df354283

📥 Commits

Reviewing files that changed from the base of the PR and between 7073aec and 9b49506.

📒 Files selected for processing (1)
  • vendors/CMakeLists.txt

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d5878be3-457a-42fb-bfeb-37de05236314

📥 Commits

Reviewing files that changed from the base of the PR and between 85a43ef and 7073aec.

📒 Files selected for processing (2)
  • cmake/MafiaNetPin.cmake
  • vendors/CMakeLists.txt

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

MafiaNet now uses versioned, SHA-256-pinned precompiled archives. CMake selects the platform archive, verifies and extracts it, loads MafiaNet through find_package, and exposes its imported targets to sibling directories.

Changes

MafiaNet archive integration

Layer / File(s) Summary
Version and archive hash pins
cmake/MafiaNetPin.cmake
The commit pin is replaced by version 0.15.0 and five platform-specific SHA-256 pins.
Platform selection and archive installation
vendors/CMakeLists.txt
CMake selects supported platform archives, rejects unsupported combinations, downloads pinned archives, verifies them, and extracts them.
Imported package configuration
vendors/CMakeLists.txt
CMake maps consumer configurations to Release, loads MafiaNet with find_package, and promotes imported targets to global scope.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 7073a

The build now downloads and verifies pinned MafiaNet release archives instead of compiling the dependency from source; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant CMakeConfigure
  participant MafiaHubRelease
  participant ArchiveExtractor
  participant MafiaNetPackage
  CMakeConfigure->>MafiaHubRelease: Download pinned platform archive
  MafiaHubRelease-->>CMakeConfigure: Return archive
  CMakeConfigure->>CMakeConfigure: Verify SHA-256
  CMakeConfigure->>ArchiveExtractor: Extract archive
  ArchiveExtractor-->>CMakeConfigure: Provide MafiaNetConfig.cmake
  CMakeConfigure->>MafiaNetPackage: find_package(MafiaNet)
  MafiaNetPackage-->>CMakeConfigure: Provide imported targets
Loading

Suggested reviewers: zpl-zak

Poem

A rabbit checks each hash with care
And hops through archives in the air
The right platform joins the queue
CMake finds the package too
Global targets bloom anew

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: replacing source-based MafiaNet consumption with precompiled release archives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch vendors/mafianet-prebuilt

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

CMAKE_MAP_IMPORTED_CONFIG_* was live during find_package(MafiaNet), so it
also seeded the OpenSSL::SSL/Crypto imported targets that
find_dependency(OpenSSL) creates inside that call. Those carry a single
config-less IMPORTED_LOCATION, and a mapping they cannot satisfy fails
Debug configures ('IMPORTED_LOCATION not set for ... configuration
Debug') -- exactly what the Linux/macOS CI legs hit. Set
MAP_IMPORTED_CONFIG_* directly on the MafiaNet targets instead, in the
same loop that promotes them IMPORTED_GLOBAL.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants