NetBSD 8 adds new address family changing AF_MAX and new ifaddrs flags.

[pusateri]

libc-test on NetBSD 8.0 fails without these changes.
ifa_addrflags was added to struct ifaddrs to support AF_CAN.
AF_MAX got bumped up one by AF_CAN.
RT_IFLIST format changed to support ifa_addrflags.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[semarie]

the NetBSD changes is an interesting breaking changes. the commit is NetBSD/src@6f66312#diff-b3f45d20f63d013e46f59f3f88a84d3dL477

for explaining a bit, NetBSD changed the name of constants, for example NET_RT_OOIFLIST (old-old-iflist) was renamed to NET_RT_OOOIFLIST (old-old-old-iflist), but keeping the binary compatibility (an old compiled binary using NET_RT_OOIFLIST (3 as value), still work the same way on differents systems versions.

Regarding the proposed PR, I dunno how it will be resolved. Commited as it, it will break NetBSD (pre 8) as NET_RT_OOIFLIST doesn't mean the same thing on pre-8 and post-8. And if not commited, it will break NetBSD (post 8).

@alexcrichton @gnzlbg something has to be done. The minimal thing would be policy on what is the supported version for each OS supported by libc ("last stable version" would be a good start for example). It would at least permit to know which change should be considered and which one is not.

More advanced stuff would be rust-lang/rfcs#2048 as it would permit supporting several OS versions (but the rfc stands for defining supported versions also).

[gnzlbg]

Does NetBSD still expose symbols of the APIs that were using the old version of ifaddrs ? If so, we could add an ifaddrs2 type that includes these changes, and _2 version of the APIs using ifaddrs2, and use #[link_name] to select what gets linked to which.

The alternative would be to just support NetBSD >= 8.0 and make a breaking change without bumping the major version of the libc crate.

[pusateri]

No, NetBSD 8.0 does not expose an older version of struct ifaddrs.

[gnzlbg]

No, NetBSD 8.0 does not expose an older version of struct ifaddrs

This is not exactly what I asked. I asked whether NetBSD 8.0 system libraries contain older versions of symbols that use the old ifaddrs struct on their ABI (note that if these are exposed they won't show up on any headers).

[pusateri]

I copied the NetBSD 7.2 binary for /sbin/route to NetBSD 8.0 and ran 'route monitor' which should print routing socket ifaddr messages when interfaces go up/down or add/delete addresses. After unplugging the ethernet interface, it didn't print the same route info under NetBSD 8.0 that it did under 7.2 and then crashed when I plugged the ethernet cable back in which worked fine in 7.2. It appears that 8.0 is not binary backward compatible with 7.2.

[gnzlbg]

It appears that 8.0 is not binary backward compatible with 7.2.

Yes, that appears to be the case indeed. I think "breaking" support for NetBSD 7.2 here and starting supporting NetBSD >= 8.0 is the only reasonable path forward beyond "doing nothing". For things like const values, we typically just do these changes, but adding a field to a struct prevents code that was compiling before from compiling now.

@alexcrichton what should we do here? This is an API-breaking change, but it makes little sense to gate this on "libc 1.0" like other similar changes: for all we know NetBSD 9.0 might add another new field to another struct and break compatibility again =/

---

EDIT: we could add a cargo feature "netbsd-8" that can be used to add these fields, but then we should think what we do with the const values, since they will be incorrect for netbsd 7.. I would prefer to not explicitly support multiple netbsd versions until there is a more "robust" way of doing that.

[semarie]

@jakllsch @ryoon as you seems involved in rust on netbsd side, I think having feedback from you will be appreciate: some netbsd version could be not longer supported by the Rust libc (or more complex to support in pkgsrc).

if you are not the right netbsd developers for this question, sorry about that, but please ping someone else.

[jakllsch]

I've got a string of commits sitting in my trees that result in a very similar diff to this pull request. I've not done a pull request for this yet due to NetBSD 7.x still being a supported release series, the official Rust x86_64-unknown-netbsd artifacts (last I checked) being built against a NetBSD-7, and that these changes only apply to NetBSD 7.99.39 or later, and also break support for NetBSD-7 (if any libc crate consumers utilize the ifaddrs structure in any way).

NetBSD-8 and -current still (as built by the NetBSD Foundation) support legacy NetBSD binaries (dating as far back as necessary for Rust) by default. Using the old syscalls/ioctls with the old structures should work, it's a bug if it doesn't. Sometimes backwards compatiblity bugs do creep in, as there isn't rigorous testing for it. I'll note that the routing socket interface has been a repeated source of binary compatibility problems in the past.

Unless there is a pressing need for the information in the addrflags field, or the AF_CAN or AF_ETHER constants by consumers of the libc crate, I would like to keep the official Rust compiler distribution artifact for NetBSD compatible with the oldest supported NetBSD release series. This will mean that some libc-test items must be expected to fail on one or the other supported stable NetBSD release series.

The latest four commits at https://github.com/jakllsch/rust-libc/commits/netbsd-20181205 show my work in this area. The only commit of these four that I would feel comfortable having merged with things as they are now (that is, with NetBSD-7 still being supported and the Rust platform versioning issue being unresolved) is jakllsch@f566707 which simply removes the NetBSD *_MAXID constants from the libc crate alltogether.

[pusateri]

I just tried running cargo test on NetBSD 7.2 and it fails there too.
/usr/include/netinet/dccp.h is missing (It was deleted before 7.1 release) which means 7.1 will probably fail too.

   Compiling libc-test v0.1.0 (/home/pusateri/Projects/libc/libc-test)
error: failed to run custom build command for `libc-test v0.1.0 (/home/pusateri/Projects/libc/libc-test)`
process didn't exit successfully: `/home/pusateri/Projects/libc/target/debug/build/libc-test-aa7af2bb6c9032d2/build-script-build` (exit code: 101)
--- stdout
-----------------------------------------
cargo:rerun-if-changed=../src/lib.rs
cargo:rerun-if-changed=../src/macros.rs
cargo:rerun-if-changed=../src/dox.rs
cargo:rerun-if-changed=../src/unix/mod.rs
cargo:rerun-if-changed=../src/unix/bsd/mod.rs
cargo:rerun-if-changed=../src/unix/bsd/netbsdlike/mod.rs
cargo:rerun-if-changed=../src/unix/bsd/netbsdlike/netbsd/mod.rs
cargo:rerun-if-changed=../src/unix/bsd/netbsdlike/netbsd/x86_64.rs
OPT_LEVEL = Some("0")
HOST = Some("x86_64-unknown-netbsd")
CC_x86_64-unknown-netbsd = None
CC_x86_64_unknown_netbsd = None
HOST_CC = None
CC = None
CFLAGS_x86_64-unknown-netbsd = None
CFLAGS_x86_64_unknown_netbsd = None
HOST_CFLAGS = None
CFLAGS = None
DEBUG = Some("true")
running: "cc" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "-m64" "-Wall" "-Wextra" "-Wall" "-Wextra" "-Werror" "-Wno-unused-parameter" "-Wno-type-limits" "-Wno-address-of-packed-member" "-Wno-deprecated-declarations" "-Wno-deprecated-declarations" "-D_NETBSD_SOURCE=1" "-o" "/home/pusateri/Projects/libc/target/debug/build/libc-test-beca0575d5180d93/out/main.o" "-c" "/home/pusateri/Projects/libc/target/debug/build/libc-test-beca0575d5180d93/out/main.c"
cargo:warning=/home/pusateri/Projects/libc/target/debug/build/libc-test-beca0575d5180d93/out/main.c:67:26: fatal error: netinet/dccp.h: No such file or directory
cargo:warning= #include <netinet/dccp.h>
cargo:warning=                          ^
cargo:warning=compilation terminated.
exit code: 1

--- stderr
error: attributes on struct pattern or literal fields are unstable (see issue #38814)
   --> ../src/unix/bsd/netbsdlike/netbsd/mod.rs:711:5
    |
711 | /     #[cfg(any(target_arch = "sparc", target_arch = "sparc64",
712 | |               target_arch = "x86", target_arch = "x86_64"))]
    | |____________________________________________________________^
    | 
   ::: ../src/lib.rs
    |
177 | / cfg_if! {
178 | |     if #[cfg(windows)] {
179 | |         mod windows;
180 | |         pub use windows::*;
...   |
201 | |     }
202 | | }
    | |_- in this macro invocation
    |
    = help: add #![feature(struct_field_attributes)] to the crate attributes to enable

error: attributes on struct pattern or literal fields are unstable (see issue #38814)
   --> ../src/unix/bsd/netbsdlike/netbsd/mod.rs:715:5
    |
715 | /     #[cfg(any(target_arch = "sparc", target_arch = "sparc64",
716 | |               target_arch = "x86", target_arch = "x86_64"))]
    | |____________________________________________________________^
    | 
   ::: ../src/lib.rs
    |
177 | / cfg_if! {
178 | |     if #[cfg(windows)] {
179 | |         mod windows;
180 | |         pub use windows::*;
...   |
201 | |     }
202 | | }
    | |_- in this macro invocation
    |
    = help: add #![feature(struct_field_attributes)] to the crate attributes to enable

thread 'main' panicked at '

Internal error occurred: Command "cc" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "-m64" "-Wall" "-Wextra" "-Wall" "-Wextra" "-Werror" "-Wno-unused-parameter" "-Wno-type-limits" "-Wno-address-of-packed-member" "-Wno-deprecated-declarations" "-Wno-deprecated-declarations" "-D_NETBSD_SOURCE=1" "-o" "/home/pusateri/Projects/libc/target/debug/build/libc-test-beca0575d5180d93/out/main.o" "-c" "/home/pusateri/Projects/libc/target/debug/build/libc-test-beca0575d5180d93/out/main.c" with args "cc" did not execute successfully (status code exit code: 1).

', /home/pusateri/.cargo/registry/src/github.1485827954.workers.dev-1ecc6299db9ec823/cc-1.0.25/src/lib.rs:2260:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

[alexcrichton]

We don't really have a policy for these sorts of changes. ABI compatibility breaks seems to basically never happen on the "tier 1" platforms and so we just haven't gotten around to developing a policy for this. I think it's probably fine to do this on a platform-by-platform basis, so if those involved with the NetBSD Rust community feel that this is an appropriate change to make then seems like we should make it!

[semarie]

ABI compatibility breaks seems to basically never happen on the "tier 1" platforms

should I point to you the MSVC 2015 entry present in the RFC 2048 ? breaking changes already occurs on Windows in the past. The RFC pointed gets() function removal as example.

[retep998]

The C standard library on Windows absolutely has made breaking ABI changes in the past, hence msvcr100.dll msvcr110.dll msvcr120.dll vcruntime140.dll and so on. Of course libc doesn't actually expose that much on Windows, so libc would be unlikely to notice those breaking changes.

[inferiorhumanorgans]

IMO it would be worth defining a support matrix and including the ABI version with the host triplet here (e.g. amd64-unknown-freebsd12 or amd64-netbsd8). FWIW here's how things look on the other BSDs:

DragonFly 5.4: sys/ioctl_compat.h was removed

FreeBSD 11: pass
FreeBSD 12: struct mismatch

NetBSD 7.0: syn failed to compile
NetBSD 7.1: netinet/dccp.h missing + attributes on struct pattern or literal fields are unstable
NetBSD 7.2: netinet/dccp.h missing + attributes on struct pattern or literal fields are unstable

OpenBSD 6.4: KERN_CPUSTATS undeclared

I think there's not necessarily a pressing need to support the latest interfaces, because most functions will work. But there is a need to ensure that tests aren't left known broken or disabled because not everything works (e.g. uname(3) on FreeBSD).

[gnzlbg]

@bors: r+

---

Feel free to submit a PR adding information about the supported NetBSD version to the readme. Also, if any of you is involved in NetBSD development, it might be interesting to discuss how we could avoid breakage in the future.

Some simple "C guidelines" like:

• only extending C enums by adding new values at their end
• when the layout of a type changes (e.g. new fields added), keep the old API in the library binary so that we can link to that from here (e.g. by using a special link name), and so that we can add "versioned" version of the types (e.g. foo -> foo2, api_foo -> api_foo2) that call the different symbols

[bors]

📌 Commit 0f712b3 has been approved by gnzlbg

[bors]

⌛ Testing commit 0f712b3 with merge 18b0102...

[bors]

☀️ Test successful - checks-cirrus, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing 18b0102 to master...

[inferiorhumanorgans]

@gnzlbg I'm happy to submit a PR to update the documentation but am not sure what the intended support goals are. For instance the FreeBSD support seems to be targeting FreeBSD 11 (the previous stable release), but I'm seeing commits for OpenBSD 6.5 (which has not yet been released).

I'm not a *BSD committer only an end-user with a small build farm. I would suggest that the NetBSD team is well acquainted with how to maintain binary compatibility but that rust-libc is making an incorrect assumption about how the ABI is versioned. IMO the path of least resistance would be to version the host triplet on *BSD to align properly with how *BSD maintains a stable ABI.