Skip to content

refactor!: crates re-export no whole crate - #8247

Merged
kixelated merged 6 commits into
mainfrom
quest/m2/api-shape/reexports
Nov 9, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m2/api-shape/reexports

Conversation

@kixelated

@kixelated kixelated commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Finishes quest/m2/api-shape/reexports.md.

Problem

Twelve of moq's crates re-exported whole crates (pub use moq_net;, pub use rustls;, pub use axum;, ...). A whole-crate re-export makes that crate's entire API part of moq's, so any breaking release of the dependency is a semver break of moq, and rust2's FFI exporter refuses each one. moq-mux's container::Producer::modify also returned a Guard no path could name.

Approach

  • Every whole-crate pub use is gone, moq's own crates included (the census in the quest matched a fresh grep: 28 sites in 17 files).
  • Where a signature named a third-party type only in passing, moq now owns the type, so the crate leaves its API:
    • WebTransport Request::reject takes a u16 HTTP status (moq-tokio's quic::web and moq-uring's), validated to 400..=599, and HandshakeError::Status carries a u16. This matches moq-tokio's existing Error::status() -> Option<u16>. http no longer appears in either crate's own signatures.
    • moq_tokio::watch::Files::new returns moq_tokio::Result (new Error::Watch(String)), not notify::Result.
    • moq_tokio::jemalloc::Allocator replaces the tikv_jemallocator re-export: a binary installs #[global_allocator] static ALLOC: moq_tokio::jemalloc::Allocator = moq_tokio::jemalloc::Allocator;, so no binary needs the crate and the allocator is always the one jemalloc::run profiles.
  • moq_mux::container re-exports Guard beside Producer.
  • Types moq's signatures are built on stay, and callers name the crate themselves: moq-net in hang and moq-tokio, bytes and kio in moq-net, rustls configs (moq-quic, moq-rtmp, moq-tokio), web-transport-proto's ConnectRequest/ConnectResponse, usage-rs (every moq-tokio config derives usage_rs::Args), axum routers (moq-hls, moq-webrtc), url::Url, moq-archive's ObjectStore, moq-video's platform handles and wgpu device, tokio-tungstenite in qmux, mp4-atom in moq-mux, moq-congestion's Factory, and jni::Env for tls::init_android.
  • About 150 callers in apps, bindings, tests, benches, and the interop clients name the crates directly. New direct dependencies use { workspace = true }; jni, tokio-tungstenite, and wgpu gained workspace entries, and their existing per-crate pins (moq-tokio, qmux, moq-video) moved onto them.
  • The wire-compat harness compiles test/interop/compat/container.rs against both the current and the released hang. The adapter now depends on moq-net itself: the checkout's path for the current hang, and the exact version the released hang resolves (cargo tree --invert moq-net) for the released one. Both builds were checked locally.

Impact

Breaking, Rust only. No wire, JS, or bindings change. Migration for every item below: add the named crate as your own dependency at the version moq's Cargo.lock uses, and import it directly instead of through moq's path.

  • hang: hang::moq_net is removed. Depend on moq-net.
  • moq-net: moq_net::bytes and moq_net::kio are removed. Depend on bytes and kio.
  • moq-quic: moq_quic::rustls is removed. Depend on rustls 0.23.
  • moq-rtmp: moq_rtmp::rustls (tls feature) is removed. Depend on rustls 0.23.
  • moq-tokio:
    • moq_tokio::{moq_net, rustls, notify, jni, congestion} are removed. Depend on moq-net, rustls, jni (Android), and moq-congestion (for quic::Controller).
    • cli::usage_rs is removed. Depend on usage-rs.
    • jemalloc::tikv_jemallocator is removed. Use jemalloc::Allocator as the #[global_allocator] (new).
    • quic::web::{http, proto} are removed. Depend on web-transport-proto for ConnectRequest/ConnectResponse (it re-exports http).
    • quic::web::Request::reject takes u16 instead of http::StatusCode: reject(StatusCode::NOT_FOUND) becomes reject(404). quic::web::HandshakeError::Status carries u16.
    • watch::Files::new returns moq_tokio::Result<Files>. The new Error::Watch(String) variant (watch feature) carries the failure.
  • moq-uring: moq_uring::url and quic::web::{http, proto} are removed. Depend on url and web-transport-proto. quic::web::Request::reject takes u16, and quic::web::HandshakeError::Status carries u16, as in moq-tokio.
  • moq-hls: moq_hls::axum is removed. Depend on axum 0.8.
  • moq-webrtc: moq_webrtc::{axum, url} are removed. Depend on axum 0.8 and url 2.
  • moq-archive: moq_archive::object_store is removed. Depend on object_store 0.14.
  • moq-video: moq_video::{ndk, objc2_core_foundation, objc2_core_video, windows} and render::wgpu are removed. Depend on the platform crate (ndk 0.9, objc2-core-foundation/objc2-core-video 0.3, windows 0.62) or wgpu 30.
  • moq-mux: container::fmp4::mp4_atom is removed. Depend on mp4-atom. container::Guard is new (additive), the type Producer::modify returns.
  • qmux: ws::{tokio_tungstenite, tungstenite} are removed. Depend on tokio-tungstenite 0.30, which re-exports tungstenite.

Alternatives

  • Single-type re-exports (pub use tikv_jemallocator::Jemalloc;) instead of whole crates. Rejected: the dependency's breaking releases still break moq through that type.
  • A moq-owned status type for reject. Rejected: u16 is moq-tokio's existing spelling for an HTTP status (Error::status()), and the method already validated the range at runtime.
  • Callers adding tikv-jemallocator themselves, as the quest's Plan expected. Rejected in favor of jemalloc::Allocator; see the choices below.

Choices made without asking

  • jemalloc::Allocator instead of callers depending on tikv-jemallocator. The Plan listed tikv_jemallocator among the reasons that "justify the dependency, not the re-export", but also says to wrap where that removes the crate from moq's API. A four-method forwarding GlobalAlloc does that, keeps the binaries' manifests unchanged (moq-relay, moq-cli, moq-boy), and guarantees the profiled allocator is the installed one. Otherwise three binaries would each need an optional tikv-jemallocator dependency, and moq-tokio's own (kept only for its profiling features) would need a cargo-shear exemption.
  • reject(u16): see Alternatives. HandshakeError::Status changed with it, so http leaves moq-tokio's and moq-uring's own signatures entirely.
  • Kept, not wrapped (checked as the Plan asked): cli::Merge/answer (usage-rs's traits are derived on every moq-tokio config struct, so a wrapper would remove nothing), the axum routers (callers merge them into their own axum app), url::Url (moq-tokio takes it throughout too), moq-archive's object store (the archive is generic over ObjectStore and its paths), moq-video's platform handles (the point is interop with the platform API), and jni::Env (the only alternative is raw pointers, an unsafe API).
  • mp4-atom and tungstenite: moq-dev/moq#8214 (open) plans media and transport quests that remove these crates from moq-mux's and qmux's signatures. Neither has run, so this PR drops only the pub use lines, as the quest said. Those quests' remaining work (mp4-atom types in moq-mux's errors, tungstenite types in qmux's Client/Upgraded/Error) is unchanged. The perf quests that described qmux's tungstenite re-exports now say "names ... types".
  • Workspace entries for jni, tokio-tungstenite, and wgpu, with moq-tokio's, qmux's, and moq-video's pins moved onto them. Other crates' existing direct pins (rustls in nine crates, for example) are left alone.

Follow-ups

  • Plan: speed up or move to nightly mesh_withdraw::{random,capped_random}_meshes_retract_once (200 seeded meshes each, 40-45s alone on main in debug, over the 60s nextest cap under full-suite load). .config/nextest.toml says the slowest honest test runs in about 9s. Not this PR's: it predates the branch.

Checks

  • just fix origin/main: passed (fmt, clippy fix, cargo sort, hakari, shear).
  • just ci origin/main on 7f57d72: every module passed except test js. Rust: check rs and test rs (13709 tests) passed.
  • The test js failure isn't this branch's. One @moq/net test (a connection samples the transport only while a reservation reads the estimate, js/net/src/connection/bandwidth.test.ts:156, expected 3 samples, got 0) fails in this worktree whenever it shares a run with ticker.test.ts, in either order. In a fresh worktree, bun run test in js/net passes 3129/3129 on this branch's head (c1c2f26) and on main (08a1f8a), the ticker-then-bandwidth pair included. The branch changes no JS. I didn't find the local state behind it: reinstalling frozen dependencies and deleting js/*/dist didn't change the result.
  • cargo clippy --workspace --all-targets --all-features -D warnings and RUSTDOCFLAGS=-D warnings cargo doc --workspace --all-features --no-deps (both excluding fuzz): passed.
  • Code behind macOS, Windows, and Android cfgs doesn't build on Linux. I checked every changed file's newly named crates against its package's dependencies: moq-ffi's Android JNI_OnLoad now has its own jni, and moq-video's Windows example uses moq-video's own windows dependency.
  • test/interop/compat/container.rs built against the current hang (path) and the released hang 0.21.11 with moq-net resolved by cargo tree.
  • quest check --base origin/main: 615 documents ok.

After merging main (a928b3f, then f4c32b6) and the review fixes (56da615):

  • The first merge resolved one conflict in rs/moq-c/src/consume.rs (main split the frame loop into run_group, which named moq_net::kio), and main's new an_open_waits_only_for_stream_credit test in rs/moq-uring/tests/integration/web.rs named quic::web::proto; both now name the crates directly. The second kept quest/m2/api-shape/reexports.md deleted (main had reworded it) and quest/m2/moq-quic-unused-surface.md deleted (main finished it in moq-dev/moq#8302).
  • just fix origin/main: passed.
  • just check origin/main on f4c32b6: every module passed except test rs, where 9958 of 13812 tests ran and two hit nextest's 60s cap: moq-net::integration mesh_withdraw::{random,capped_random}_meshes_retract_once. That's pre-existing. On main (961491d) the pair run alone takes 40s and 45s (both SLOW), and this branch changes neither test nor moq-net's logic. On a heavily loaded host the full suite pushes them past 60s. An earlier run's check py failure (a startup race creating the target symlink) passed on rerun and in this run.

Models

  • Size: L
  • Implementer: Claude Code, claude-opus-5-5, xhigh effort
  • Implementer: Claude Code, claude-opus-5-5, high effort (iterate, L)

🤖 Generated with Claude Code

(Written by Claude Opus 5.5 with xhigh)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, high effort
  • Session: 01a1be67-8603-79b1-85e8-852dd2272b2d, round 2
  • Head: f4c32b6
  • Base: origin/main at 961491d
  • Score: 9/10, No actionable defects were found in the merge or its interaction with the re-export removals, though builds and tests were not run as instructed.
  • Findings: none

kixelated and others added 3 commits November 7, 2026 20:22
Drop every whole-crate `pub use` from moq's crates, moq's own included,
so a breaking release of a dependency is no longer a semver break of the
crates that re-exported it. Callers name the crate as their own
dependency.

Wrap the few third-party types a signature named only in passing:
WebTransport `reject` takes a u16 status and `HandshakeError::Status`
carries one (moq-tokio and moq-uring), `watch::Files::new` returns
moq-tokio's error, and `moq_tokio::jemalloc::Allocator` replaces the
`tikv_jemallocator` re-export. moq-mux's `container` re-exports the
`Guard` that `Producer::modify` returns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title Crates re-export no whole crate refactor!: crates re-export no whole crate Nov 8, 2026
@kixelated
kixelated marked this pull request as ready for review November 8, 2026 05:59

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed c1c2f26.

No actionable findings. The direction follows the planned API boundary: callers name their dependencies directly, while small owned surfaces replace the incidental status, watcher-error and allocator exports. Both WebTransport reject paths preserve the 400–599 check before sending; the allocator forwards all four GlobalAlloc operations; the watcher preserves diagnostic details without exporting notify's result type. The container Guard becomes nameable, and the compatibility adapter selects the moq-net version resolved by released hang rather than mixing it with the checkout.

The remaining third-party types in foundational signatures are explicitly documented choices; removing whole-crate exports does not itself remove those dependencies from the public type graph. Migration guidance is in Impact rather than release guides.

Verification limits: GitHub-only static review of the API changes, caller/dependency rewrites, substantive wrapper implementations and compatibility harness. No compilation, feature-matrix, platform, allocator, filesystem-watch or interoperability tests run; author-reported checks are not independently verified. Rechecked open/non-draft state, full head and published reviews immediately before this COMMENT.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — two leftovers from the migration, both one-liners.

Reviewed changes

  • Whole-crate re-exports removed — the 28 pub use <crate> sites (and their as aliases) are gone from the twelve crates, and the ~150 callers in apps, bindings, tests, benches, and the interop clients name the crate directly. No whole-crate pub use, stale crate::<removed> path, or stale doc//README reference remains under the tree.
  • Incidental third-party types owned — Request::reject(u16) with HandshakeError::Status(u16) in moq-tokio and moq-uring (400..=599 validation unchanged, tests extended with 600/99), watch::Files::new -> moq_tokio::Result<Files> with Error::Watch(String), and jemalloc::Allocator forwarding all four GlobalAlloc operations.
  • container::Guard named — moq-mux re-exports the type Producer::modify returns.
  • Compatibility harness — the interop adapter now takes moq-net itself: the checkout path for current hang and the cargo tree-resolved version for released hang, so hang::moq_net leaving the API is covered on both sides.
  • Quest completed — api-shape/reexports.md deleted with its milestone entry, and the two remaining quests that leaned on it reworded.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Comment thread rs/moq-tokio/src/watch.rs Resolved
Comment thread rs/moq-relay/Cargo.toml Resolved
kixelated and others added 3 commits November 8, 2026 17:33
…exports

# Conflicts:
#	rs/moq-c/src/consume.rs
…tures

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…exports

# Conflicts:
#	quest/m2/api-shape/reexports.md
#	quest/m2/moq-quic-unused-surface.md

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed 56da615.

No actionable findings in the substantive updates since c1c2f26. Compared against the current base to separate main's changes. The relay now explicitly enables its direct tokio-tungstenite connect/handshake features, and the private notify-returning watcher helper lets TLS wrap the original diagnostic once while the public constructor retains moq-tokio's error type. Main's new C consumer and WebTransport test paths are migrated to direct dependencies consistently.

Verification: GitHub-only static review; no builds, feature-matrix, watcher, TLS, or interoperability tests run. Author-reported checks were not independently verified.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

Changes since the prior pullfrog review at c1c2f265f.

  • Cert-watch errors now carry one prefix — watch::Files gained a crate-private watch(paths) -> notify::Result<Self>; Files::new wraps it once in crate::Error::Watch, and tls::Reload::spawn calls watch directly, restoring failed to watch TLS material: <notify detail>.
  • moq-relay declares its own tungstenite features — tokio-tungstenite = { workspace = true, features = ["connect", "handshake"], optional = true }, matching qmux's declaration rather than inheriting it.
  • Main merged with a faithful resolution — moq-c/src/consume.rs's run_group names kio::wait, and main's new an_open_waits_only_for_stream_credit test names web_transport_proto directly. Per-file numstat of the branch change is identical before and after the merge except for that test, so neither side lost work.
  • Quest milestone README — dropped the completed reexports.md entry while keeping main's new JS/bindings entries.

Re-verified that no whole-crate pub use (or as alias) remains anywhere in the tree and that no reference to a removed path survives in rs/, test/, doc/, or the bindings trees.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

No commits landed since the prior pullfrog review at 56da6156d, so the incremental delta is empty. Both prior pullfrog threads are resolved: moq-relay now declares tokio-tungstenite's connect/handshake features itself (rs/moq-relay/Cargo.toml), and watch::Files gained a crate-private watch returning notify's own error so TLS surfaces a single failed to watch TLS material: <detail> prefix (rs/moq-tokio/src/watch.rs, rs/moq-tokio/src/tls.rs). The branch is unchanged from the approved state.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant