nng-sys: upgrade to NNG 2.0.0-alpha.6#34
Conversation
BREAKING CHANGES: - nng_init() must be called before using any NNG functions - nng_close() renamed to nng_socket_close() - nng_send_aio()/nng_recv_aio() renamed to nng_socket_send()/nng_socket_recv() - Many functions now return nng_err enum instead of c_int - ZeroTier transport removed (NNG_AF_ZT, nng_sockaddr_zt, nng_zt_*) - nanomsg compatibility layer removed (compat feature and headers) - NNG_FLAG_ALLOC removed - Many option getter/setter functions removed or renamed - TLS must now be configured via nng_tls_config_* functions - URL structure is now opaque with accessor functions For the complete migration guide, see: https://github.com/nanomsg/nng/blob/master/docs/ref/migrate/nng1.md Changes: - Bump nng-sys version to 0.4.0+v2.0.0-alpha.6 - Bump nng-sys/nng submodule to v2.0.0-alpha.6 - Regenerate bindings for NNG 2.0 API - Simplify wrapper.h (now just nng.h and http.h) - Remove compat.h and supplemental.h - Update build.rs: remove compat feature handling, bump pkg-config to 2.0.0 - Add NNG_AF_ABSTRACT to nng_sockaddr_family conversion - Update README with NNG 2.0 warning, migration guide, and fixed examples - Update tests for NNG 2.0 API (nng_init, nng_socket_close, nng_err) Note: The `links = "nng"` attribute is temporarily commented out because anng and nng crates still depend on nng-sys 0.3.0 from crates.io.
📝 WalkthroughWalkthroughUpgrade nng-sys to a v0.4 pre-release targeting NNG 2.0-alpha: regenerate large bindings (HTTP/TLS/URL/error API surface), remove legacy compat/supplemental wiring and local path overrides, add CI matrix flag/steps to optionally build/install NNG, and add an install script for the nng submodule. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant Matrix as "Job Matrix"
participant PM as "Package Manager (apt / brew)"
participant Script as ".github/scripts/install-nng.sh"
participant Sub as "nng-sys/nng (submodule)"
participant CMake as "CMake / Build"
participant Cargo as "cargo / cargo-hack"
GH->>Matrix: start job
Matrix->>PM: install base packages
alt matrix.install_nng == true
Matrix->>Script: run install-nng.sh
Script->>Sub: cd into submodule
Script->>CMake: configure & build NNG
CMake-->>Script: produce shared libs
Script->>PM: sudo install & ldconfig (Linux)
end
Matrix->>Cargo: run per-crate cargo-hack checks & builds
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✏️ Tip: You can disable this entire section by setting 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. Comment |
- Build NNG from nng-sys/nng submodule instead of Debian packages (v1.x) - Update linux-sys 'system + bindgen' job to build NNG from source - Update linux-nng job to build NNG from source - Update macOS jobs to build NNG from submodule (consolidate to 2 configs) - Remove compat and supplemental test jobs (features don't exist in NNG 2.0)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WORKAROUND: MSVC doesn't allow empty structs in C (error C2016), and NNG has a bug where nng/src/core/stats.h defines an empty nni_stat_item struct when NNG_ENABLE_STATS is OFF. Until this is fixed upstream, we must always enable stats when building with MSVC to avoid the empty struct compilation error.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
nng-sys/build.rs (2)
253-275: Empty include paths can cause bindgen failure when system defaults succeed.The pipeline failure (
'nng/http.h' file not found) stems from this code path. Whentry_compile_probe_without_paths()succeeds butCFLAGSis empty or lacks-Iflags,include_dirswill be empty. Whilecc::Buildcan find headers via system default paths, bindgen requires explicit-Iflags.Proposed fix: probe for common system include paths
if try_compile_probe_without_paths() { // Success! The system compiler can find NNG without any help. - // No need to emit link-search or metadata paths. println!("cargo:warning=found NNG via system compiler defaults"); let mut include_dirs = Vec::new(); let cflags = get_env_var("CFLAGS").unwrap_or_default(); let mut cflags = cflags.split_whitespace(); while let Some(flag) = cflags.next() { if let Some(mut path) = flag.strip_prefix("-I") { if path.is_empty() { // -I $path let Some(next_path) = cflags.next() else { break; }; path = next_path; } else { // -I$path } include_dirs.push(path.into()); } } + // If no explicit include paths found, check common system locations + if include_dirs.is_empty() { + for candidate in &["/usr/include", "/usr/local/include"] { + let nng_header = Path::new(candidate).join("nng/nng.h"); + if nng_header.exists() { + include_dirs.push(PathBuf::from(candidate)); + break; + } + } + } return Some((LibrarySource::Manual, include_dirs)); }
319-336: The pkg-config version check will fail to detect NNG 2.0.0-alpha.6.The change at line 323 uses
.atleast_version("2.0.0"), but pkg-config treats pre-release versions (e.g., "2.0.0-alpha.6") as less than the final release version. This causes thetry_pkg_config()probe to fail silently, forcing the build to fall back to system compiler defaults or the vendored source. This is likely the root cause of the pipeline failure reporting'nng/http.h' file not found.Update line 323 to either:
.atleast_version("2.0.0-alpha")to match pre-release versions, or.atleast_version("1.99.0")to accept all 2.0 pre-releasesnng-sys/src/lib.rs (1)
9-46: Checknng_initreturn value in the example.The example should check the return value of
nng_init, which returnsnng_err. Useassert_eq!(nng_err::NNG_OK, nng_init(null_mut()));as shown in the test suite.nng-sys/tests/tests.rs (1)
12-49: Callnng_init()beforenng_version()in the version test; fix version parsing to handle dotted suffixes; remove debug statement.
verify_runtime_version_matches_constants()violates NNG 2.0's documented requirement thatnng_init()must be called before any NNG function. This test callsnng_version()without initialization, which can cause flakiness or undefined behavior.Additionally:
- Line 86–87:
split('.')with hard-coded 3-part expectation breaks for versions with dotted suffixes (e.g.,"1.0.0alpha.6"). Usesplitn(3, '.')instead.- Line 102: Remove the
dbg!(&runtime_suffix);statement.Proposed fix
mod tests { - use nng_sys::*; use std::{ ffi::{CStr, CString}, os::raw::c_char, ptr::null_mut, + sync::Once, }; + static NNG_INIT: Once = Once::new(); + + fn ensure_nng_init() { + NNG_INIT.call_once(|| unsafe { + assert_eq!(nng_err::NNG_OK, nng_init(core::ptr::null())); + }); + } + #[test] fn basic() { - unsafe { - // Initialize NNG (required in NNG 2.0) - assert_eq!(nng_err::NNG_OK, nng_init(null_mut())); + ensure_nng_init(); + unsafe { let url = CString::new("inproc://nng_sys/tests/basic").unwrap(); let url = url.as_bytes_with_nul().as_ptr() as *const c_char;#[test] fn verify_runtime_version_matches_constants() { + ensure_nng_init(); unsafe {- let parts: Vec<&str> = version_str.split('.').collect(); - assert_eq!(parts.len(), 3, "Version string should have 3 parts"); - - let runtime_major: u32 = parts[0].parse().expect("a valid major version"); - let runtime_minor: u32 = parts[1].parse().expect("a valid minor version"); - // The patch part may contain a suffix like "0dev", so extract only the numeric prefix - let patch_numeric: String = parts[2] + let mut parts = version_str.splitn(3, '.'); + let runtime_major: u32 = parts.next().unwrap().parse().expect("a valid major version"); + let runtime_minor: u32 = parts.next().unwrap().parse().expect("a valid minor version"); + let patch_and_suffix = parts.next().unwrap_or(""); + + // The patch part may contain a suffix like "0dev" or "0alpha.6" + let patch_numeric: String = patch_and_suffix .chars() .take_while(|c| c.is_ascii_digit()) .collect(); let runtime_patch: u32 = patch_numeric.parse().expect("a valid patch version"); - let runtime_suffix: String = parts[2] + let runtime_suffix: String = patch_and_suffix .chars() .skip_while(|c| c.is_ascii_digit()) .collect(); - - dbg!(&runtime_suffix);
🤖 Fix all issues with AI agents
In @.github/workflows/build-configurations.yml:
- Around line 19-56: The workflow misses an apt-get update before installs,
fails to export the NNG installation location after building the submodule, and
the linux-nng job installs Ubuntu libnng1/libnng-dev (NNG 1.x) which is
incompatible with nng-sys 0.4.0 (NNG 2.0.0-alpha.6). Fix by: add a sudo apt-get
update step before any apt installs in each matrix entry (e.g., for "system +
bindgen" and others), after building NNG from submodule write the install prefix
into $GITHUB_ENV (export NNG_DIR or NNG_INCLUDE_DIR/NNG_LIB_DIR) so build.rs
sees /usr or /usr/local, and update the "linux-nng" job to either build the
submodule NNG (set build_nng true/use same build steps as the submodule entries)
or change the job to document/explicitly test NNG 1.x compatibility if that is
intentional.
In @nng-sys/README.md:
- Around line 9-19: Update the README examples that call nng_init(null_mut()) to
check the returned nng_err rather than ignoring it: replace the bare
nng_init(null_mut()) calls in the examples with an assertion or conditional that
compares the result to nng_err::NNG_OK (e.g., assert_eq!(nng_err::NNG_OK,
nng_init(null_mut())) or an if/expect that logs/returns on error), so both
example blocks referencing nng_init(null_mut()) follow the same error-checking
pattern used in tests/tests.rs.
In @nng-sys/src/bindings.rs:
- Around line 23-31: NNG_OPT_UDP_BOUND_PORT is incorrectly set to the TCP
string; update the constant NNG_OPT_UDP_BOUND_PORT so its byte string is
"udp-bound-port\0" (matching the UDP option name and length) instead of the
current "tcp-bound-port\0" to avoid silent misconfiguration when querying UDP
options.
🧹 Nitpick comments (2)
nng-sys/build.rs (2)
448-477: Consider validating include paths before invoking bindgen.When
include_dirsis empty, bindgen will fail with cryptic clang errors. Adding early validation would provide a clearer error message.Suggested validation
fn generate_bindings(include_dirs: &[PathBuf]) { + // Validate that we have include paths with NNG headers + let has_nng_header = include_dirs.iter().any(|dir| dir.join("nng/nng.h").exists()); + if !has_nng_header && !include_dirs.is_empty() { + println!("cargo:warning=include paths don't contain nng/nng.h, bindgen may fail"); + } + let mut builder = bindgen::Builder::default()
75-82: Stale comment references removed features.The comment on line 77 mentions
compatandsupplementalfeatures, but these were removed as part of the NNG 2.0 migration. Consider updating or removing this comment.// 6. Regenerate bindings if they aren't guaranteed to match pre-generated ones. // For example, the system-provided library could be a newer or older version. - // We don't need to check for `compat` or `supplemental` directly here as they imply `bindgen`. if cfg!(feature = "bindgen") || !matches!(source, LibrarySource::Vendored) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.github/workflows/build-configurations.ymlanng/Cargo.tomlnng-sys/Cargo.tomlnng-sys/README.mdnng-sys/build.rsnng-sys/nngnng-sys/src/bindings.rsnng-sys/src/compat.hnng-sys/src/lib.rsnng-sys/src/supplemental.hnng-sys/src/wrapper.hnng-sys/tests/tests.rsnng/Cargo.toml
💤 Files with no reviewable changes (4)
- anng/Cargo.toml
- nng/Cargo.toml
- nng-sys/src/supplemental.h
- nng-sys/src/compat.h
🧰 Additional context used
🧬 Code graph analysis (2)
nng-sys/tests/tests.rs (1)
nng-sys/src/bindings.rs (2)
nng_init(1686-1686)nng_socket_close(275-275)
nng-sys/src/bindings.rs (3)
nng/src/socket.rs (1)
nng_socket(530-532)nng/src/aio.rs (1)
nng_aio(448-450)nng/src/pipe.rs (1)
nng_pipe(97-99)
🪛 GitHub Actions: check
nng-sys/build.rs
[error] 482-482: unable to generate bindings: ClangDiagnostic("src/wrapper.h:2:10: fatal error: 'nng/http.h' file not found")"
nng-sys/src/wrapper.h
[error] 2-2: fatal error: 'nng/http.h' file not found
nng-sys/Cargo.toml
[error] 1-1: Build failed due to missing NNG headers required for bindings generation.
🪛 LanguageTool
nng-sys/README.md
[style] ~101-~101: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...xist. Note: Versions of this crate prior to 0.3.0 used a different scheme (`<NNG_...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
🔇 Additional comments (6)
nng-sys/nng (2)
1-1: The submodule commit hash has been verified and correctly matches the v2.0.0-alpha.6 release tag.
1-1: File path is incorrect; the version is innng-sys/Cargo.tomlline 3, notnng-sys/nng.The stability concern about NNG v2.0.0-alpha.6 is valid and already documented prominently in
nng-sys/README.mdwith a dedicated "⚠️ NNG 2.0 Alpha" section that explains breaking changes and includes a migration guide for users upgrading from NNG 1.x. As of January 2026, NNG 2.0.0 has not been released as a stable version; the latest stable release remains v1.11 (June 2025). Developers should review the README migration guide before adopting this crate.Likely an incorrect or invalid review comment.
nng-sys/src/bindings.rs (1)
3-6: Generated NNG 2.x surface looks consistent (init params +nng_errmodel).The move to an
nng_errenum plus explicitnng_init/nng_finientrypoints matches the stated upstream breaking changes; just ensure the generated signatures align with the exact NNG revision you vendored.Also applies to: 96-134, 1674-1690
nng-sys/src/lib.rs (1)
159-170:NNG_AF_ABSTRACTconversion addition looks right.nng-sys/Cargo.toml (1)
3-3: Re-enablelinks = "nng"once downstream crates are updated to nng-sys v0.4.0.The
links = "nng"attribute is appropriately commented out for now due to workspace compatibility constraints with downstream crates (anngandnng), but should be re-enabled before this version ships broadly to avoid potential linker conflicts in downstream workspaces. Thepkg-config = "0.3.32"dependency is compatible with the current build.rs implementation.Likely an incorrect or invalid review comment.
nng-sys/src/wrapper.h (1)
1-2: Thenng/http.hinclude is correct and intentional for NNG 2.0.0-alpha.6—no fix needed.The wrapper.h file was recently updated as part of the NNG 2.0.0-alpha.6 upgrade (commit 62c0403). The simplified header file now includes only
<nng/nng.h>and<nng/http.h>, which is intentional. The bindings have been successfully generated with 94 HTTP-related functions and types, confirming thathttp.his available and correctly included. No CI failures are reported in any workflow configurations or logs.Likely an incorrect or invalid review comment.
On macOS, /usr/local/lib is not in the default library search path. When we install NNG to /usr/local, the linker can't find libnng unless we explicitly tell Rust where to look. Add RUSTFLAGS=-L native=/usr/local/lib to the macOS system nng job.
Create .github/scripts/build-nng.sh to handle OS-specific NNG build from the submodule. This reduces duplication between Linux and macOS jobs - the script auto-detects the OS and handles: - Install prefix (/usr on Linux, /usr/local on macOS) - Parallel job count (nproc vs sysctl -n hw.ncpu) - ldconfig on Linux only
Replace apt-installed libnng1/libnng-dev packages with building NNG from the nng-sys/nng submodule. This ensures the hack job tests against the exact NNG version tracked in the repository when checking all feature combinations.
flxo
force-pushed
the
nng-sys/nng-v2
branch
2 times, most recently
from
dde2598 to
016df71
Compare
nng-sys v0.4 targets NNG v2, while nng and anng still depend on nng-sys v0.3 (NNG v1). Running feature-powerset on the entire workspace fails since these packages require incompatible NNG versions. This workaround runs cargo hack on each package individually: - nng-sys: Install NNG v2 from submodule, set NNG_DIR=/usr - nng/anng: Use vendored NNG v1 (builds from source, no NNG_DIR needed)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build-configurations.yml:
- Around line 121-127: The GitHub Actions job sets RUSTFLAGS to add
/usr/local/lib but doesn't expose header search paths, so update the job's env
block (the same section that defines RUSTFLAGS) to also set either CFLAGS:
-I/usr/local/include or NNG_INCLUDE_DIR: /usr/local/include so install-nng.sh /
build.rs can find headers; modify the env map alongside RUSTFLAGS to include one
of those keys (CFLAGS or NNG_INCLUDE_DIR) to ensure header discovery on macOS.
♻️ Duplicate comments (1)
.github/workflows/build-configurations.yml (1)
61-66: Missingapt-get updatebefore package installation.The
apt-get installcommand runs without a priorapt-get update, which can cause intermittent failures on GitHub runners when package indices are stale.✏️ Suggested fix
- name: Install dependencies run: | + sudo apt-get update sudo apt-get install -y ${{ matrix.apt }}
🧹 Nitpick comments (5)
.github/scripts/install-nng.sh (2)
10-16: Consider adding a fallback for CPU count detection.If
nprocorsysctlfails or is unavailable,JOBSwill be empty. Whilemake -j""typically defaults to sequential builds, explicitly setting a fallback improves robustness.🔧 Suggested improvement
# OS-specific settings if [[ "$OSTYPE" == "darwin"* ]]; then PREFIX="/usr/local" - JOBS=$(sysctl -n hw.ncpu) + JOBS=$(sysctl -n hw.ncpu 2>/dev/null || echo 2) else PREFIX="/usr" - JOBS=$(nproc) + JOBS=$(nproc 2>/dev/null || echo 2) fi
18-20: Optional: Use Ninja for faster builds.Since
ninja-buildis already installed in the workflow (line 123 of check.yml), you could leverage it for faster parallel builds.⚡ Optional: Use Ninja generator
-cmake -DCMAKE_INSTALL_PREFIX="$PREFIX" -DBUILD_SHARED_LIBS=ON \ - -DNNG_TESTS=OFF -DNNG_TOOLS=OFF .. -make -j"$JOBS" +cmake -G Ninja -DCMAKE_INSTALL_PREFIX="$PREFIX" -DBUILD_SHARED_LIBS=ON \ + -DNNG_TESTS=OFF -DNNG_TOOLS=OFF .. +ninjanng-sys/build.rs (2)
75-78: Stale comment references removed features.The comment on line 77 mentions
compatandsupplementalfeatures, but these were removed as part of the NNG 2.0 upgrade. The comment should be updated to reflect the current feature set.✏️ Suggested fix
// 6. Regenerate bindings if they aren't guaranteed to match pre-generated ones. // For example, the system-provided library could be a newer or older version. - // We don't need to check for `compat` or `supplemental` directly here as they imply `bindgen`. if cfg!(feature = "bindgen") || !matches!(source, LibrarySource::Vendored) {
145-152: Consider removing debug print statement.The
println!("version: {version}")on line 146 will output to cargo build logs for every build. If this was added for debugging, consider removing it or gating it behind a verbose flag.✏️ Suggested fix
fn parse_version(version: &str) -> (u64, u64, u64) { - println!("version: {version}"); let mut it = version.split('_'); let major = it.next().unwrap().parse::<u64>().unwrap(); let minor = it.next().unwrap().parse::<u64>().unwrap(); let patch = it.next().unwrap().parse::<u64>().unwrap(); (major, minor, patch) }nng-sys/tests/tests.rs (1)
46-47: Consider asserting return values for consistency.Other NNG calls in this test use
assert_eq!(0, ...)to verify success. While socket close failures during test teardown are unlikely to cause issues, checking the return value would be consistent with the rest of the test.✏️ Suggested fix
- nng_socket_close(req_socket); - nng_socket_close(rep_socket); + assert_eq!(0, nng_socket_close(req_socket)); + assert_eq!(0, nng_socket_close(rep_socket));
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/scripts/install-nng.sh.github/workflows/build-configurations.yml.github/workflows/check.ymlanng/Cargo.tomlnng-sys/Cargo.tomlnng-sys/build.rsnng-sys/tests/tests.rsnng/Cargo.toml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: flxo
Repo: nanomsg/nng-rs PR: 34
File: nng-sys/src/bindings.rs:23-31
Timestamp: 2026-01-13T10:08:20.101Z
Learning: In NNG 2.0 (nng-sys 0.4.0+v2.0.0-alpha.6), the C header nng.h defines NNG_OPT_UDP_BOUND_PORT as a macro alias to NNG_OPT_TCP_BOUND_PORT, so both Rust constants correctly point to the same string value b"tcp-bound-port\0" in the generated bindings.
📚 Learning: 2026-01-13T10:08:20.101Z
Learnt from: flxo
Repo: nanomsg/nng-rs PR: 34
File: nng-sys/src/bindings.rs:23-31
Timestamp: 2026-01-13T10:08:20.101Z
Learning: In NNG 2.0 (nng-sys 0.4.0+v2.0.0-alpha.6), the C header nng.h defines NNG_OPT_UDP_BOUND_PORT as a macro alias to NNG_OPT_TCP_BOUND_PORT, so both Rust constants correctly point to the same string value b"tcp-bound-port\0" in the generated bindings.
Applied to files:
nng-sys/build.rs
🧬 Code graph analysis (1)
nng-sys/tests/tests.rs (1)
nng-sys/src/bindings.rs (2)
nng_init(1686-1686)nng_socket_close(275-275)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: nng-sys on Windows --features vendored
- GitHub Check: ubuntu / stable / features
🔇 Additional comments (13)
anng/Cargo.toml (1)
29-33: LGTM! Temporary workaround for the NNG v1/v2 transition.The TODO comment clearly documents why the path dependency is commented out. This is a reasonable approach while
anngstill depends on nng-sys v0.3 (NNG v1) and cannot yet migrate to v0.4 (NNG v2).nng/Cargo.toml (1)
29-33: LGTM! Consistent with the anng workaround.Same temporary approach as
anng/Cargo.toml, keeping the path dependency commented until the crate is migrated to nng-sys v0.4..github/workflows/check.yml (1)
123-150: Well-documented workaround for the NNG v1/v2 transition.The extensive comments clearly explain the rationale for per-package cargo hack invocations. The workflow correctly:
- Installs NNG v1 system libs for
nng/anngfeature-powerset checks (including non-vendored combinations)- Removes NNG v1 before installing NNG v2
- Runs
nng-syschecks against NNG v2 from the submoduleThe TODO on lines 130-133 provides a clear path for reverting once the migration is complete.
nng-sys/Cargo.toml (2)
18-29: Temporary workaround acknowledged, but be aware of the implications.The detailed comment explains the Cargo limitation well. However, commenting out
links = "nng"removes Cargo's protection against duplicate native library linking. During this transition period where both nng-sys 0.3 and 0.4 coexist in the workspace, ensure that:
- CI jobs don't accidentally link both versions in the same binary
- The
linksattribute is restored oncenngandanngmigrate to v0.4The current per-package cargo-hack approach in CI (check.yml) correctly isolates the builds, so this should be safe for now.
1-17: Version bump and author addition look correct.The version
0.4.0+v2.0.0-alpha.6appropriately reflects the major API change (NNG 2.0) with the upstream version as build metadata. Therust-version = "1.82.0"requirement aligns with the use of unsafe extern blocks in generated bindings.nng-sys/build.rs (2)
319-336: LGTM!The pkg-config version bump to 2.0.0 correctly ensures system NNG discovery only succeeds for NNG 2.x installations, preventing accidental linkage against incompatible NNG 1.x libraries.
385-396: LGTM!The MSVC workaround is well-documented with the upstream issue reference. The conditional logic correctly enables stats when either the feature is explicitly requested or when targeting MSVC to avoid the empty struct compilation error.
nng-sys/tests/tests.rs (3)
14-16: LGTM!The
nng_init(null_mut())call is correctly added at the start of the test, fulfilling the NNG 2.0 requirement that initialization must occur before using any NNG functions.
67-73: LGTM!The version string construction correctly incorporates
NNG_RELEASE_SUFFIXusingCStr::from_bytes_with_nul_unchecked. Since this is a compile-time constant from the generated bindings, the safety invariants (null-terminated, valid content) are upheld. Usingto_string_lossyprovides additional safety for display.
91-115: LGTM!The patch version parsing correctly handles release suffixes (e.g., "0dev" → patch=0, suffix="dev"). The logic cleanly splits numeric and suffix portions, and the suffix comparison mirrors the construction approach for consistency.
.github/workflows/build-configurations.yml (3)
74-92: LGTM!The explicit comment clarifies that the
nngcrate has not yet been upgraded to NNG v2, so this job intentionally tests against system-provided NNG 1.x. This cleanly separates testing concerns betweennng-sys(NNG 2.0) andnng(NNG 1.x).
19-56: LGTM!The
build_nngmatrix flag cleanly distinguishes between system NNG builds (from submodule) and vendored builds. The matrix structure is well-organized and maintainable.
137-142: LGTM!The macOS dependency installation and conditional NNG build steps follow the same clear pattern as Linux. Homebrew's auto-update behavior means no explicit update step is needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
bump
NNGto2.0.0-alpha.6BREAKING CHANGES:
nng_init()must be called before using any NNG functionsnng_close()renamed tonng_socket_close()nng_send_aio()/nng_recv_aio()renamed tonng_socket_send()/nng_socket_recv()nng_errenum instead ofc_intNNG_AF_ZT,nng_sockaddr_zt,nng_zt_*)compatfeature and headers)NNG_FLAG_ALLOCremovednng_tls_config_*functionsFor the complete migration guide, see the migration guilde.
Changes:
nng-sysversion to0.4.0+v2.0.0-alpha.6nng-sys/nngsubmodule tov2.0.0-alpha.6wrapper.h(now justnng.handhttp.h)compat.handsupplemental.hbuild.rs: remove compat feature handling, bumppkg-configto2.0.0NNG_AF_ABSTRACTtonng_sockaddr_familyconversionNNG2.0 warning, migration guide, and fixed examplesnng_init,nng_socket_close,nng_err)Note: The
links = "nng"attribute is temporarily commented out becauseanngandnngcrates still depend onnng-sysversion 0.3.0 from crates.io and cargo treats this as a conflict.Summary by CodeRabbit
New Features
Bug Fixes & Improvements
CI / Chores
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.