libc
libc copied to clipboard
rust-lang
freebsd: move net/if_mib.h contents to submodule
There is a conflict of NETLINK_GENERIC definitions between net/if_mib.h and netlink/netlink.h. netlink.h is already exported in the crate root for Linux (and those definitions are already used by at least crates neli and netlink-packet-route), and if_mib is not much used yet, so this moves if_mib contents into its own namespace to leave place for netlink support on FreeBSD (rust-lang/libc#3201).
Looks like it is the first time namespacing is used in the libc crate, so I had to guess a few things, being quite new to this project.
The moved symbols did not appear in semver, so I just added the new ones. sysinfo from @GuillaumeGomez seems to be the only impacted public crate.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.
Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:
@rustbot author: the review is finished, PR author should check the comments and take action accordingly@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
Even if only sysinfo is impacted, it remains a breaking change (but thanks for the heads-up!).
New revision completely drops the semver part -- will gladly use any hint to get it working and add it back.
To avoid the semver issue, the only way I see is to reexport items from the submodule by default.
To avoid the semver issue, the only way I see is to reexport items from the submodule by default.
That would avoid the breaking change, but that would not work for NETLINK_GENERIC, which this PR is all about.
Indeed. Let's see if libc's maintainers have another way to do it. But otherwise, I'm very fine with the breaking change. I'll just update libc in sysinfo and everything should be fine.
I'm baffled by the tests still trying to look at ifmib::* after I removed them from semver...
I'm baffled by the tests still trying to look at
ifmib::*after I removed them from semver...
OK I realize they come from the generated tests. This part is essentially dedicated to ctest, and I don't see a way there to tell a header's contents live in a submodule.
Looks like for now we'll have to flag those definitions not to be checked.
Note also that private code might use these items. The libs-maintainers team has been considering making a new major release (0.3? 1.0? I'm not sure though) and I think it'd be better to include this change there. Let's wait for that release.
Note also that private code might use these items. The libs-maintainers team has been considering making a new major release (0.3? 1.0? I'm not sure though) and I think it'd be better to include this change there. Let's wait for that release.
How can we track that here, when the symbols moved were not included in semver to start with? Is there a "list of breaking changes" somewhere to add this PR to for consideration?
:umbrella: The latest upstream changes (presumably #3341) made this pull request unmergeable. Please resolve the merge conflicts.
See https://github.com/rust-lang/libc/issues/3248, I've added this to the task list.
What about adding a feature flag to libc? An off-by-default "ifmib" feature would let crates like @ydirson's use the constants in their correct namespace, not conflicting with the new constant introduced by #3201 , while other crates won't see a change. And libc could remove the feature flag when it does next make a major release.
As I understand it, such a flag would enable either ifmib of netlink, so for 0.2 it could make sense to let ifmib be the default, and rather add a netlink feature flag to disable it and enable netlink support?
Whatever it's named, the default option should be the status quo, and the alternative would be to put ifmib into a submodule. BTW, I've seen other crates that call this "unstable".
I've seen other crates that call this "unstable".
That would seem a bit too generic here, we could just name it netlink.
However I see some moves on the 0.3 front, so should I proceed with this suggestion for 0.2?
:umbrella: The latest upstream changes (presumably #3587) made this pull request unmergeable. Please resolve the merge conflicts.