Repository navigation
refactor!: crates re-export no whole crate - #8247
Conversation
Quest-Claim: 98781f6b892c40d6
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
left a comment
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
ℹ️ 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 theirasaliases) 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-cratepub use, stalecrate::<removed>path, or staledoc//README reference remains under the tree. - Incidental third-party types owned —
Request::reject(u16)withHandshakeError::Status(u16)in moq-tokio and moq-uring (400..=599 validation unchanged, tests extended with 600/99),watch::Files::new -> moq_tokio::Result<Files>withError::Watch(String), andjemalloc::Allocatorforwarding all fourGlobalAllocoperations. container::Guardnamed — moq-mux re-exports the typeProducer::modifyreturns.- Compatibility harness — the interop adapter now takes
moq-netitself: the checkout path for current hang and thecargo tree-resolved version for released hang, sohang::moq_netleaving the API is covered on both sides. - Quest completed —
api-shape/reexports.mddeleted with its milestone entry, and the two remaining quests that leaned on it reworded.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
…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
left a comment
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Changes since the prior pullfrog review at c1c2f265f.
- Cert-watch errors now carry one prefix —
watch::Filesgained a crate-privatewatch(paths) -> notify::Result<Self>;Files::newwraps it once incrate::Error::Watch, andtls::Reload::spawncallswatchdirectly, restoringfailed 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'srun_groupnameskio::wait, and main's newan_open_waits_only_for_stream_credittest namesweb_transport_protodirectly. 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.mdentry 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.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ 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.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

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'scontainer::Producer::modifyalso returned aGuardno path could name.Approach
pub useis gone, moq's own crates included (the census in the quest matched a fresh grep: 28 sites in 17 files).Request::rejecttakes au16HTTP status (moq-tokio'squic::weband moq-uring's), validated to 400..=599, andHandshakeError::Statuscarries au16. This matches moq-tokio's existingError::status() -> Option<u16>.httpno longer appears in either crate's own signatures.moq_tokio::watch::Files::newreturnsmoq_tokio::Result(newError::Watch(String)), notnotify::Result.moq_tokio::jemalloc::Allocatorreplaces thetikv_jemallocatorre-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 onejemalloc::runprofiles.moq_mux::containerre-exportsGuardbesideProducer.bytesand kio in moq-net, rustls configs (moq-quic, moq-rtmp, moq-tokio), web-transport-proto'sConnectRequest/ConnectResponse, usage-rs (every moq-tokio config derivesusage_rs::Args), axum routers (moq-hls, moq-webrtc),url::Url, moq-archive'sObjectStore, moq-video's platform handles and wgpu device, tokio-tungstenite in qmux, mp4-atom in moq-mux, moq-congestion'sFactory, andjni::Envfortls::init_android.{ workspace = true };jni,tokio-tungstenite, andwgpugained workspace entries, and their existing per-crate pins (moq-tokio, qmux, moq-video) moved onto them.test/interop/compat/container.rsagainst 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.lockuses, and import it directly instead of through moq's path.hang::moq_netis removed. Depend onmoq-net.moq_net::bytesandmoq_net::kioare removed. Depend onbytesandkio.moq_quic::rustlsis removed. Depend onrustls0.23.moq_rtmp::rustls(tlsfeature) is removed. Depend onrustls0.23.moq_tokio::{moq_net, rustls, notify, jni, congestion}are removed. Depend onmoq-net,rustls,jni(Android), andmoq-congestion(forquic::Controller).cli::usage_rsis removed. Depend onusage-rs.jemalloc::tikv_jemallocatoris removed. Usejemalloc::Allocatoras the#[global_allocator](new).quic::web::{http, proto}are removed. Depend onweb-transport-protoforConnectRequest/ConnectResponse(it re-exportshttp).quic::web::Request::rejecttakesu16instead ofhttp::StatusCode:reject(StatusCode::NOT_FOUND)becomesreject(404).quic::web::HandshakeError::Statuscarriesu16.watch::Files::newreturnsmoq_tokio::Result<Files>. The newError::Watch(String)variant (watchfeature) carries the failure.moq_uring::urlandquic::web::{http, proto}are removed. Depend onurlandweb-transport-proto.quic::web::Request::rejecttakesu16, andquic::web::HandshakeError::Statuscarriesu16, as in moq-tokio.moq_hls::axumis removed. Depend onaxum0.8.moq_webrtc::{axum, url}are removed. Depend onaxum0.8 andurl2.moq_archive::object_storeis removed. Depend onobject_store0.14.moq_video::{ndk, objc2_core_foundation, objc2_core_video, windows}andrender::wgpuare removed. Depend on the platform crate (ndk0.9,objc2-core-foundation/objc2-core-video0.3,windows0.62) orwgpu30.container::fmp4::mp4_atomis removed. Depend onmp4-atom.container::Guardis new (additive), the typeProducer::modifyreturns.ws::{tokio_tungstenite, tungstenite}are removed. Depend ontokio-tungstenite0.30, which re-exportstungstenite.Alternatives
pub use tikv_jemallocator::Jemalloc;) instead of whole crates. Rejected: the dependency's breaking releases still break moq through that type.reject. Rejected:u16is moq-tokio's existing spelling for an HTTP status (Error::status()), and the method already validated the range at runtime.tikv-jemallocatorthemselves, as the quest's Plan expected. Rejected in favor ofjemalloc::Allocator; see the choices below.Choices made without asking
jemalloc::Allocatorinstead of callers depending ontikv-jemallocator. The Plan listedtikv_jemallocatoramong 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 forwardingGlobalAllocdoes 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 optionaltikv-jemallocatordependency, and moq-tokio's own (kept only for itsprofilingfeatures) would need a cargo-shear exemption.reject(u16): see Alternatives.HandshakeError::Statuschanged with it, sohttpleaves moq-tokio's and moq-uring's own signatures entirely.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 overObjectStoreand its paths), moq-video's platform handles (the point is interop with the platform API), andjni::Env(the only alternative is raw pointers, an unsafe API).pub uselines, as the quest said. Those quests' remaining work (mp4-atom types in moq-mux's errors, tungstenite types in qmux'sClient/Upgraded/Error) is unchanged. The perf quests that described qmux's tungstenite re-exports now say "names ... types".jni,tokio-tungstenite, andwgpu, 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
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.tomlsays 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/mainon 7f57d72: every module passed excepttest js. Rust:check rsandtest rs(13709 tests) passed.test jsfailure isn't this branch's. One@moq/nettest (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 withticker.test.ts, in either order. In a fresh worktree,bun run testinjs/netpasses 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 deletingjs/*/distdidn't change the result.cargo clippy --workspace --all-targets --all-features -D warningsandRUSTDOCFLAGS=-D warnings cargo doc --workspace --all-features --no-deps(both excluding fuzz): passed.JNI_OnLoadnow has its ownjni, and moq-video's Windows example uses moq-video's ownwindowsdependency.test/interop/compat/container.rsbuilt against the current hang (path) and the released hang 0.21.11 with moq-net resolved bycargo tree.quest check --base origin/main: 615 documents ok.After merging main (a928b3f, then f4c32b6) and the review fixes (56da615):
rs/moq-c/src/consume.rs(main split the frame loop intorun_group, which namedmoq_net::kio), and main's newan_open_waits_only_for_stream_credittest inrs/moq-uring/tests/integration/web.rsnamedquic::web::proto; both now name the crates directly. The second keptquest/m2/api-shape/reexports.mddeleted (main had reworded it) andquest/m2/moq-quic-unused-surface.mddeleted (main finished it in moq-dev/moq#8302).just fix origin/main: passed.just check origin/mainon f4c32b6: every module passed excepttest 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'scheck pyfailure (a startup race creating thetargetsymlink) passed on rerun and in this run.Models
🤖 Generated with Claude Code
(Written by Claude Opus 5.5 with xhigh)
Review
Rewritten by
quest reviewafter each round.01a1be67-8603-79b1-85e8-852dd2272b2d, round 2No actionable defects were found in the merge or its interaction with the re-export removals, though builds and tests were not run as instructed.