Replace netdev with if-addrs to fix macOS compilation by ivonnyssen · Pull Request #16 · RReverser/ascom-alpaca-rs

ivonnyssen · 2026-04-07T01:09:17Z

Summary

Replaces the netdev dependency with if-addrs to fix a trait recursion overflow that prevents compilation on macOS.

Problem

netdev transitively depends on objc2, which defines a blanket IntoIterator impl for &Retained<T>. This causes infinite trait recursion when the Rust trait solver evaluates serde_ndim's NDim: IntoIterator bound during compilation of the camera image array serialization code. This is a known Rust compiler issue (rust-lang/rust#136856).

Solution

if-addrs provides the same network interface enumeration functionality using only libc (unix) / windows-sys (windows) — no objc2 dependency chain. This eliminates the root cause entirely.

The main adaptation is a GroupedInterface struct that re-groups if-addrs' per-address entries by interface name, since if-addrs returns one entry per address while the discovery code expects one entry per interface.

Motivation

This blocks running CI on macOS. The crate does not compile on macOS at all with the current netdev dependency.

Changes

Cargo.toml: Replace netdev with if-addrs
src/discovery.rs: Add GroupedInterface struct with get_active_interfaces() function; merge interface index across address entries so multicast isn't skipped when the first entry lacks an index
src/client/discovery.rs: Adapt to GroupedInterface API
src/server/discovery.rs: Adapt to GroupedInterface API, improve multi-homed network handling
Cargo.lock: Updated

Test plan

cargo clippy passes (full CI feature matrix verified on Linux)
Compiles on macOS
Discovery still works on multi-homed hosts
Loopback interfaces correctly excluded from discovery

netdev's transitive objc2 dependencies define a blanket IntoIterator impl for &Retained<T> that causes infinite trait recursion when the Rust trait solver evaluates serde_ndim's NDim: IntoIterator bound. Replacing netdev with if-addrs eliminates the root cause entirely, since if-addrs depends only on libc (unix) and windows-sys (windows). The main adaptation is a GroupedInterface struct that re-groups if-addrs' per-address entries by interface name to match the per-interface model the discovery code expects. Co-Authored-By: Claude Opus 4.6 <[email protected]>

- Fix clippy::match_wildcard_for_single_variants by using explicit IfAddr variants instead of _ wildcards - Change GroupedInterface.index to Option<u32> to avoid silently using index 0 (kernel picks interface) when if-addrs returns None; multicast operations now skip the interface with a warning instead - Accumulate is_loopback across all addresses on a grouped interface via OR, rather than only checking the first address seen - Replace test DEFAULT_ADDR with UDP socket trick to find the default route interface (matching old netdev::get_default_interface behaviour), ensuring both v4 and v6 test addresses come from the same interface Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Copilot

Pull request overview

This PR replaces the netdev crate with if-addrs for network interface enumeration to avoid a macOS compilation failure caused by an objc2-related trait recursion overflow, and updates discovery client/server code to use the new grouped interface representation.

Changes:

Swap dependency from netdev to if-addrs to remove the problematic macOS transitive dependency chain.
Introduce GroupedInterface + get_active_interfaces() that groups per-address if-addrs entries by interface.
Update discovery client/server code to use GroupedInterface and handle interface-index absence.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
Cargo.toml	Replaces `netdev` dependency with `if-addrs`.
Cargo.lock	Removes `netdev`/objc2-related transitive crates and adds `if-addrs`.
src/discovery.rs	Adds `GroupedInterface` + new interface enumeration; updates tests to find “default” interface IP without `netdev`.
src/client/discovery.rs	Updates client discovery to use grouped interface addresses and optional interface index.
src/server/discovery.rs	Updates server multicast joining to use `GroupedInterface` and new interface enumeration API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+                let Some(index) = intf.index else {
+                    tracing::warn!("skipping multicast send: no interface index");
+                    return Ok(());


+    let Some(index) = intf.index else {
+        tracing::warn!("skipping multicast join: no interface index");
+        return;
+    };
+    match socket.join_multicast_v6(&DISCOVERY_ADDR_V6, index) {
        Ok(()) => tracing::trace!("success"),
        Err(err) => tracing::warn!(%err),


When multiple address entries exist for the same interface, preserve the first available index rather than discarding it. This prevents multicast operations from being skipped when the first address entry has index=None but a later one provides Some. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

RReverser

This blocks running CI on macOS. The crate does not compile on macOS at all with the current netdev dependency.

If the goal is to fix compilation on macOS, please add a corresponding CI test. Merely running the discovery test on macOS as a separate job should be enough - no need to run the whole nested matrix of tests.

RReverser · 2026-04-19T09:40:54Z


 #[tracing::instrument(level = "debug", ret, skip(socket))]
 fn join_multicast_groups(socket: &UdpSocket, listen_addr: Ipv6Addr) {
+    let interfaces = match get_active_interfaces() {


We should surface this as an error through Result. Can't do much discovery if we couldn't get a list of interfaces.

Prior CI ran clippy only on ubuntu-latest. The discovery code had never been exercised in CI, which hid a latent SO_BROADCAST gap and a test-only panic on runners lacking link-local IPv6. - Add a discovery-tests matrix across ubuntu-latest, macos-latest, windows-latest. - Share the Swatinem/rust-cache@v2 cache per OS via shared-key so the seven clippy matrix jobs and three discovery-tests jobs reuse one cache per OS. ~/.cargo/registry is identical across feature sets so this is a clear win; target/ reuse is partial but still helps. - Add two CI-friendly tests that don't touch the DEFAULT_ADDR LazyLock (which requires link-local IPv6 on the default interface, unavailable on headless runners): - test_unspecified_v4_only: server on 0.0.0.0 exercises IPv4 subnet broadcast discovery. - test_loopback_v6_only: server on ::1 exercises IPv6 server bind, join_multicast_v6 on the loopback interface, V6 unicast send, and V6 response handling end-to-end. Co-Authored-By: Claude Opus 4.7 <[email protected]>

Without SO_BROADCAST, sending to IPv4 subnet broadcast addresses (e.g. 127.255.255.255, 192.168.1.255) returns EACCES on Linux and macOS. Windows accepts the sends regardless, which is why the pre-existing tests appeared to work there but not elsewhere. This was latent because the discovery tests never ran in CI. Discovered while adding the discovery-tests matrix: Ubuntu and macOS jobs failed with "Permission denied (os error 13)" on every send_to, producing zero discovered servers. Also simplify the test assertion to use Iterator::any instead of collecting into a Vec just to check is_empty.

Previously, if if_addrs failed to enumerate interfaces, the server logged a warning and bound successfully with no multicast joins — a silently crippled discovery server. Per PR review: can't do useful discovery if we can't list interfaces, so surface it via Result and let the caller decide. Co-Authored-By: Claude Opus 4.7 <[email protected]>

On Windows, if-addrs reports separate interface indices for V4 and V6 entries of the same adapter (IfIndex vs Ipv6IfIndex in IP_ADAPTER_ADDRESSES), and these can differ. The previous grouping kept whichever index came first, so IPv6 multicast ops (set_multicast_if_v6, join_multicast_v6) could get the wrong index on Windows when the V4 entry was enumerated first. Rename GroupedInterface.index -> ipv6_index and populate it only from V6 entries. The V4 index is not stored because IPv4 discovery uses subnet-directed broadcast, where interface selection happens via the routing table. On Unix, if_nametoindex returns the same value regardless of address family, so restricting to V6 entries is equivalent. CI can't exercise this directly (our runners have V4-only NICs plus loopback with equal V4/V6 indices) but the fix is a straight read of the Windows API contract. Co-Authored-By: Claude Opus 4.7 <[email protected]>

Switch from 10.254.254.254 to 192.0.2.1 (RFC 5737) in the UDP socket trick used to infer the default-route IP. VPNs commonly push routes for RFC1918 prefixes (10/8, 172.16/12, 192.168/16), so a 10.0.0.0/8 target can resolve to the VPN tun interface instead of the true default route. TEST-NET ranges are reserved for documentation and are very rarely matched by a more-specific route. No effect in CI (no VPN) but reduces flakiness for developers running tests on machines with a corporate VPN. Co-Authored-By: Claude Opus 4.7 <[email protected]>

ivonnyssen · 2026-04-22T00:26:05Z

Thanks for the reviews! Pushed updates (force-push to squash 10 exploratory commits into 5 clean ones). Summary:

Addressed

@RReverser — "Merely running the discovery test on macOS as a separate job should be enough" → Added a discovery-tests matrix across ubuntu-latest, macos-latest, windows-latest. Went broader than just macOS because the discovery code had never been in CI at all; this caught two latent bugs that I also fixed (see below). Cache is shared per-OS via shared-key so the matrix isn't painful.
@RReverser (src/server/discovery.rs:33) — "We should surface this as an error through Result" → Fixed in commit 4c93440. join_multicast_groups now returns Result<()> and Server::bind propagates it.
@copilot (src/discovery.rs:156) — "Use a TEST-NET address instead of 10.254.254.254" → Fixed in commit 7508725. Switched to 192.0.2.1 (RFC 5737 TEST-NET-1).

Declining (with evidence)

@copilot (src/client/discovery.rs:50 and src/server/discovery.rs:27) — "Fallback when intf.index is None" → I ran a probe across all three CI runners (temporary commit, since reverted) dumping every interface's name and index as seen by if-addrs. index was Some(_) for every operational interface on every OS. Ubuntu/macOS if_nametoindex only returns None if the interface disappears mid-enumeration (race); Windows's per-AF indices are None only when that address family isn't configured, which wouldn't matter for the code paths. Given no observed occurrence and the complexity of a correct fallback (IPV6_MULTICAST_IF is sticky on the socket; join_multicast_v6 with index 0 has platform-dependent semantics), I'd rather skip with a warning than invent fallback logic that can't be tested.

Bonus fix surfaced during investigation

Commit 3f91e5c: Windows V4/V6 dual-index bug. While investigating the None-index question, I found that if-addrs on Windows reports separate indices for V4 and V6 entries of the same adapter (IfIndex vs Ipv6IfIndex in IP_ADAPTER_ADDRESSES), and they can differ. The GroupedInterface.index: Option<u32> field conflated the two, so IPv6 multicast ops could pick the V4 index. Renamed to ipv6_index and populated only from V6 entries. On Unix this is equivalent (if_nametoindex is AF-agnostic). CI can't exercise this directly (the runners have V4-only NICs plus loopback with matching V4/V6 indices), but the Windows API docs make the fix straight-forward.
Commit 6c0fd61: SO_BROADCAST gap. The new CI matrix immediately failed on Ubuntu and macOS with EACCES on every send_to. Root cause: the client never set SO_BROADCAST, which Linux and macOS require to send to IPv4 subnet broadcast addresses. Windows is lax so pre-existing tests there appeared to work.
Two new CI-friendly tests (test_unspecified_v4_only, test_loopback_v6_only) that exercise real discovery paths without touching DEFAULT_ADDR — which panics on runners lacking link-local IPv6 on the default interface.

Final commit stack on top of ddec9a0:

7508725 Use TEST-NET-1 address for default-route detection in tests
3f91e5c Use V6-specific interface index for IPv6 multicast ops
4c93440 Propagate get_active_interfaces() error from discovery server bind
6c0fd61 Set SO_BROADCAST on discovery client socket
d5ad1c0 Add cross-platform CI discovery tests

All three OSes green.

claude and others added 2 commits April 6, 2026 23:32

ivonnyssen marked this pull request as ready for review April 12, 2026 21:25

Copilot AI review requested due to automatic review settings April 12, 2026 21:25

Copilot started reviewing on behalf of ivonnyssen April 12, 2026 21:25 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

RReverser reviewed Apr 19, 2026

View reviewed changes

ivonnyssen force-pushed the pr/macos-compilation-fix branch from eb06dd0 to 7089003 Compare April 21, 2026 19:29

ivonnyssen and others added 5 commits April 21, 2026 17:17

ivonnyssen force-pushed the pr/macos-compilation-fix branch from dfc6dc5 to 7508725 Compare April 22, 2026 00:18

ivonnyssen added a commit to ivonnyssen/ascom-alpaca-rs that referenced this pull request Apr 22, 2026

Integrate PR RReverser#16: macOS compilation fix

63f93ef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace netdev with if-addrs to fix macOS compilation#16

Replace netdev with if-addrs to fix macOS compilation#16
ivonnyssen wants to merge 8 commits intoRReverser:mainfrom
ivonnyssen:pr/macos-compilation-fix

ivonnyssen commented Apr 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

RReverser left a comment

Uh oh!

RReverser Apr 19, 2026

Uh oh!

ivonnyssen commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

ivonnyssen commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Motivation

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

RReverser left a comment

Choose a reason for hiding this comment

Uh oh!

RReverser Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

ivonnyssen commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ivonnyssen commented Apr 7, 2026 •

edited

Loading