Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FreeBSD support #49

Merged
merged 3 commits into from
Feb 1, 2024
Merged

FreeBSD support #49

merged 3 commits into from
Feb 1, 2024

Conversation

ydirson
Copy link
Contributor

@ydirson ydirson commented Dec 18, 2023

rtnetlink has 2 problems building on FreeBSD:

  • no support for Linux namespaces and traffic control, addressed in first commit
  • lack of AddressFamily::Bridge in netlink-packet-route, addressed in second commit

I originally thought about hiding namespaces and traffic control behind a ns feature, what do you think?

That second point brings me back to my original suggestion of having enum values defined once, regardless of target platform support.

@cathay4t
Copy link
Member

@ydirson Please fix the CI failure.

Meanwhile, please include https://github.com/rust-netlink/netlink-packet-route/blob/main/.github/workflows/build.yml to this PR, so we know this project can actually compile on FreeBSD. (You may only keep x86_64-unknown-freebsd and x86_64-unknown-linux-gnu).

Thank you!

@ydirson
Copy link
Contributor Author

ydirson commented Jan 10, 2024

@ydirson Please fix the CI failure.

Meanwhile, please include https://github.com/rust-netlink/netlink-packet-route/blob/main/.github/workflows/build.yml to this PR, so we know this project can actually compile on FreeBSD. (You may only keep x86_64-unknown-freebsd and x86_64-unknown-linux-gnu).

The libc support still has not been merged, so this will not build yet out of the fox on freebsd. Things more very slowly there, but it's not clear if I should resubmit my patch for 0.2 instead but protected by a feature flag.

@ydirson
Copy link
Contributor Author

ydirson commented Jan 10, 2024

@ydirson Please fix the CI failure.

I'm unsure how to handle examples that can only build under a given cfg(), since examples (and tests etc) are implicitly discovered. I just cannot find any hint of cargo supporting this use-case.

I tried guarding the example code with #[cfg(any(linux, android))], but that ends with a build failure when there is no valid main(). Is the only way out to have an empty main() for unsupported configuration?

@cathay4t
Copy link
Member

cathay4t commented Jan 18, 2024

@ydirson Please fix the CI failure.

I'm unsure how to handle examples that can only build under a given cfg(), since examples (and tests etc) are implicitly discovered. I just cannot find any hint of cargo supporting this use-case.

I tried guarding the example code with #[cfg(any(linux, android))], but that ends with a build failure when there is no valid main(). Is the only way out to have an empty main() for unsupported configuration?

Instead of #[cfg(any(linux, android))], can we do #[cfg(not(freebsd))], the former one ruled out some os(e.g. fuchsia) from compiling this crate which might works well before.

Not supported there.

Signed-off-by: Yann Dirson <[email protected]>
I'm not really happy with the readability, there ought to be a real
way to mark an example as "non applicable in cfg so and so".

Signed-off-by: Yann Dirson <[email protected]>
@ydirson
Copy link
Contributor Author

ydirson commented Jan 18, 2024

Instead of #[cfg(any(linux, android))], can we do #[cfg(not(freebsd))], the former one ruled out some os(e.g. fuchsia) from compiling this crate which might works well before.

Done - that had to be #[cfg(target_os = "freebsd")], though

@cathay4t cathay4t merged commit 4cbb9be into rust-netlink:main Feb 1, 2024
3 checks passed
@cathay4t
Copy link
Member

cathay4t commented Feb 1, 2024

@ydirson The 0.14.1 release contains this PR now. Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants